Commit 72d03091 authored by Tengda Wu's avatar Tengda Wu
Browse files

tracing: Fix illegal address access of trace_event_file in tracing_release_file_tr() 

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: default avatarTengda Wu <wutengda2@huawei.com>
parent e85539aa

kernel/trace/trace_events_hist.c

+54 −8
Original line number Diff line number Diff line
@@ -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