[PATCH] staging: rtl8712: check register_netdev() return value

[shaojie.dong at isrc.iscas.ac.cn]

Hi

> 
> 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.
>

--> check code history author<larry.finger at lwfinger.net> changed synchronous firmware loading to asynchronous firmware loading
    before this change, register_netdev() was not calling in firmware related function.
    For asynchronous loading, maybe register_netdev() be calling in rtl871x_load_fw_cb() is to ensure the netdev be registered after firmware loading completed

--> for potential use after free issue
    Could I only call "free_irq(adapter->pnetdev->irq, adapter->pnetdev)" when register_netdev() failed ?
    If no need to change drivers/staging/rtl8712/hal_init.c file, I could give up my patch, thank you !

> -----原始邮件-----
> 发件人: "Dan Carpenter" <dan.carpenter at oracle.com>
> 发送时间: 2020-12-10 01:46:15 (星期四)
> 收件人: shaojie.dong at isrc.iscas.ac.cn
> 抄送: Larry.Finger at lwfinger.net, florian.c.schilhabel at googlemail.com, gregkh at linuxfoundation.org, devel at driverdev.osuosl.org, linux-kernel at vger.kernel.org
> 主题: Re: [PATCH] staging: rtl8712: check register_netdev() return value
> 
> On Wed, Dec 09, 2020 at 11:01:24PM +0800, shaojie.dong at isrc.iscas.ac.cn wrote:
> > From: "shaojie.dong" <shaojie.dong at isrc.iscas.ac.cn>
> > 
> > Function register_netdev() can fail, so we should check it's return value
> > 
> > Signed-off-by: shaojie.dong <shaojie.dong at 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 at linuxdriverproject.org
> > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
</shaojie.dong at isrc.iscas.ac.cn></shaojie.dong at isrc.iscas.ac.cn></dan.carpenter at oracle.com></larry.finger at lwfinger.net>