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@xxxxxxxxxx>
---
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);
}
/**
--
js
suse labs
