[PATCH][next] drivers: base: swnode: check if swnode is null before dereferencing it

[PATCH][next] drivers: base: swnode: check if swnode is null before dereferencing it

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

The macro to_software_mode can potentially return NULL, so also add
a null check on the swnode before dereferencing it to avoid any null
pointer dereferences.

Detected by CoverityScan, CID#1476052 ("Explicit null dereferenced")

Fixes: 59abd83672f7 ("drivers: base: Introducing software nodes to the firmware node framework")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/base/swnode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 306bb93287af..204aa7d049e0 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -477,7 +477,8 @@ software_node_get_parent(const struct fwnode_handle *fwnode)
 {
 	struct software_node *swnode = to_software_node(fwnode);
 
-	return swnode->parent ? &swnode->parent->fwnode : NULL;
+	return swnode ? (swnode->parent ? &swnode->parent->fwnode : NULL) :
+			NULL;
 }
 
 struct fwnode_handle *
-- 
2.19.1

Re: [PATCH][next] drivers: base: swnode: check if swnode is null before dereferencing it

[Rafael J. Wysocki]

On Sat, Dec 22, 2018 at 1:43 PM Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> The macro to_software_mode can potentially return NULL, so also add
> a null check on the swnode before dereferencing it to avoid any null
> pointer dereferences.
>
> Detected by CoverityScan, CID#1476052 ("Explicit null dereferenced")
>
> Fixes: 59abd83672f7 ("drivers: base: Introducing software nodes to the firmware node framework")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/base/swnode.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 306bb93287af..204aa7d049e0 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -477,7 +477,8 @@ software_node_get_parent(const struct fwnode_handle *fwnode)
>  {
>         struct software_node *swnode = to_software_node(fwnode);
>
> -       return swnode->parent ? &swnode->parent->fwnode : NULL;
> +       return swnode ? (swnode->parent ? &swnode->parent->fwnode : NULL) :
> +                       NULL;
>  }
>
>  struct fwnode_handle *
> --

Applied, thanks!

Re: [PATCH][next] drivers: base: swnode: check if swnode is null before dereferencing it

[Rafael J. Wysocki]

On Sat, Dec 22, 2018 at 1:43 PM Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> The macro to_software_mode can potentially return NULL, so also add
> a null check on the swnode before dereferencing it to avoid any null
> pointer dereferences.
>
> Detected by CoverityScan, CID#1476052 ("Explicit null dereferenced")
>
> Fixes: 59abd83672f7 ("drivers: base: Introducing software nodes to the firmware node framework")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/base/swnode.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 306bb93287af..204aa7d049e0 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -477,7 +477,8 @@ software_node_get_parent(const struct fwnode_handle *fwnode)
>  {
>         struct software_node *swnode = to_software_node(fwnode);
>
> -       return swnode->parent ? &swnode->parent->fwnode : NULL;
> +       return swnode ? (swnode->parent ? &swnode->parent->fwnode : NULL) :
> +                       NULL;
>  }
>
>  struct fwnode_handle *
> --

Applied, thanks!

Re: [PATCH][next] drivers: base: swnode: check if swnode is null before dereferencing it

[Dan Carpenter]

On Sat, Dec 22, 2018 at 12:43:33PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The macro to_software_mode can potentially return NULL, so also add
> a null check on the swnode before dereferencing it to avoid any null
> pointer dereferences.
> 
> Detected by CoverityScan, CID#1476052 ("Explicit null dereferenced")
> 
> Fixes: 59abd83672f7 ("drivers: base: Introducing software nodes to the firmware node framework")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

to_software_mode() can't return NULL though...

We shouldn't change the code just to make the static checker happy.

Sometimes if we just silence every static checker warning maybe we will
fix some bugs in the middle of silencing all the false positive, but
Smatch is almost at the stage of being able to parse this code
correctly.  Maybe by the end of the year.  So let's hold off and then
think about taking the fix everything approach next year.

regards,
dan carpenter

Re: [PATCH][next] drivers: base: swnode: check if swnode is null before dereferencing it

[Dan Carpenter]

On Sat, Dec 22, 2018 at 12:43:33PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The macro to_software_mode can potentially return NULL, so also add
> a null check on the swnode before dereferencing it to avoid any null
> pointer dereferences.
> 
> Detected by CoverityScan, CID#1476052 ("Explicit null dereferenced")
> 
> Fixes: 59abd83672f7 ("drivers: base: Introducing software nodes to the firmware node framework")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

to_software_mode() can't return NULL though...

We shouldn't change the code just to make the static checker happy.

Sometimes if we just silence every static checker warning maybe we will
fix some bugs in the middle of silencing all the false positive, but
Smatch is almost at the stage of being able to parse this code
correctly.  Maybe by the end of the year.  So let's hold off and then
think about taking the fix everything approach next year.

regards,
dan carpenter