eventfs/tracing: Add callback for release of an eventfs_inode

Synthetic events create and destroy tracefs files when they are created and removed. The tracing subsystem has its own file descriptor representing the state of the events attached to the tracefs files. There's a race between the eventfs files and this file descriptor of the tracing system where the following can cause an issue: With two scripts 'A' and 'B' doing: Script 'A': echo "hello int aaa" > /sys/kernel/tracing/synthetic_events while : do echo 0 > /sys/kernel/tracing/events/synthetic/hello/enable done Script 'B': echo > /sys/kernel/tracing/synthetic_events Script 'A' creates a synthetic event "hello" and then just writes zero into its enable file. Script 'B' removes all synthetic events (including the newly created "hello" event). What happens is that the opening of the "enable" file has: { struct trace_event_file *file = inode->i_private; int ret; ret = tracing_check_open_get_tr(file->tr); [..] But deleting the events frees the "file" descriptor, and a "use after free" happens with the dereference at "file->tr". The file descriptor does have a reference counter, but there needs to be a way to decrement it from the eventfs when the eventfs_inode is removed that represents this file descriptor. Add an optional "release" callback to the eventfs_entry array structure, that gets called when the eventfs file is about to be removed. This allows for the creating on the eventfs file to increment the tracing file descriptor ref counter. When the eventfs file is deleted, it can call the release function that will call the put function for the tracing file descriptor. This will protect the tracing file from being freed while a eventfs file that references it is being opened. Link: https://lore.kernel.org/linux-trace-kernel/20240426073410.17154-1-Tze-nan.Wu@mediatek.com/ Link: https://lore.kernel.org/linux-trace-kernel/20240502090315.448cba46@gandalf.local.home

Steven Rostedt (Google) committed 2 years ago

Commit b63db581 parent e67572c3 files · +36 −2Browse files

Unified · Split

3 files

fs/tracefs/event_inode.c +21 −2
include/linux/tracefs.h +3 −0
kernel/trace/trace_events.c +12 −0

fs/tracefs/event_inode.c+21−2

@@ -84,10 +84,17 @@ enum { · 83 unmodified lines
84	84	static void release_ei(struct kref *ref)
85	85	{
86	86	struct eventfs_inode *ei = container_of(ref, struct eventfs_inode, kref);
	87	+ const struct eventfs_entry *entry;
87	88	struct eventfs_root_inode *rei;
88	89
89	90	WARN_ON_ONCE(!ei->is_freed);
90	91
	92	+ for (int i = 0; i < ei->nr_entries; i++) {
	93	+ entry = &ei->entries[i];
	94	+ if (entry->release)
	95	+ entry->release(entry->name, ei->data);
	96	+ }
	97	+
91	98	kfree(ei->entry_attrs);
92	99	kfree_const(ei->name);
93	100	if (ei->is_events) {
@@ -112,6 +119,18 @@ static inline void free_ei(struct eventfs_inode *ei) · 18 unmodified lines
112	119	}
113	120	}
114	121
	122	+/*
	123	+ * Called when creation of an ei fails, do not call release() functions.
	124	+ */
	125	+static inline void cleanup_ei(struct eventfs_inode *ei)
	126	+{
	127	+ if (ei) {
	128	+ /* Set nr_entries to 0 to prevent release() function being called */
	129	+ ei->nr_entries = 0;
	130	+ free_ei(ei);
	131	+ }
	132	+}
	133	+
115	134	static inline struct eventfs_inode get_ei(struct eventfs_inode ei)
116	135	{
117	136	if (ei)
@@ -734,7 +753,7 @@ struct eventfs_inode eventfs_create_dir(const char name, struct eventfs_inode · 616 unmodified lines
734	753
735	754	/* Was the parent freed? */
736	755	if (list_empty(&ei->list)) {
737		− free_ei(ei);
	756	+ cleanup_ei(ei);
738	757	ei = NULL;
739	758	}
740	759	return ei;
@@ -835,7 +854,7 @@ struct eventfs_inode eventfs_create_events_dir(const char name, struct dentry · 94 unmodified lines
835	854	return ei;
836	855
837	856	fail:
838		− free_ei(ei);
	857	+ cleanup_ei(ei);
839	858	tracefs_failed_creating(dentry);
840	859	return ERR_PTR(-ENOMEM);
841	860	}

include/linux/tracefs.h+3−0

@@ -62,6 +62,8 @@ struct eventfs_file; · 61 unmodified lines
62	62	typedef int (eventfs_callback)(const char name, umode_t mode, void *data,
63	63	const struct file_operations **fops);
64	64
	65	+typedef void (eventfs_release)(const char name, void *data);
	66	+
65	67	/**
66	68	* struct eventfs_entry - dynamically created eventfs file call back handler
67	69	* @name: Then name of the dynamic file in an eventfs directory
@@ -72,6 +74,7 @@ typedef int (eventfs_callback)(const char name, umode_t mode, void *data, · 4 unmodified lines
72	74	struct eventfs_entry {
73	75	const char *name;
74	76	eventfs_callback callback;
	77	+ eventfs_release release;
75	78	};
76	79
77	80	struct eventfs_inode;

kernel/trace/trace_events.c+12−0

@@ -2552,6 +2552,14 @@ static int event_callback(const char name, umode_t mode, void **data, · 2551 unmodified lines
2552	2552	return 0;
2553	2553	}
2554	2554
	2555	+/* The file is incremented on creation and freeing the enable file decrements it */
	2556	+static void event_release(const char name, void data)
	2557	+{
	2558	+ struct trace_event_file *file = data;
	2559	+
	2560	+ event_file_put(file);
	2561	+}
	2562	+
2555	2563	static int
2556	2564	event_create_dir(struct eventfs_inode parent, struct trace_event_file file)
2557	2565	{
@@ -2566,6 +2574,7 @@ event_create_dir(struct eventfs_inode parent, struct trace_event_file file) · 8 unmodified lines
2566	2574	{
2567	2575	.name = "enable",
2568	2576	.callback = event_callback,
	2577	+ .release = event_release,
2569	2578	},
2570	2579	{
2571	2580	.name = "filter",
@@ -2634,6 +2643,9 @@ event_create_dir(struct eventfs_inode parent, struct trace_event_file file) · 62 unmodified lines
2634	2643	return ret;
2635	2644	}
2636	2645
	2646	+ /* Gets decremented on freeing of the "enable" file */
	2647	+ event_file_get(file);
	2648	+
2637	2649	return 0;
2638	2650	}
2639	2651