| Message ID | 20211111140007.7244-1-linma@zju.edu.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v0] hamradio: delete unnecessary free_netdev() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On Thu, 11 Nov 2021 22:00:07 +0800 Lin Ma wrote: > The former patch "defer 6pack kfree after unregister_netdev" adds > free_netdev() function in sixpack_close(), which is a bad copy from the > similar code in mkiss_close(). However, this free is unnecessary as the > flag needs_free_netdev is set to true in sp_setup(), hence the > unregister_netdev() will free the netdev automatically. > > Signed-off-by: Lin Ma <linma@zju.edu.cn> > --- > drivers/net/hamradio/6pack.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c > index bfdf89e54752..180c8f08169b 100644 > --- a/drivers/net/hamradio/6pack.c > +++ b/drivers/net/hamradio/6pack.c > @@ -677,8 +677,6 @@ static void sixpack_close(struct tty_struct *tty) > /* Free all 6pack frame buffers after unreg. */ > kfree(sp->rbuff); > kfree(sp->xbuff); > - > - free_netdev(sp->dev); sp is netdev_priv() tho, so this is now a UAF. I'd go for removing the needs_free_netdev = true instead. > } > > /* Perform I/O control on an active 6pack channel. */
Hi Jakub > On Thu, 11 Nov 2021 22:00:07 +0800 Lin Ma wrote: > > The former patch "defer 6pack kfree after unregister_netdev" adds > > free_netdev() function in sixpack_close(), which is a bad copy from the > > similar code in mkiss_close(). However, this free is unnecessary as the > > flag needs_free_netdev is set to true in sp_setup(), hence the > > unregister_netdev() will free the netdev automatically. > > > > Signed-off-by: Lin Ma <linma@zju.edu.cn> > > --- > > drivers/net/hamradio/6pack.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c > > index bfdf89e54752..180c8f08169b 100644 > > --- a/drivers/net/hamradio/6pack.c > > +++ b/drivers/net/hamradio/6pack.c > > @@ -677,8 +677,6 @@ static void sixpack_close(struct tty_struct *tty) > > /* Free all 6pack frame buffers after unreg. */ > > kfree(sp->rbuff); > > kfree(sp->xbuff); > > - > > - free_netdev(sp->dev); > > sp is netdev_priv() tho, so this is now a UAF. I'd go for removing the > needs_free_netdev = true instead. > > > } > > > > /* Perform I/O control on an active 6pack channel. */ Okay, that make senses, wait a minute. Thanks Lin
diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c index bfdf89e54752..180c8f08169b 100644 --- a/drivers/net/hamradio/6pack.c +++ b/drivers/net/hamradio/6pack.c @@ -677,8 +677,6 @@ static void sixpack_close(struct tty_struct *tty) /* Free all 6pack frame buffers after unreg. */ kfree(sp->rbuff); kfree(sp->xbuff); - - free_netdev(sp->dev); } /* Perform I/O control on an active 6pack channel. */
The former patch "defer 6pack kfree after unregister_netdev" adds free_netdev() function in sixpack_close(), which is a bad copy from the similar code in mkiss_close(). However, this free is unnecessary as the flag needs_free_netdev is set to true in sp_setup(), hence the unregister_netdev() will free the netdev automatically. Signed-off-by: Lin Ma <linma@zju.edu.cn> --- drivers/net/hamradio/6pack.c | 2 -- 1 file changed, 2 deletions(-)