tracing: Fix uaf issue when open the hist or hist_debug file
Commit Message
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(-)
Comments
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
>
@@ -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