Re: Re: [PATCH] staging: rtl8712: check register_netdev() return value
From: shaojie . dong
Date: Thu Dec 10 2020 - 10:07:16 EST
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@xxxxxxxxxxxx> 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@xxxxxxxxxx>
> 发送时间: 2020-12-10 01:46:15 (星期四)
> 收件人: shaojie.dong@xxxxxxxxxxxxxxxx
> 抄送: Larry.Finger@xxxxxxxxxxxx, florian.c.schilhabel@xxxxxxxxxxxxxx, gregkh@xxxxxxxxxxxxxxxxxxx, devel@xxxxxxxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
> 主题: Re: [PATCH] staging: rtl8712: check register_netdev() return value
>
> On Wed, Dec 09, 2020 at 11:01:24PM +0800, shaojie.dong@xxxxxxxxxxxxxxxx wrote:
> > From: "shaojie.dong" <shaojie.dong@xxxxxxxxxxxxxxxx>
> >
> > Function register_netdev() can fail, so we should check it's return value
> >
> > Signed-off-by: shaojie.dong <shaojie.dong@xxxxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxxxxx
> > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
</shaojie.dong@xxxxxxxxxxxxxxxx></shaojie.dong@xxxxxxxxxxxxxxxx></dan.carpenter@xxxxxxxxxx></larry.finger@xxxxxxxxxxxx>