Skip to content

Commit efe4186

Browse files
stonezdmdavem330
authored andcommitted
drivers: hamradio: 6pack: fix UAF bug caused by mod_timer()
When a 6pack device is detaching, the sixpack_close() will act to cleanup necessary resources. Although del_timer_sync() in sixpack_close() won't return if there is an active timer, one could use mod_timer() in sp_xmit_on_air() to wake up timer again by calling userspace syscall such as ax25_sendmsg(), ax25_connect() and ax25_ioctl(). This unexpected waked handler, sp_xmit_on_air(), realizes nothing about the undergoing cleanup and may still call pty_write() to use driver layer resources that have already been released. One of the possible race conditions is shown below: (USE) | (FREE) ax25_sendmsg() | ax25_queue_xmit() | ... | sp_xmit() | sp_encaps() | sixpack_close() sp_xmit_on_air() | del_timer_sync(&sp->tx_t) mod_timer(&sp->tx_t,...) | ... | unregister_netdev() | ... (wait a while) | tty_release() | tty_release_struct() | release_tty() sp_xmit_on_air() | tty_kref_put(tty_struct) //FREE pty_write(tty_struct) //USE | ... The corresponding fail log is shown below: =============================================================== BUG: KASAN: use-after-free in __run_timers.part.0+0x170/0x470 Write of size 8 at addr ffff88800a652ab8 by task swapper/2/0 ... Call Trace: ... queue_work_on+0x3f/0x50 pty_write+0xcd/0xe0pty_write+0xcd/0xe0 sp_xmit_on_air+0xb2/0x1f0 call_timer_fn+0x28/0x150 __run_timers.part.0+0x3c2/0x470 run_timer_softirq+0x3b/0x80 __do_softirq+0xf1/0x380 ... This patch reorders the del_timer_sync() after the unregister_netdev() to avoid UAF bugs. Because the unregister_netdev() is well synchronized, it flushs out any pending queues, waits the refcount of net_device decreases to zero and removes net_device from kernel. There is not any running routines after executing unregister_netdev(). Therefore, we could not arouse timer from userspace again. Signed-off-by: Duoming Zhou <[email protected]> Reviewed-by: Lin Ma <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 7a2fb91 commit efe4186

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

drivers/net/hamradio/6pack.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -668,11 +668,11 @@ static void sixpack_close(struct tty_struct *tty)
668668
*/
669669
netif_stop_queue(sp->dev);
670670

671+
unregister_netdev(sp->dev);
672+
671673
del_timer_sync(&sp->tx_t);
672674
del_timer_sync(&sp->resync_t);
673675

674-
unregister_netdev(sp->dev);
675-
676676
/* Free all 6pack frame buffers after unreg. */
677677
kfree(sp->rbuff);
678678
kfree(sp->xbuff);

0 commit comments

Comments
 (0)