linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: rtl8712: check register_netdev() return value
@ 2020-12-06 15:59 shaojie.dong
  2020-12-06 16:03 ` Greg KH
  2020-12-06 17:10 ` kernel test robot
  0 siblings, 2 replies; 6+ messages in thread
From: shaojie.dong @ 2020-12-06 15:59 UTC (permalink / raw)
  To: Larry.Finger, florian.c.schilhabel, gregkh
  Cc: devel, linux-kernel, shaojie.dong

From: "shaojie.dong" <shaojie.dong@isrc.iscas.ac.cn>

Function register_netdev() can fail, so we should check it's return value

Signed-off-by: shaojie.dong <shaojie.dong@isrc.iscas.ac.cn>
---
 drivers/staging/rtl8712/hal_init.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c
index 715f1fe8b..fbcc6de1b 100644
--- a/drivers/staging/rtl8712/hal_init.c
+++ b/drivers/staging/rtl8712/hal_init.c
@@ -45,7 +45,8 @@ static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context)
 	}
 	adapter->fw = firmware;
 	/* firmware available - start netdev */
-	register_netdev(adapter->pnetdev);
+	if (register_netdev(adapter->pnetdev) != 0)
+		dev_err(&udev->dev, "r8712u: register_netdev() failed\n");
 	complete(&adapter->rtl8712_fw_ready);
 }
 
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] staging: rtl8712: check register_netdev() return value
  2020-12-06 15:59 [PATCH] staging: rtl8712: check register_netdev() return value shaojie.dong
@ 2020-12-06 16:03 ` Greg KH
  2020-12-06 17:10 ` kernel test robot
  1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2020-12-06 16:03 UTC (permalink / raw)
  To: shaojie.dong; +Cc: Larry.Finger, florian.c.schilhabel, devel, linux-kernel

On Sun, Dec 06, 2020 at 11:59:07PM +0800, shaojie.dong@isrc.iscas.ac.cn wrote:
> From: "shaojie.dong" <shaojie.dong@isrc.iscas.ac.cn>
> 
> Function register_netdev() can fail, so we should check it's return value

You just check it, you are not doing anything with it, which is just the
same as not checking this.

Please fix this properly.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] staging: rtl8712: check register_netdev() return value
  2020-12-06 15:59 [PATCH] staging: rtl8712: check register_netdev() return value shaojie.dong
  2020-12-06 16:03 ` Greg KH
@ 2020-12-06 17:10 ` kernel test robot
  1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-12-06 17:10 UTC (permalink / raw)
  To: shaojie.dong, Larry.Finger, florian.c.schilhabel, gregkh
  Cc: kbuild-all, devel, linux-kernel, shaojie.dong

[-- Attachment #1: Type: text/plain, Size: 3276 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/0day-ci/linux/commits/shaojie-dong-isrc-iscas-ac-cn/staging-rtl8712-check-register_netdev-return-value/20201207-000540
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 138f3e1265488a9163be7f379073297ba8545cca
config: arc-allmodconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a5d44fc70b0f1b3d0a23e3c3bab16a04e4352ad2
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review shaojie-dong-isrc-iscas-ac-cn/staging-rtl8712-check-register_netdev-return-value/20201207-000540
        git checkout a5d44fc70b0f1b3d0a23e3c3bab16a04e4352ad2
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/usb/ch9.h:36,
                    from include/linux/usb.h:6,
                    from drivers/staging/rtl8712/hal_init.c:19:
   drivers/staging/rtl8712/hal_init.c: In function 'rtl871x_load_fw_cb':
>> drivers/staging/rtl8712/hal_init.c:49:12: error: 'udev' undeclared (first use in this function); did you mean 'cdev'?
      49 |   dev_err(&udev->dev, "r8712u: register_netdev() failed\n");
         |            ^~~~
   include/linux/dev_printk.h:112:11: note: in definition of macro 'dev_err'
     112 |  _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |           ^~~
   drivers/staging/rtl8712/hal_init.c:49:12: note: each undeclared identifier is reported only once for each function it appears in
      49 |   dev_err(&udev->dev, "r8712u: register_netdev() failed\n");
         |            ^~~~
   include/linux/dev_printk.h:112:11: note: in definition of macro 'dev_err'
     112 |  _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |           ^~~

vim +49 drivers/staging/rtl8712/hal_init.c

    31	
    32	static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context)
    33	{
    34		struct _adapter *adapter = context;
    35	
    36		if (!firmware) {
    37			struct usb_device *udev = adapter->dvobjpriv.pusbdev;
    38			struct usb_interface *usb_intf = adapter->pusb_intf;
    39	
    40			dev_err(&udev->dev, "r8712u: Firmware request failed\n");
    41			usb_put_dev(udev);
    42			usb_set_intfdata(usb_intf, NULL);
    43			complete(&adapter->rtl8712_fw_ready);
    44			return;
    45		}
    46		adapter->fw = firmware;
    47		/* firmware available - start netdev */
    48		if (register_netdev(adapter->pnetdev) != 0)
  > 49			dev_err(&udev->dev, "r8712u: register_netdev() failed\n");
    50		complete(&adapter->rtl8712_fw_ready);
    51	}
    52	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66253 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] staging: rtl8712: check register_netdev() return value
  2020-12-09 15:01 shaojie.dong
  2020-12-09 15:13 ` Greg KH
@ 2020-12-09 17:46 ` Dan Carpenter
  1 sibling, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2020-12-09 17:46 UTC (permalink / raw)
  To: shaojie.dong
  Cc: Larry.Finger, florian.c.schilhabel, gregkh, devel, linux-kernel

