| Message ID | 20211217021356.27322-1-linma@zju.edu.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b2f37aead1b82a770c48b5d583f35ec22aabb61e |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v0] hamradio: improve the incomplete fix to avoid NPD | expand |
Hello: This patch was applied to netdev/net.git (master) by David S. Miller <davem@davemloft.net>: On Fri, 17 Dec 2021 10:13:56 +0800 you wrote: > The previous commit 3e0588c291d6 ("hamradio: defer ax25 kfree after > unregister_netdev") reorder the kfree operations and unregister_netdev > operation to prevent UAF. > > This commit improves the previous one by also deferring the nullify of > the ax->tty pointer. Otherwise, a NULL pointer dereference bug occurs. > Partial of the stack trace is shown below. > > [...] Here is the summary with links: - [v0] hamradio: improve the incomplete fix to avoid NPD https://git.kernel.org/netdev/net/c/b2f37aead1b8 You are awesome, thank you!
diff --git a/drivers/net/hamradio/mkiss.c b/drivers/net/hamradio/mkiss.c index 7da2bb8a443c..edde9c3ae12b 100644 --- a/drivers/net/hamradio/mkiss.c +++ b/drivers/net/hamradio/mkiss.c @@ -794,14 +794,14 @@ static void mkiss_close(struct tty_struct *tty) */ netif_stop_queue(ax->dev); - ax->tty = NULL; - unregister_netdev(ax->dev); /* Free all AX25 frame buffers after unreg. */ kfree(ax->rbuff); kfree(ax->xbuff); + ax->tty = NULL; + free_netdev(ax->dev); }
The previous commit 3e0588c291d6 ("hamradio: defer ax25 kfree after unregister_netdev") reorder the kfree operations and unregister_netdev operation to prevent UAF. This commit improves the previous one by also deferring the nullify of the ax->tty pointer. Otherwise, a NULL pointer dereference bug occurs. Partial of the stack trace is shown below. BUG: kernel NULL pointer dereference, address: 0000000000000538 RIP: 0010:ax_xmit+0x1f9/0x400 ... Call Trace: dev_hard_start_xmit+0xec/0x320 sch_direct_xmit+0xea/0x240 __qdisc_run+0x166/0x5c0 __dev_queue_xmit+0x2c7/0xaf0 ax25_std_establish_data_link+0x59/0x60 ax25_connect+0x3a0/0x500 ? security_socket_connect+0x2b/0x40 __sys_connect+0x96/0xc0 ? __hrtimer_init+0xc0/0xc0 ? common_nsleep+0x2e/0x50 ? switch_fpu_return+0x139/0x1a0 __x64_sys_connect+0x11/0x20 do_syscall_64+0x33/0x40 entry_SYSCALL_64_after_hwframe+0x44/0xa9 The crash point is shown as below static void ax_encaps(...) { ... set_bit(TTY_DO_WRITE_WAKEUP, &ax->tty->flags); // ax->tty = NULL! ... } By placing the nullify action after the unregister_netdev, the ax->tty pointer won't be assigned as NULL net_device framework layer is well synchronized. Signed-off-by: Lin Ma <linma@zju.edu.cn> --- drivers/net/hamradio/mkiss.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)