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

[Nathan Lynch]

Markus Elfring  writes:
>
> 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? The subject uses the word
"fix" but the commit message doesn't seem to identify one.

Can you share how Coccinelle is being invoked and its output?

[PATCH] powerpc/pseries: Fix exception handling in pSeries_reconfig_add_node()

[Markus Elfring]

Date: Fri, 17 Mar 2023 09:26:13 +0100

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.

Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring 
---
 arch/powerpc/platforms/pseries/reconfig.c | 26 ---
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/reconfig.c 
b/arch/powerpc/platforms/pseries/reconfig.c
index 599bd2c78514..14154f48ef63 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -23,15 +23,17 @@
 static int pSeries_reconfig_add_node(const char *path, struct property 
*proplist)
 {
     struct device_node *np;
-    int err = -ENOMEM;
+    int err;
 
     np = kzalloc(sizeof(*np), GFP_KERNEL);
     if (!np)
-        goto out_err;
+        return -ENOMEM;
 
     np->full_name = kstrdup(kbasename(path), GFP_KERNEL);
-    if (!np->full_name)
-        goto out_err;
+    if (!np->full_name) {
+        err = -ENOMEM;
+        goto free_device_node;
+    }
 
     np->properties = proplist;
     of_node_set_flag(np, OF_DYNAMIC);
@@ -40,25 +42,25 @@ static int pSeries_reconfig_add_node(const char *path, 
struct property *proplist
     np->parent = pseries_of_derive_parent(path);
     if (IS_ERR(np->parent)) {
         err = PTR_ERR(np->parent);
-        goto out_err;
+        goto free_name;
     }
 
     err = of_attach_node(np);
     if (err) {
         printk(KERN_ERR "Failed to add device node %s\n", path);
-        goto out_err;
+        goto put_node;
     }
 
     of_node_put(np->parent);
 
     return 0;
 
-out_err:
-    if (np) {
-        of_node_put(np->parent);
-        kfree(np->full_name);
-        kfree(np);
-    }
+put_node:
+    of_node_put(np->parent);
+free_name:
+    kfree(np->full_name);
+free_device_node:
+    kfree(np);
     return err;
 }
 
--
2.40.0