On Wed, Dec 09, 2020 at 11:01:24PM +0800, shaojie.dong@isrc.iscas.ac.cn wrote:
> From: "shaojie.dong" <shaojie.dong@isrc.iscas.ac.cn>
> 
> Function register_netdev() can fail, so we should check it's return value
> 
> Signed-off-by: shaojie.dong <shaojie.dong@isrc.iscas.ac.cn>
> ---
>  drivers/staging/rtl8712/hal_init.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c
> index 715f1fe8b..38a3e3d44 100644
> --- a/drivers/staging/rtl8712/hal_init.c
> +++ b/drivers/staging/rtl8712/hal_init.c
> @@ -45,7 +45,10 @@ static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context)
>  	}
>  	adapter->fw = firmware;
>  	/* firmware available - start netdev */
> -	register_netdev(adapter->pnetdev);
> +	if (register_netdev(adapter->pnetdev) != 0) {
> +		netdev_err(adapter->pnetdev, "register_netdev() failed\n");
> +		free_netdev(adapter->pnetdev);
> +	}

This function should not be calling register_netdev().  What does that
have to do with firmware?  It should also not free_netdev() because
that will just lead to a use after free in the caller.

regards,
dan carpenter

>  	complete(&adapter->rtl8712_fw_ready);
>  }
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] staging: rtl8712: check register_netdev() return value
  2020-12-09 15:01 shaojie.dong
@ 2020-12-09 15:13 ` Greg KH
  2020-12-09 17:46 ` Dan Carpenter
  1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2020-12-09 15:13 UTC (permalink / raw)
  To: shaojie.dong; +Cc: Larry.Finger, florian.c.schilhabel, devel, linux-kernel

On Wed, Dec 09, 2020 at 11:01:24PM +0800, shaojie.dong@isrc.iscas.ac.cn wrote:
> From: "shaojie.dong" <shaojie.dong@isrc.iscas.ac.cn>
> 
> Function register_netdev() can fail, so we should check it's return value
> 
> Signed-off-by: shaojie.dong <shaojie.dong@isrc.iscas.ac.cn>

I doubt you sign your name with a '.' in it, right?

Please resend with the correct name, and using Capital letters where
needed.

> ---
>  drivers/staging/rtl8712/hal_init.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c
> index 715f1fe8b..38a3e3d44 100644
> --- a/drivers/staging/rtl8712/hal_init.c
> +++ b/drivers/staging/rtl8712/hal_init.c
> @@ -45,7 +45,10 @@ static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context)
>  	}
>  	adapter->fw = firmware;
>  	/* firmware available - start netdev */
> -	register_netdev(adapter->pnetdev);
> +	if (register_netdev(adapter->pnetdev) != 0) {
> +		netdev_err(adapter->pnetdev, "register_netdev() failed\n");
> +		free_netdev(adapter->pnetdev);
> +	}

Did you test this to see if this really properly cleans everything up?

And your if statement can be simplified, please do so.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] staging: rtl8712: check register_netdev() return value
@ 2020-12-09 15:01 shaojie.dong
  2020-12-09 15:13 ` Greg KH
  2020-12-09 17:46 ` Dan Carpenter
  0 siblings, 2 replies; 6+ messages in thread
From: shaojie.dong @ 2020-12-09 15:01 UTC (permalink / raw)
  To: Larry.Finger, florian.c.schilhabel, gregkh
  Cc: devel, linux-kernel, shaojie.dong

From: "shaojie.dong" <shaojie.dong@isrc.iscas.ac.cn>

Function register_netdev() can fail, so we should check it's return value

Signed-off-by: shaojie.dong <shaojie.dong@isrc.iscas.ac.cn>
---
 drivers/staging/rtl8712/hal_init.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c
index 715f1fe8b..38a3e3d44 100644
--- a/drivers/staging/rtl8712/hal_init.c
+++ b/drivers/staging/rtl8712/hal_init.c
@@ -45,7 +45,10 @@ static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context)
 	}
 	adapter->fw = firmware;
 	/* firmware available - start netdev */
-	register_netdev(adapter->pnetdev);
+	if (register_netdev(adapter->pnetdev) != 0) {
+		netdev_err(adapter->pnetdev, "register_netdev() failed\n");
+		free_netdev(adapter->pnetdev);
+	}
 	complete(&adapter->rtl8712_fw_ready);
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-09 17:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-06 15:59 [PATCH] staging: rtl8712: check register_netdev() return value shaojie.dong
2020-12-06 16:03 ` Greg KH
2020-12-06 17:10 ` kernel test robot
2020-12-09 15:01 shaojie.dong
2020-12-09 15:13 ` Greg KH
2020-12-09 17:46 ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).