| Message ID | 20240924093519.767036-1-xialonglong@kylinos.cn |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] tty: n_gsm: Fix use-after-free in gsm_cleanup_mux | expand |
Cc Daniel. On 24. 09. 24, 11:35, Longlong Xia wrote: > BUG: KASAN: slab-use-after-free in gsm_cleanup_mux+0x7e5/0x820 [n_gsm] > Read of size 8 at addr ffff88814941c700 by task poc/3395 > > CPU: 0 UID: 0 PID: 3395 Comm: poc Not tainted 6.11.0+ #46 > Hardware name: VMware, Inc. VMware Virtual Platform/440BX > Desktop Reference Platform, BIOS 6.00 11/12/2020 > Call Trace: > <TASK> > dump_stack_lvl+0x6c/0x90 > print_report+0xce/0x610 > kasan_complete_mode_report_info+0x5d/0x1e0 > gsm_cleanup_mux+0x7e5/0x820 [n_gsm] > kasan_report+0xbd/0xf0 > gsm_cleanup_mux+0x7e5/0x820 [n_gsm] > __asan_report_load8_noabort+0x14/0x20 > gsm_cleanup_mux+0x7e5/0x820 [n_gsm] > __pfx_gsm_cleanup_mux+0x10/0x10 [n_gsm] > __rseq_handle_notify_resume+0x188/0xc50 > __kasan_check_write+0x14/0x20 > gsmld_ioctl+0x3c3/0x15b0 [n_gsm] > __kasan_check_write+0x14/0x20 > __pfx_gsmld_ioctl+0x10/0x10 [n_gsm] > do_syscall_64+0x88/0x160 > __kasan_check_write+0x14/0x20 > ldsem_down_read+0x94/0x4e0 > __pfx_ldsem_down_read+0x10/0x10 > __pfx___rseq_handle_notify_resume+0x10/0x10 > switch_fpu_return+0xed/0x200 > tty_ioctl+0x660/0x1260 Could you decode the above to line numbers using ./scripts/decode_stacktrace.sh? And then trim the unnecessary entries like: > __pfx___handle_mm_fault+0x10/0x10 > __pfx_tty_ioctl+0x10/0x10 > __count_memcg_events+0xf5/0x3d0 > fdget+0x2de/0x4f0 > __x64_sys_ioctl+0x132/0x1b0 > x64_sys_call+0x1205/0x20d0 > do_syscall_64+0x7c/0x160 > clear_bhb_loop+0x15/0x70 > entry_SYSCALL_64_after_hwframe+0x76/0x7e up to here. BTW do you use ORC or is this with unreliable FRAME_POINTERs? I am asking because the stack traces contain a full load of ballast. Like do_syscall_64() after tty_ioctl(). > Allocated by task 808: Also drop this: > kasan_save_stack+0x28/0x50 > kasan_save_track+0x14/0x30 > kasan_save_alloc_info+0x36/0x40 > __kasan_kmalloc+0xb1/0xc0 > __kmalloc_noprof+0x1f6/0x4b0 up to here ^^^. > gsm_data_alloc.constprop.0+0x2e/0x1a0 [n_gsm] > gsm_send+0x2f/0x5d0 [n_gsm] > gsm_queue+0x522/0x730 [n_gsm] > gsm1_receive+0x58b/0xb70 [n_gsm] > gsmld_receive_buf+0x173/0x2a0 [n_gsm] > tty_ldisc_receive_buf+0x115/0x1e0 > tty_port_default_receive_buf+0x66/0xa0 > flush_to_ldisc+0x1b0/0x7c0 > process_scheduled_works+0x2bc/0x10c0 > worker_thread+0x3d4/0x970 > kthread+0x2b6/0x390 > ret_from_fork+0x39/0x80 > ret_from_fork_asm+0x1a/0x30 > > Freed by task 3377: And here: > kasan_save_stack+0x28/0x50 > kasan_save_track+0x14/0x30 > kasan_save_free_info+0x3a/0x50 > __kasan_slab_free+0x54/0x70 ^^^ > kfree+0x126/0x420 > gsm_cleanup_mux+0x3ae/0x820 [n_gsm] > gsmld_ioctl+0x3c3/0x15b0 [n_gsm] > tty_ioctl+0x660/0x1260 this: > __x64_sys_ioctl+0x132/0x1b0 > x64_sys_call+0x1205/0x20d0 > do_syscall_64+0x7c/0x160 > entry_SYSCALL_64_after_hwframe+0x76/0x7e ^^^ > [Analysis] > gsm_msg on the tx_ctrl_list or tx_data_list of gsm_mux > can be freed by multi threads through ioctl,which leads > to the occurrence of uaf. Protect it by gsm tx lock. LGTM. But Daniel might have a different opinion... > Signed-off-by: Longlong Xia <xialonglong@kylinos.cn> > --- > drivers/tty/n_gsm.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c > index 5d37a0984916..1ed68a6aba4e 100644 > --- a/drivers/tty/n_gsm.c > +++ b/drivers/tty/n_gsm.c > @@ -3125,6 +3125,7 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc) > int i; > struct gsm_dlci *dlci; > struct gsm_msg *txq, *ntxq; > + unsigned long flags; > > gsm->dead = true; > mutex_lock(&gsm->mutex); > @@ -3157,12 +3158,15 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc) > mutex_unlock(&gsm->mutex); > /* Now wipe the queues */ > tty_ldisc_flush(gsm->tty); > + > + spin_lock_irqsave(&gsm->tx_lock, flags); Perhaps use guard(spinlock_irqsave) instead? > list_for_each_entry_safe(txq, ntxq, &gsm->tx_ctrl_list, list) > kfree(txq); > INIT_LIST_HEAD(&gsm->tx_ctrl_list); > list_for_each_entry_safe(txq, ntxq, &gsm->tx_data_list, list) > kfree(txq); > INIT_LIST_HEAD(&gsm->tx_data_list); > + spin_unlock_irqrestore(&gsm->tx_lock, flags); > } > > /**
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 5d37a0984916..1ed68a6aba4e 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -3125,6 +3125,7 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc) int i; struct gsm_dlci *dlci; struct gsm_msg *txq, *ntxq; + unsigned long flags; gsm->dead = true; mutex_lock(&gsm->mutex); @@ -3157,12 +3158,15 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc) mutex_unlock(&gsm->mutex); /* Now wipe the queues */ tty_ldisc_flush(gsm->tty); + + spin_lock_irqsave(&gsm->tx_lock, flags); list_for_each_entry_safe(txq, ntxq, &gsm->tx_ctrl_list, list) kfree(txq); INIT_LIST_HEAD(&gsm->tx_ctrl_list); list_for_each_entry_safe(txq, ntxq, &gsm->tx_data_list, list) kfree(txq); INIT_LIST_HEAD(&gsm->tx_data_list); + spin_unlock_irqrestore(&gsm->tx_lock, flags); } /**
BUG: KASAN: slab-use-after-free in gsm_cleanup_mux+0x7e5/0x820 [n_gsm] Read of size 8 at addr ffff88814941c700 by task poc/3395 CPU: 0 UID: 0 PID: 3395 Comm: poc Not tainted 6.11.0+ #46 Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 11/12/2020 Call Trace: <TASK> dump_stack_lvl+0x6c/0x90 print_report+0xce/0x610 kasan_complete_mode_report_info+0x5d/0x1e0 gsm_cleanup_mux+0x7e5/0x820 [n_gsm] kasan_report+0xbd/0xf0 gsm_cleanup_mux+0x7e5/0x820 [n_gsm] __asan_report_load8_noabort+0x14/0x20 gsm_cleanup_mux+0x7e5/0x820 [n_gsm] __pfx_gsm_cleanup_mux+0x10/0x10 [n_gsm] __rseq_handle_notify_resume+0x188/0xc50 __kasan_check_write+0x14/0x20 gsmld_ioctl+0x3c3/0x15b0 [n_gsm] __kasan_check_write+0x14/0x20 __pfx_gsmld_ioctl+0x10/0x10 [n_gsm] do_syscall_64+0x88/0x160 __kasan_check_write+0x14/0x20 ldsem_down_read+0x94/0x4e0 __pfx_ldsem_down_read+0x10/0x10 __pfx___rseq_handle_notify_resume+0x10/0x10 switch_fpu_return+0xed/0x200 tty_ioctl+0x660/0x1260 __pfx___handle_mm_fault+0x10/0x10 __pfx_tty_ioctl+0x10/0x10 __count_memcg_events+0xf5/0x3d0 fdget+0x2de/0x4f0 __x64_sys_ioctl+0x132/0x1b0 x64_sys_call+0x1205/0x20d0 do_syscall_64+0x7c/0x160 clear_bhb_loop+0x15/0x70 entry_SYSCALL_64_after_hwframe+0x76/0x7e Allocated by task 808: kasan_save_stack+0x28/0x50 kasan_save_track+0x14/0x30 kasan_save_alloc_info+0x36/0x40 __kasan_kmalloc+0xb1/0xc0 __kmalloc_noprof+0x1f6/0x4b0 gsm_data_alloc.constprop.0+0x2e/0x1a0 [n_gsm] gsm_send+0x2f/0x5d0 [n_gsm] gsm_queue+0x522/0x730 [n_gsm] gsm1_receive+0x58b/0xb70 [n_gsm] gsmld_receive_buf+0x173/0x2a0 [n_gsm] tty_ldisc_receive_buf+0x115/0x1e0 tty_port_default_receive_buf+0x66/0xa0 flush_to_ldisc+0x1b0/0x7c0 process_scheduled_works+0x2bc/0x10c0 worker_thread+0x3d4/0x970 kthread+0x2b6/0x390 ret_from_fork+0x39/0x80 ret_from_fork_asm+0x1a/0x30 Freed by task 3377: kasan_save_stack+0x28/0x50 kasan_save_track+0x14/0x30 kasan_save_free_info+0x3a/0x50 __kasan_slab_free+0x54/0x70 kfree+0x126/0x420 gsm_cleanup_mux+0x3ae/0x820 [n_gsm] gsmld_ioctl+0x3c3/0x15b0 [n_gsm] tty_ioctl+0x660/0x1260 __x64_sys_ioctl+0x132/0x1b0 x64_sys_call+0x1205/0x20d0 do_syscall_64+0x7c/0x160 entry_SYSCALL_64_after_hwframe+0x76/0x7e [Analysis] gsm_msg on the tx_ctrl_list or tx_data_list of gsm_mux can be freed by multi threads through ioctl,which leads to the occurrence of uaf. Protect it by gsm tx lock. Signed-off-by: Longlong Xia <xialonglong@kylinos.cn> --- drivers/tty/n_gsm.c | 4 ++++ 1 file changed, 4 insertions(+)