| From 3e0588c291d6ce225f2b891753ca41d45ba42469 Mon Sep 17 00:00:00 2001 |
| From: Lin Ma <linma@zju.edu.cn> |
| Date: Mon, 8 Nov 2021 18:37:21 +0800 |
| Subject: hamradio: defer ax25 kfree after unregister_netdev |
| |
| From: Lin Ma <linma@zju.edu.cn> |
| |
| commit 3e0588c291d6ce225f2b891753ca41d45ba42469 upstream. |
| |
| There is a possible race condition (use-after-free) like below |
| |
| (USE) | (FREE) |
| ax25_sendmsg | |
| ax25_queue_xmit | |
| dev_queue_xmit | |
| __dev_queue_xmit | |
| __dev_xmit_skb | |
| sch_direct_xmit | ... |
| xmit_one | |
| netdev_start_xmit | tty_ldisc_kill |
| __netdev_start_xmit | mkiss_close |
| ax_xmit | kfree |
| ax_encaps | |
| | |
| |
| Even though there are two synchronization primitives before the kfree: |
| 1. wait_for_completion(&ax->dead). This can prevent the race with |
| routines from mkiss_ioctl. However, it cannot stop the routine coming |
| from upper layer, i.e., the ax25_sendmsg. |
| |
| 2. netif_stop_queue(ax->dev). It seems that this line of code aims to |
| halt the transmit queue but it fails to stop the routine that already |
| being xmit. |
| |
| This patch reorder the kfree after the unregister_netdev to avoid the |
| possible UAF as the unregister_netdev() is well synchronized and won't |
| return if there is a running routine. |
| |
| 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 | 9 +++++---- |
| 1 file changed, 5 insertions(+), 4 deletions(-) |
| |
| --- a/drivers/net/hamradio/mkiss.c |
| +++ b/drivers/net/hamradio/mkiss.c |
| @@ -803,13 +803,14 @@ static void mkiss_close(struct tty_struc |
| */ |
| netif_stop_queue(ax->dev); |
| |
| - /* Free all AX25 frame buffers. */ |
| - kfree(ax->rbuff); |
| - kfree(ax->xbuff); |
| - |
| ax->tty = NULL; |
| |
| unregister_netdev(ax->dev); |
| + |
| + /* Free all AX25 frame buffers after unreg. */ |
| + kfree(ax->rbuff); |
| + kfree(ax->xbuff); |
| + |
| free_netdev(ax->dev); |
| } |
| |