[v2] xen/common: Don't dereference overlay_node after checking that it is NULL

[PATCH v2] xen/common: Don't dereference overlay_node after checking that it is NULL

Posted by Javi Merino 3 months, 3 weeks ago

In remove_nodes(), overlay_node is dereferenced when printing the
error message even though it is known to be NULL.  Return without
printing as an error message is already printed by the caller.

The semantic patch that spots this code is available in

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/null/deref_null.cocci?id=1f874787ed9a2d78ed59cb21d0d90ac0178eceb0

Fixes: 7e5c4a8b86f1 ("xen/arm: Implement device tree node removal functionalities")
Signed-off-by: Javi Merino <javi.merino@cloud.com>
---
CC: Vikram Garhwal <vikram.garhwal@amd.com>
CC: Julien Grall <julien@xen.org>

 xen/common/dt-overlay.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/xen/common/dt-overlay.c b/xen/common/dt-overlay.c
index 5663a049e90a..1b197381f616 100644
--- a/xen/common/dt-overlay.c
+++ b/xen/common/dt-overlay.c
@@ -427,11 +427,7 @@ static int remove_nodes(const struct overlay_track *tracker)
     {
         overlay_node = (struct dt_device_node *)tracker->nodes_address[j];
         if ( overlay_node == NULL )
-        {
-            printk(XENLOG_ERR "Device %s is not present in the tree. Removing nodes failed\n",
-                   overlay_node->full_name);
             return -EINVAL;
-        }
 
         rc = remove_descendant_nodes_resources(overlay_node);
         if ( rc )
-- 
2.42.0

Re: [PATCH v2] xen/common: Don't dereference overlay_node after checking that it is NULL

Posted by Julien Grall 3 months, 3 weeks ago

Hi Javi,

On 11/01/2024 12:09, Javi Merino wrote:
> In remove_nodes(), overlay_node is dereferenced when printing the
> error message even though it is known to be NULL.  Return without
> printing as an error message is already printed by the caller.
> 
> The semantic patch that spots this code is available in
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/null/deref_null.cocci?id=1f874787ed9a2d78ed59cb21d0d90ac0178eceb0
> 
> Fixes: 7e5c4a8b86f1 ("xen/arm: Implement device tree node removal functionalities")
> Signed-off-by: Javi Merino <javi.merino@cloud.com>

Acked-by: Julien Grall <jgrall@amazon.com>

And committed.

Cheers,

-- 
Julien Grall

Re: [PATCH v2] xen/common: Don't dereference overlay_node after checking that it is NULL

Posted by Vikram Garhwal 3 months, 3 weeks ago

On Thu, Jan 11, 2024 at 12:09:27PM +0000, Javi Merino wrote:
> In remove_nodes(), overlay_node is dereferenced when printing the
> error message even though it is known to be NULL.  Return without
> printing as an error message is already printed by the caller.
> 
> The semantic patch that spots this code is available in
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/null/deref_null.cocci?id=1f874787ed9a2d78ed59cb21d0d90ac0178eceb0
> 
> Fixes: 7e5c4a8b86f1 ("xen/arm: Implement device tree node removal functionalities")
> Signed-off-by: Javi Merino <javi.merino@cloud.com>
Reviewed-by: Vikram Garhwal <vikram.garhwal@amd.com>

> ---
> CC: Vikram Garhwal <vikram.garhwal@amd.com>
> CC: Julien Grall <julien@xen.org>
> 
>  xen/common/dt-overlay.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/xen/common/dt-overlay.c b/xen/common/dt-overlay.c
> index 5663a049e90a..1b197381f616 100644
> --- a/xen/common/dt-overlay.c
> +++ b/xen/common/dt-overlay.c
> @@ -427,11 +427,7 @@ static int remove_nodes(const struct overlay_track *tracker)
>      {
>          overlay_node = (struct dt_device_node *)tracker->nodes_address[j];
>          if ( overlay_node == NULL )
> -        {
> -            printk(XENLOG_ERR "Device %s is not present in the tree. Removing nodes failed\n",
> -                   overlay_node->full_name);
>              return -EINVAL;
> -        }
>  
>          rc = remove_descendant_nodes_resources(overlay_node);
>          if ( rc )
> -- 
> 2.42.0
>