hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IBRD5T -------------------------------- In handling concurrency issues with instance_rmdir, the previous commit uses `file->private_data` to pass `trace_event_file` object between file open and close. However, this approach is ineffective for "hist" file because when event_hist_open() is called, `file->private_data` is reassigned to `seq_file`, causing the loss of `trace_event_file`. Consequently, during tracing_single_release_file_tr(), an error occurs where `seq_file` is mistakenly treated as `trace_event_file`, leading to illegal address access. To fix it, a new struct `hist_file_data` is introduced to hold both `struct file` and `struct trace_event_file`. During event_hist_open(), `hist_file_data` is stored in `seq_file->private`. During event_hist_release(), `trace_event_file` is retrieved from `seq_file->private` and released. Due to changes in `seq_file->private` data, adjustments need to be made at locations within "hist" that originally used `seq_file->private`. Fixes: 3e663e6829c9 ("tracing: Avoid use-after-free in tracing_open_file_tr()") Signed-off-by: Tengda Wu <wutengda2@huawei.com> --- kernel/trace/trace_events_hist.c | 62 +++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 920588dfdeb4..532e4dbc98e6 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -4794,15 +4794,21 @@ static void hist_trigger_show(struct seq_file *m, n_entries, (u64)atomic64_read(&hist_data->map->drops)); } +struct hist_file_data { + struct file *file; + struct trace_event_file *event_file; +}; + static int hist_show(struct seq_file *m, void *v) { + struct hist_file_data *hist_file = m->private; struct event_trigger_data *data; struct trace_event_file *event_file; int n = 0, ret = 0; mutex_lock(&event_mutex); - event_file = event_file_data(m->private); + event_file = event_file_data(hist_file->file); if (unlikely(!event_file)) { ret = -ENODEV; goto out_unlock; @@ -4819,24 +4825,50 @@ static int hist_show(struct seq_file *m, void *v) return ret; } +static int tracing_release_hist_file_tr(struct inode *inode, struct file *file) +{ + struct seq_file *m = file->private_data; + struct hist_file_data *hist_file = m->private; + struct trace_event_file *event_file = hist_file->event_file; + + trace_array_put(event_file->tr); + event_file_put(event_file); + kfree(hist_file); + + return single_release(inode, file); +} + static int event_hist_open(struct inode *inode, struct file *file) { + struct hist_file_data *hist_file; int ret; + hist_file = kzalloc(sizeof(*hist_file), GFP_KERNEL); + if (!hist_file) + return -ENOMEM; + ret = tracing_open_file_tr(inode, file); - if (ret) + if (ret) { + kfree(hist_file); return ret; + } + + hist_file->file = file; + hist_file->event_file = file->private_data; /* Clear private_data to avoid warning in single_open() */ file->private_data = NULL; - return single_open(file, hist_show, file); + ret = single_open(file, hist_show, hist_file); + if (ret) + kfree(hist_file); + return ret; } const struct file_operations event_hist_fops = { .open = event_hist_open, .read = seq_read, .llseek = seq_lseek, - .release = tracing_single_release_file_tr, + .release = tracing_release_hist_file_tr, }; #ifdef CONFIG_HIST_TRIGGERS_DEBUG @@ -5070,13 +5102,14 @@ static void hist_trigger_debug_show(struct seq_file *m, static int hist_debug_show(struct seq_file *m, void *v) { + struct hist_file_data *hist_file = m->private; struct event_trigger_data *data; struct trace_event_file *event_file; int n = 0, ret = 0; mutex_lock(&event_mutex); - event_file = event_file_data(m->private); + event_file = event_file_data(hist_file->file); if (unlikely(!event_file)) { ret = -ENODEV; goto out_unlock; @@ -5095,22 +5128,35 @@ static int hist_debug_show(struct seq_file *m, void *v) static int event_hist_debug_open(struct inode *inode, struct file *file) { + struct hist_file_data *hist_file; int ret; + hist_file = kzalloc(sizeof(*hist_file), GFP_KERNEL); + if (!hist_file) + return -ENOMEM; + ret = tracing_open_file_tr(inode, file); - if (ret) + if (ret) { + kfree(hist_file); return ret; + } + + hist_file->file = file; + hist_file->event_file = file->private_data; /* Clear private_data to avoid warning in single_open() */ file->private_data = NULL; - return single_open(file, hist_debug_show, file); + ret = single_open(file, hist_debug_show, hist_file); + if (ret) + kfree(hist_file); + return ret; } const struct file_operations event_hist_debug_fops = { .open = event_hist_debug_open, .read = seq_read, .llseek = seq_lseek, - .release = tracing_single_release_file_tr, + .release = tracing_release_hist_file_tr, }; #endif -- 2.34.1