linux-kernel - Re: [PATCH] staging: r8188eu: remove unnecessary NULL check

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 23 Nov 2021 11:06:21 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Vihas Mak <makvihas@...il.com>
Cc:     Larry.Finger@...inger.net, phil@...lpotter.co.uk,
        gregkh@...uxfoundation.org, straube.linux@...il.com,
        martin@...ser.cx, paskripkin@...il.com,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: r8188eu: remove unnecessary NULL check

On Tue, Nov 23, 2021 at 01:23:50AM +0530, Vihas Mak wrote:
> remove unnecessary NULL check surrounding rtw_free_netdev(), as the check
> is already performed inside rtw_free_netdev() in
> drivers/staging/r8188eu/os_dep/osdep_service.c.
> 
> Signed-off-by: Vihas Mak <makvihas@...il.com>
> ---
>  drivers/staging/r8188eu/os_dep/usb_intf.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> index 5a35d9fe3fc9..392bd7868519 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> @@ -466,8 +466,7 @@ static void rtw_usb_if1_deinit(struct adapter *if1)
>  		if1->hw_init_completed);
>  	rtw_handle_dualmac(if1, 0);
>  	rtw_free_drv_sw(if1);
> -	if (pnetdev)
> -		rtw_free_netdev(pnetdev);
> +	rtw_free_netdev(pnetdev);
>  }

I'm not a huge fan of these sorts of patches.  They don't make the code
more readable because they hide the complexity.

Occasionally we will get a forest cobra in our yard and everyone is
screaming and panicking.  I'm like, "Calm down.  Once you've spotted the
snake, even a deadly snake, then the danger has passed."  You can just
stay two or three meters away and you're fine.  Call a snake catcher.

What you're doing here is you've got a potential NULL dereference which
is the snake.  And this patch is saying, "Snakes are so messy!  Let's
hide it in the bushes next to the sidewalk where no one can see it."

Hash tag, folksy wisdom.  #snakes

On the other hand, it might be worth checking if "pnetdev" can even be
NULL at this point, and then deleting both of the NULL checks.  That
would be a very good clean up.

regards,
dan carpenter