linux-kernel - Re: [PATCH v2] rtl8712: add a check for the status of register

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] [day] [month] [year] [list]

Message-ID: <20190102103421.GG3781@kadam>
Date:   Wed, 2 Jan 2019 13:34:21 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Kangjie Lu <kjlu@....edu>
Cc:     devel@...verdev.osuosl.org,
        Florian Schilhabel <florian.c.schilhabel@...glemail.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org,
        André Lehmann <lenneman@....de>,
        pakki001@....edu, Larry Finger <Larry.Finger@...inger.net>
Subject: Re: [PATCH v2] rtl8712: add a check for the status of register_netdev

On Tue, Dec 25, 2018 at 08:39:53PM -0600, Kangjie Lu wrote:
> register_netdev() may fail, so let's check its return value, and if it
> fails, issue an error message.
> 
> Signed-off-by: Kangjie Lu <kjlu@....edu>
> ---
>  drivers/staging/rtl8712/hal_init.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c
> index 7cdd609cab6c..ca0858660a45 100644
> --- a/drivers/staging/rtl8712/hal_init.c
> +++ b/drivers/staging/rtl8712/hal_init.c
> @@ -32,10 +32,10 @@
>  static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context)
>  {
>  	struct _adapter *padapter = context;
> +	struct usb_device *udev = padapter->dvobjpriv.pusbdev;
>  
>  	complete(&padapter->rtl8712_fw_ready);
>  	if (!firmware) {
> -		struct usb_device *udev = padapter->dvobjpriv.pusbdev;
>  		struct usb_interface *pusb_intf = padapter->pusb_intf;
>  
>  		dev_err(&udev->dev, "r8712u: Firmware request failed\n");
> @@ -45,7 +45,8 @@ static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context)
>  	}
>  	padapter->fw = firmware;
>  	/* firmware available - start netdev */
> -	register_netdev(padapter->pnetdev);
> +	if (register_netdev(padapter->pnetdev))
> +		dev_err(&udev->dev, "r8712u: Registering netdev failed\n");

This error handling doesn't seem complete.  You're just adding the
minimum to make the static analysis tool happy.

If you leave the code as-is, maybe someone else will see the static
analysis warning and fix it properly.  But you're silencing the warning
so now no one will fix it.

Warnings are valuable.  Don't silence them without fixing the bug.

regards,
dan carpenter