| From b2f37aead1b82a770c48b5d583f35ec22aabb61e Mon Sep 17 00:00:00 2001 |
| From: Lin Ma <linma@zju.edu.cn> |
| Date: Fri, 17 Dec 2021 10:13:56 +0800 |
| Subject: hamradio: improve the incomplete fix to avoid NPD |
| |
| From: Lin Ma <linma@zju.edu.cn> |
| |
| commit b2f37aead1b82a770c48b5d583f35ec22aabb61e upstream. |
| |
| 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> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/net/hamradio/mkiss.c | 4 ++-- |
| 1 file changed, 2 insertions(+), 2 deletions(-) |
| |
| --- a/drivers/net/hamradio/mkiss.c |
| +++ b/drivers/net/hamradio/mkiss.c |
| @@ -803,14 +803,14 @@ static void mkiss_close(struct tty_struc |
| */ |
| 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); |
| } |
| |