Messages in this thread Patch in this message | | | Date | Thu, 10 Dec 2020 23:05:34 +0800 (GMT+08:00) | From | shaojie.dong@isrc ... | Subject | Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value |
| |
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@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@oracle.com> > 发送时间: 2020-12-10 01:46:15 (星期四) > 收件人: shaojie.dong@isrc.iscas.ac.cn > 抄送: Larry.Finger@lwfinger.net, florian.c.schilhabel@googlemail.com, gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org > 主题: Re: [PATCH] staging: rtl8712: check register_netdev() return value > > 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 </shaojie.dong@isrc.iscas.ac.cn></shaojie.dong@isrc.iscas.ac.cn></dan.carpenter@oracle.com></larry.finger@lwfinger.net> | |