| Message ID | 20231212113317.4159890-1-zhengyejian1@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tracing: Fix uaf issue when open the hist or hist_debug file | expand |
On Tue, 12 Dec 2023 19:33:17 +0800 Zheng Yejian <zhengyejian1@huawei.com> wrote: > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c > index 1abc07fba1b9..00447ea7dabd 100644 > --- a/kernel/trace/trace_events_hist.c > +++ b/kernel/trace/trace_events_hist.c > @@ -5623,10 +5623,12 @@ static int event_hist_open(struct inode *inode, struct file *file) > { > int ret; > > - ret = security_locked_down(LOCKDOWN_TRACEFS); > + ret = tracing_open_file_tr(inode, file); > if (ret) > return ret; > > + /* Clear private_data to avoid warning in single_open */ > + file->private_data = NULL; > return single_open(file, hist_show, file); > } > > @@ -5634,7 +5636,7 @@ const struct file_operations event_hist_fops = { > .open = event_hist_open, > .read = seq_read, > .llseek = seq_lseek, > - .release = single_release, > + .release = tracing_release_file_tr, single_release() still needs to be called. This can't simply be replaced with tracing_release_file_tr(). > }; > > #ifdef CONFIG_HIST_TRIGGERS_DEBUG > @@ -5900,10 +5902,12 @@ static int event_hist_debug_open(struct inode *inode, struct file *file) > { > int ret; > > - ret = security_locked_down(LOCKDOWN_TRACEFS); > + ret = tracing_open_file_tr(inode, file); > if (ret) > return ret; > > + /* Clear private_data to avoid warning in single_open */ > + file->private_data = NULL; > return single_open(file, hist_debug_show, file); > } > > @@ -5911,7 +5915,7 @@ const struct file_operations event_hist_debug_fops = { > .open = event_hist_debug_open, > .read = seq_read, > .llseek = seq_lseek, > - .release = single_release, > + .release = tracing_release_file_tr, Same here. This just causes a leak of the single resources. What needs to be done is to add a: tracing_single_release_file_tr() That does both: int tracing_single_release_file_tr(struct inode *inode, struct file *filp) { struct trace_event_file *file = inode->i_private; trace_array_put(file->tr); event_file_put(file); return single_release(inode, filp); } -- Steve > }; > #endif >
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 1abc07fba1b9..00447ea7dabd 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -5623,10 +5623,12 @@ static int event_hist_open(struct inode *inode, struct file *file) { int ret; - ret = security_locked_down(LOCKDOWN_TRACEFS); + ret = tracing_open_file_tr(inode, file); if (ret) return ret; + /* Clear private_data to avoid warning in single_open */ + file->private_data = NULL; return single_open(file, hist_show, file); } @@ -5634,7 +5636,7 @@ const struct file_operations event_hist_fops = { .open = event_hist_open, .read = seq_read, .llseek = seq_lseek, - .release = single_release, + .release = tracing_release_file_tr, }; #ifdef CONFIG_HIST_TRIGGERS_DEBUG @@ -5900,10 +5902,12 @@ static int event_hist_debug_open(struct inode *inode, struct file *file) { int ret; - ret = security_locked_down(LOCKDOWN_TRACEFS); + ret = tracing_open_file_tr(inode, file); if (ret) return ret; + /* Clear private_data to avoid warning in single_open */ + file->private_data = NULL; return single_open(file, hist_debug_show, file); } @@ -5911,7 +5915,7 @@ const struct file_operations event_hist_debug_fops = { .open = event_hist_debug_open, .read = seq_read, .llseek = seq_lseek, - .release = single_release, + .release = tracing_release_file_tr, }; #endif
KASAN report following issue. The root cause is when opening 'hist' file of an instance and accessing 'trace_event_file' in hist_show(), but 'trace_event_file' has been freed due to the instance being removed. 'hist_debug' file has the same problem. To fix it, use tracing_{open, release}_file_tr() as instead when open these two files. BUG: KASAN: slab-use-after-free in hist_show+0x11e0/0x1278 Read of size 8 at addr ffff242541e336b8 by task head/190 CPU: 4 PID: 190 Comm: head Not tainted 6.7.0-rc5-g26aff849438c #133 Hardware name: linux,dummy-virt (DT) Call trace: dump_backtrace+0x98/0xf8 show_stack+0x1c/0x30 dump_stack_lvl+0x44/0x58 print_report+0xf0/0x5a0 kasan_report+0x80/0xc0 __asan_report_load8_noabort+0x1c/0x28 hist_show+0x11e0/0x1278 seq_read_iter+0x344/0xd78 seq_read+0x128/0x1c0 vfs_read+0x198/0x6c8 ksys_read+0xf4/0x1e0 __arm64_sys_read+0x70/0xa8 invoke_syscall+0x70/0x260 el0_svc_common.constprop.0+0xb0/0x280 do_el0_svc+0x44/0x60 el0_svc+0x34/0x68 el0t_64_sync_handler+0xb8/0xc0 el0t_64_sync+0x168/0x170 Allocated by task 188: kasan_save_stack+0x28/0x50 kasan_set_track+0x28/0x38 kasan_save_alloc_info+0x20/0x30 __kasan_slab_alloc+0x6c/0x80 kmem_cache_alloc+0x15c/0x4a8 trace_create_new_event+0x84/0x348 __trace_add_new_event+0x18/0x88 event_trace_add_tracer+0xc4/0x1a0 trace_array_create_dir+0x6c/0x100 trace_array_create+0x2e8/0x568 instance_mkdir+0x48/0x80 tracefs_syscall_mkdir+0x90/0xe8 vfs_mkdir+0x3c4/0x610 do_mkdirat+0x144/0x200 __arm64_sys_mkdirat+0x8c/0xc0 invoke_syscall+0x70/0x260 el0_svc_common.constprop.0+0xb0/0x280 do_el0_svc+0x44/0x60 el0_svc+0x34/0x68 el0t_64_sync_handler+0xb8/0xc0 el0t_64_sync+0x168/0x170 Freed by task 191: kasan_save_stack+0x28/0x50 kasan_set_track+0x28/0x38 kasan_save_free_info+0x34/0x58 __kasan_slab_free+0xe4/0x158 kmem_cache_free+0x19c/0x508 event_file_put+0xa0/0x120 remove_event_file_dir+0x180/0x320 event_trace_del_tracer+0xb0/0x180 __remove_instance+0x224/0x508 instance_rmdir+0x44/0x78 tracefs_syscall_rmdir+0xbc/0x140 vfs_rmdir+0x1cc/0x4c8 do_rmdir+0x220/0x2b8 __arm64_sys_unlinkat+0xc0/0x100 invoke_syscall+0x70/0x260 el0_svc_common.constprop.0+0xb0/0x280 do_el0_svc+0x44/0x60 el0_svc+0x34/0x68 el0t_64_sync_handler+0xb8/0xc0 el0t_64_sync+0x168/0x170 Signed-off-by: Zheng Yejian <zhengyejian1@huawei.com> --- kernel/trace/trace_events_hist.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)