[PATCH] staging: rtl8712: check register_netdev() return value
From: "shaojie.dong" Function register_netdev() can fail, so we should check it's return value Signed-off-by: shaojie.dong --- 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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8712: check register_netdev() return value
From: "shaojie.dong"
Function register_netdev() can fail, so we should check it's return value
Signed-off-by: shaojie.dong
---
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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
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 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"
> 发送时间: 2020-12-10 01:46:15 (星期四)
> 收件人: shaojie.d...@isrc.iscas.ac.cn
> 抄送: larry.fin...@lwfinger.net, florian.c.schilha...@googlemail.com,
gre...@linuxfoundation.org, de...@driverdev.osuosl.org,
linux-ker...@vger.kernel.org
> 主题: Re: [PATCH] staging: rtl8712: check register_netdev() return value
>
> On Wed, Dec 09, 2020 at 11:01:24PM +0800, shaojie.d...@isrc.iscas.ac.cn
wrote:
> > From: "shaojie.dong"
> >
> > Function register_netdev() can fail, so we should check it's return
value
> >
> > Signed-off-by: shaojie.dong
> > ---
> > 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
> > de...@linuxdriverproject.org
> >
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value
Hi
Thanks ! I will modify sign name correctly later
Sorry to say that I have no rtl8712 hardware, so that I could not test it.
From Dan Carpenter's email reply, "free_netdev(adapter->pnetdev)" function
may cause use after free issue
So that I reply email to ensure if this return value should be check or how to
handle this return value error
> -原始邮件-
> 发件人: "Greg KH"
> 发送时间: 2020-12-09 23:13:40 (星期三)
> 收件人: shaojie.d...@isrc.iscas.ac.cn
> 抄送: larry.fin...@lwfinger.net, florian.c.schilha...@googlemail.com,
de...@driverdev.osuosl.org, linux-ker...@vger.kernel.org
> 主题: Re: [PATCH] staging: rtl8712: check register_netdev() return value
>
> On Wed, Dec 09, 2020 at 11:01:24PM +0800, shaojie.d...@isrc.iscas.ac.cn
wrote:
> > From: "shaojie.dong"
> >
> > Function register_netdev() can fail, so we should check it's return
value
> >
> > Signed-off-by: shaojie.dong
>
> 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value
Hi I do not have rtl8712 hardware So that I would remain this code and give up my patch Thank you ! > -原始邮件- > 发件人: "Dan Carpenter" > 发送时间: 2020-12-10 23:16:31 (星期四) > 收件人: shaojie.d...@isrc.iscas.ac.cn > 抄送: larry.fin...@lwfinger.net, florian.c.schilha...@googlemail.com, gre...@linuxfoundation.org, de...@driverdev.osuosl.org, linux-ker...@vger.kernel.org > 主题: Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value > > On Thu, Dec 10, 2020 at 11:05:34PM +0800, shaojie.d...@isrc.iscas.ac.cn wrote: > > 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 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 ! > > > > Cleaning this up is a bit complicated and requires reworking the > firmware loading and it requires testing. I don't think you have the > hardware to actually test this driver? Probably, just leave this code > for another day. > > regards, > dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
