Thread (17 messages) 17 messages, 3 authors, 2024-10-04

Re: powerpc/pseries: Fix exception handling in pSeries_reconfig_add_node()

From: Nathan Lynch <hidden>
Date: 2023-03-20 15:48:02
Also in: cocci, kernel-janitors, lkml

Markus Elfring [off-list ref] writes:
quoted
quoted
quoted
quoted
The label “out_err” was used to jump to another pointer check despite of
the detail in the implementation of the function “pSeries_reconfig_add_node”
that it was determined already that the corresponding variable contained
a null pointer (because of a failed function call in two cases).

1. Thus return directly after a call of the function “kzalloc” failed.

2. Use more appropriate labels instead.

3. Delete a redundant check.

4. Omit an explicit initialisation for the local variable “err”.

This issue was detected by using the Coccinelle software.
Is there a correctness or safety issue here?
I got the impression that the application of only a single label like “out_err”
resulted in improvable implementation details.
I don't understand what you're trying to say here.
What does hinder you to understand the presented change description better
at the moment?

quoted
It doesn't seem to answer my question.

I hope that my answer will trigger further helpful considerations.
I don't consider this response constructive, but I want to get this back
on track. It's been brought to my attention that there is in fact a
crash bug in this function's error path:

	np->parent = pseries_of_derive_parent(path);
	if (IS_ERR(np->parent)) {
		err = PTR_ERR(np->parent);
		goto out_err;
	}
...
out_err:
	if (np) {
		of_node_put(np->parent);

np->parent can be an encoded error value, we don't want to of_node_put()
that.

I believe the patch as written happens to fix the issue. Will you please
write it up as a bug fix and resubmit?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help