* [PATCH] ethtool: potential NULL dereference in strset_prepare_data()
@ 2020-01-08 5:42 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2020-01-08 5:42 UTC (permalink / raw)
To: David S. Miller; +Cc: Michal Kubecek, Florian Fainelli, netdev, kernel-janitors
Smatch complains that the NULL checking isn't done consistently:
net/ethtool/strset.c:253 strset_prepare_data()
error: we previously assumed 'dev' could be null (see line 233)
It looks like there is a missing return on this path.
Fixes: 71921690f974 ("ethtool: provide string sets with STRSET_GET request")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
net/ethtool/strset.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index 9f2243329015..82a059c13c1c 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -239,6 +239,7 @@ static int strset_prepare_data(const struct ethnl_req_info *req_base,
return -EINVAL;
}
}
+ return 0;
}
ret = ethnl_ops_begin(dev);
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] ethtool: potential NULL dereference in strset_prepare_data()
@ 2020-01-08 5:42 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2020-01-08 5:42 UTC (permalink / raw)
To: David S. Miller; +Cc: Michal Kubecek, Florian Fainelli, netdev, kernel-janitors
Smatch complains that the NULL checking isn't done consistently:
net/ethtool/strset.c:253 strset_prepare_data()
error: we previously assumed 'dev' could be null (see line 233)
It looks like there is a missing return on this path.
Fixes: 71921690f974 ("ethtool: provide string sets with STRSET_GET request")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
net/ethtool/strset.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index 9f2243329015..82a059c13c1c 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -239,6 +239,7 @@ static int strset_prepare_data(const struct ethnl_req_info *req_base,
return -EINVAL;
}
}
+ return 0;
}
ret = ethnl_ops_begin(dev);
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ethtool: potential NULL dereference in strset_prepare_data()
2020-01-08 5:42 ` Dan Carpenter
@ 2020-01-08 9:17 ` Michal Kubecek
-1 siblings, 0 replies; 6+ messages in thread
From: Michal Kubecek @ 2020-01-08 9:17 UTC (permalink / raw)
To: netdev; +Cc: Dan Carpenter, David S. Miller, Florian Fainelli, kernel-janitors
On Wed, Jan 08, 2020 at 08:42:36AM +0300, Dan Carpenter wrote:
> Smatch complains that the NULL checking isn't done consistently:
>
> net/ethtool/strset.c:253 strset_prepare_data()
> error: we previously assumed 'dev' could be null (see line 233)
>
> It looks like there is a missing return on this path.
I believe this is a false positive as the first loop makes sure no
explicitly requested set is per device if dev is NULL so that we would
never actually call strset_prepare_set() with null dev.
But there is no point to go through the second part of the function if
it is not going to do anything and the fact that it took me few minutes
to make sure the null pointer dereference is not possible clearly
indicates the code is cleaner with the explicit return.
Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
> Fixes: 71921690f974 ("ethtool: provide string sets with STRSET_GET request")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> net/ethtool/strset.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
> index 9f2243329015..82a059c13c1c 100644
> --- a/net/ethtool/strset.c
> +++ b/net/ethtool/strset.c
> @@ -239,6 +239,7 @@ static int strset_prepare_data(const struct ethnl_req_info *req_base,
> return -EINVAL;
> }
> }
> + return 0;
> }
>
> ret = ethnl_ops_begin(dev);
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ethtool: potential NULL dereference in strset_prepare_data()
@ 2020-01-08 9:17 ` Michal Kubecek
0 siblings, 0 replies; 6+ messages in thread
From: Michal Kubecek @ 2020-01-08 9:17 UTC (permalink / raw)
To: netdev; +Cc: Dan Carpenter, David S. Miller, Florian Fainelli, kernel-janitors
On Wed, Jan 08, 2020 at 08:42:36AM +0300, Dan Carpenter wrote:
> Smatch complains that the NULL checking isn't done consistently:
>
> net/ethtool/strset.c:253 strset_prepare_data()
> error: we previously assumed 'dev' could be null (see line 233)
>
> It looks like there is a missing return on this path.
I believe this is a false positive as the first loop makes sure no
explicitly requested set is per device if dev is NULL so that we would
never actually call strset_prepare_set() with null dev.
But there is no point to go through the second part of the function if
it is not going to do anything and the fact that it took me few minutes
to make sure the null pointer dereference is not possible clearly
indicates the code is cleaner with the explicit return.
Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
> Fixes: 71921690f974 ("ethtool: provide string sets with STRSET_GET request")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> net/ethtool/strset.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
> index 9f2243329015..82a059c13c1c 100644
> --- a/net/ethtool/strset.c
> +++ b/net/ethtool/strset.c
> @@ -239,6 +239,7 @@ static int strset_prepare_data(const struct ethnl_req_info *req_base,
> return -EINVAL;
> }
> }
> + return 0;
> }
>
> ret = ethnl_ops_begin(dev);
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ethtool: potential NULL dereference in strset_prepare_data()
2020-01-08 5:42 ` Dan Carpenter
@ 2020-01-09 0:04 ` David Miller
-1 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-01-09 0:04 UTC (permalink / raw)
To: dan.carpenter; +Cc: mkubecek, f.fainelli, netdev, kernel-janitors
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Wed, 8 Jan 2020 08:42:36 +0300
> Smatch complains that the NULL checking isn't done consistently:
>
> net/ethtool/strset.c:253 strset_prepare_data()
> error: we previously assumed 'dev' could be null (see line 233)
>
> It looks like there is a missing return on this path.
>
> Fixes: 71921690f974 ("ethtool: provide string sets with STRSET_GET request")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ethtool: potential NULL dereference in strset_prepare_data()
@ 2020-01-09 0:04 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-01-09 0:04 UTC (permalink / raw)
To: dan.carpenter; +Cc: mkubecek, f.fainelli, netdev, kernel-janitors
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Wed, 8 Jan 2020 08:42:36 +0300
> Smatch complains that the NULL checking isn't done consistently:
>
> net/ethtool/strset.c:253 strset_prepare_data()
> error: we previously assumed 'dev' could be null (see line 233)
>
> It looks like there is a missing return on this path.
>
> Fixes: 71921690f974 ("ethtool: provide string sets with STRSET_GET request")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-09 0:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-08 5:42 [PATCH] ethtool: potential NULL dereference in strset_prepare_data() Dan Carpenter
2020-01-08 5:42 ` Dan Carpenter
2020-01-08 9:17 ` Michal Kubecek
2020-01-08 9:17 ` Michal Kubecek
2020-01-09 0:04 ` David Miller
2020-01-09 0:04 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.