Commit e85539aa authored by Tengda Wu's avatar Tengda Wu
Browse files

tracing: Avoid use-after-free in tracing_open_file_tr() 

hulk inclusion
category: bugfix
bugzilla: https://gitee.com/openeuler/kernel/issues/IBRD5T



--------------------------------

There is a race condition in the tracing_open_file_tr() function when
obtaining `inode->i_private`. The function __trace_remove_event_dirs()
recursively deletes and releases `trace_event_file` (which is
`inode->i_private`) and the function tracing_open_file_tr() might be
concurrently reading and using it, leading to use-after-free.

[instance_rmdir]                        [event_hist_open]

event_trace_del_tracer
tracepoint_synchronize_unregister
                                        tracing_open_file_tr
                                        file = inode->i_private
__trace_remove_event_dirs
event_put_file(file)
                                        tracing_check_open_get_tr(file->tr)

Fix this by explicitly setting `inode->i_private` to NULL when deleting
those files which ops depend on i_private to obtain trace_event_file, and
moving `inode->i_private` inside the event_mutex lock when opening those
kind of files.

Fixes: 321a6c77 ("tracing: Have trace_event_file have ref counters")
Signed-off-by: default avatarTengda Wu <wutengda2@huawei.com>
parent a7973281

kernel/trace/trace.c

+13 −6
Original line number Diff line number Diff line
@@ -4508,14 +4508,21 @@ int tracing_open_generic_tr(struct inode *inode, struct file *filp) 
 */

int tracing_open_file_tr(struct inode *inode, struct file *filp)

{

	struct trace_event_file *file = inode->i_private;

	struct trace_event_file *file;

	int ret;



	mutex_lock(&event_mutex);

	file = inode->i_private;

	if (!file) {

		mutex_unlock(&event_mutex);

		return -ENODEV;

	}



	ret = tracing_check_open_get_tr(file->tr);

	if (ret)

	if (ret) {

		mutex_unlock(&event_mutex);

		return ret;



	mutex_lock(&event_mutex);

	}



	/* Fail if the file is marked for removal */

	if (file->flags & EVENT_FILE_FL_FREED) {
@@ -4529,14 +4536,14 @@ int tracing_open_file_tr(struct inode *inode, struct file *filp) 
	if (ret)

		return ret;



	filp->private_data = inode->i_private;

	filp->private_data = file;



	return 0;

}



int tracing_release_file_tr(struct inode *inode, struct file *filp)

{

	struct trace_event_file *file = inode->i_private;

	struct trace_event_file *file = filp->private_data;



	trace_array_put(file->tr);

	event_file_put(file);

kernel/trace/trace_events.c

+32 −0
Original line number Diff line number Diff line
@@ -764,9 +764,41 @@ void event_file_put(struct trace_event_file *file) 
	}

}



static bool file_op_depends_on_i_priv(const struct dentry *dentry)

{

	const struct file_operations *fop = NULL;



	if (!d_really_is_positive(dentry) || !d_inode(dentry)->i_fop)

		return false;



	fop = d_inode(dentry)->i_fop;

	if (fop->open == tracing_open_file_tr)

		return true;

#ifdef CONFIG_HIST_TRIGGERS

	if (fop == &event_hist_fops)

		return true;

#endif

#ifdef CONFIG_HIST_TRIGGERS_DEBUG

	if (fop == &event_hist_debug_fops)

		return true;

#endif



	return false;

}



static void remove_event_file_dir(struct trace_event_file *file)

{

	struct dentry *dir = file->dir;

	struct dentry *child;



	if (dir) {

		spin_lock(&dir->d_lock);	/* probably unneeded */

		list_for_each_entry(child, &dir->d_subdirs, d_child) {

			if (file_op_depends_on_i_priv(child))

				d_inode(child)->i_private = NULL;

		}

		spin_unlock(&dir->d_lock);

	}



	tracefs_remove(dir);