lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 8 Jan 2020 10:17:52 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     netdev@...r.kernel.org
Cc:     Dan Carpenter <dan.carpenter@...cle.com>,
        "David S. Miller" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] ethtool: potential NULL dereference in
 strset_prepare_data()

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@...e.cz>

> Fixes: 71921690f974 ("ethtool: provide string sets with STRSET_GET request")
> Signed-off-by: Dan Carpenter <dan.carpenter@...cle.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
> 

Powered by blists - more mailing lists