2021-12-17 02:14:08

by Lin Ma

[permalink] [raw]
Subject: [PATCH v0] hamradio: improve the incomplete fix to avoid NPD

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 <[email protected]>
---
drivers/net/hamradio/mkiss.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

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);
}

--
2.33.1



2021-12-18 12:40:18

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH v0] hamradio: improve the incomplete fix to avoid NPD

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <[email protected]>:

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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html