user_events: Enable user processes to create and write to trace events

Minimal support for interacting with dynamic events, trace_event and ftrace. Core outline of flow between user process, ioctl and trace_event APIs.

Signed-off-by Beau Belgrave <[email protected]>

---
 kernel/trace/Kconfig             |   15 +
 kernel/trace/Makefile            |    1 +
 kernel/trace/trace_events_user.c | 1192 ++++++++++++++++++++++++++++++
 3 files changed, 1208 insertions(+)
 create mode 100644 kernel/trace/trace_events_user.c

kernel/trace/Kconfig +0 -0

index 420ff4bc67fd..21d00092436b 100644

kernel/trace/Kconfig +10 -0

+	help
+	  User trace events are user-defined trace events that
+	  can be used like an existing kernel trace event.  User trace
+	  events are generated by writing to a tracefs file.  User
+	  processes can determine if their tracing events should be
+	  generated by memory mapping a tracefs file and checking for
+	  an associated byte being non-zero.
+
+	  If in doubt, say N.
+
 config HIST_TRIGGERS
 	bool "Histogram triggers"
 	depends on ARCH_HAVE_NMI_SAFE_CMPXCHG

kernel/trace/Makefile +7 -0

index bedc5caceec7..19ef3758da95 100644
 
 
 
 
+/*
+ * Handles the final close of the file from user mode.
+ */
+static int user_events_release(struct inode *node, struct file *file)
+{
+	struct user_event_refs *refs;
+	struct user_event *user;

kernel/trace/Makefile +122 -0

+		user = refs->events[i];
+
+		if (user)
+			atomic_dec(&user->refcnt);
+	}
+
+	kfree_rcu(refs, rcu);
+out:
+	return 0;
+}
+
+static const struct file_operations user_data_fops = {
+	.write = user_events_write,
+	.write_iter = user_events_write_iter,
+	.unlocked_ioctl	= user_events_ioctl,
+	.release = user_events_release,
+};
+
+/*
+ * Maps the shared page into the user process for checking if event is enabled.
+ */
+static int user_status_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	unsigned long size = vma->vm_end - vma->vm_start;
+
+	if (size != MAX_EVENTS)
+		return -EINVAL;
+
+	return remap_pfn_range(vma, vma->vm_start,
+			       virt_to_phys(register_page_data) >> PAGE_SHIFT,
+			       size, vm_get_page_prot(VM_READ));
+}
+
+static int user_status_show(struct seq_file *m, void *p)
+{
+	struct user_event *user;
+	char status;
+	int i, active = 0, busy = 0, flags;
+
+	mutex_lock(&reg_mutex);
+
+	hash_for_each(register_table, i, user, node) {
+		status = register_page_data[user->index];
+		flags = user->flags;
+
+		seq_printf(m, "%d:%s", user->index, EVENT_NAME(user));
+
+		if (flags != 0 || status != 0)
+			seq_puts(m, " #");
+
+		if (status != 0) {
+			seq_puts(m, " Used by");
+			if (status & EVENT_STATUS_FTRACE)
+				seq_puts(m, " ftrace");
+			if (status & EVENT_STATUS_PERF)
+				seq_puts(m, " perf");
+			if (status & EVENT_STATUS_OTHER)
+				seq_puts(m, " other");
+			busy++;
+		}
+
+		if (flags & FLAG_BPF_ITER)
+			seq_puts(m, " FLAG:BPF_ITER");
+
+		seq_puts(m, "\n");
+		active++;
+	}
+
+	mutex_unlock(&reg_mutex);
+
+	seq_puts(m, "\n");
+	seq_printf(m, "Active: %d\n", active);
+	seq_printf(m, "Busy: %d\n", busy);
+	seq_printf(m, "Max: %ld\n", MAX_EVENTS);
+
+	return 0;
+}
+
+static ssize_t user_status_read(struct file *file, char __user *ubuf,
+				size_t count, loff_t *ppos)
+{
+	/*
+	 * Delay allocation of seq data until requested, most callers
+	 * will never read the status file. They will only mmap.
+	 */
+	if (file->private_data == NULL) {
+		int ret;
+
+		if (*ppos != 0)
+			return -EINVAL;
+
+		ret = single_open(file, user_status_show, NULL);
+
+		if (ret)
+			return ret;
+	}
+
+	return seq_read(file, ubuf, count, ppos);
+}
+
+static loff_t user_status_seek(struct file *file, loff_t offset, int whence)
+{
+	if (file->private_data == NULL)
+		return 0;
+
+	return seq_lseek(file, offset, whence);
+}
+
+static int user_status_release(struct inode *node, struct file *file)
+{
+	if (file->private_data == NULL)
+		return 0;
+
+	return single_release(node, file);
+}
+
+static const struct file_operations user_status_fops = {
+	.mmap = user_status_mmap,
+	.read = user_status_read,
+	.llseek  = user_status_seek,
+	.release = user_status_release,
+};

kernel/trace/Makefile +66 -0

+
+	if (!edata) {
+		pr_warn("Could not create tracefs 'user_events_data' entry\n");
+		goto err;
+	}
+
+	/* mmap with MAP_SHARED requires writable fd */
+	emmap = tracefs_create_file("user_events_status", 0644, NULL,
+				    NULL, &user_status_fops);
+
+	if (!emmap) {
+		tracefs_remove(edata);
+		pr_warn("Could not create tracefs 'user_events_mmap' entry\n");
+		goto err;
+	}
+
+	return 0;
+err:
+	return -ENODEV;
+}
+
+static void set_page_reservations(bool set)
+{
+	int page;
+
+	for (page = 0; page < MAX_PAGES; ++page) {
+		void *addr = register_page_data + (PAGE_SIZE * page);
+
+		if (set)
+			SetPageReserved(virt_to_page(addr));
+		else
+			ClearPageReserved(virt_to_page(addr));
+	}
+}
+
+static int __init trace_events_user_init(void)
+{
+	int ret;
+
+	/* Zero all bits beside 0 (which is reserved for failures) */
+	bitmap_zero(page_bitmap, MAX_EVENTS);
+	set_bit(0, page_bitmap);
+
+	register_page_data = kzalloc(MAX_EVENTS, GFP_KERNEL);
+
+	if (!register_page_data)
+		return -ENOMEM;
+
+	set_page_reservations(true);
+
+	ret = create_user_tracefs();
+
+	if (ret) {
+		pr_warn("user_events could not register with tracefs\n");
+		set_page_reservations(false);
+		kfree(register_page_data);
+		return ret;
+	}
+
+	if (dyn_event_register(&user_event_dops))
+		pr_warn("user_events could not register with dyn_events\n");
+
+	return 0;
+}
+
+fs_initcall(trace_events_user_init);

Quoted 17 lines · On Wed, 1 Dec 2021 10:25:04 -0800 Beau Belgrave <[email protected]>…

Minimal support for interacting with dynamic events, trace_event and ftrace. Core outline of flow between user process, ioctl and trace_event APIs.

Signed-off-by Beau Belgrave <[email protected]>

---
 kernel/trace/Kconfig             |   15 +
 kernel/trace/Makefile            |    1 +
 kernel/trace/trace_events_user.c | 1192 ++++++++++++++++++++++++++++++
 3 files changed, 1208 insertions(+)
 create mode 100644 kernel/trace/trace_events_user.c

kernel/trace/Kconfig +0 -0

index 420ff4bc67fd..21d00092436b 100644

kernel/trace/Kconfig config SYNTH_EVENTS · line 731

Quoted 8 lines

kernel/trace/Kconfig +5 -0

@@ -724,6 +724,21 @@ config SYNTH_EVENTS
 
 	  If in doubt, say N.
 
+config USER_EVENTS
+	bool "User trace events"
+	select TRACING
+	select DYNAMIC_EVENTS
+	default n

default n is default, so you do not need to explicitly state that.

In other words, the above line is a nop.

Quoted 32 lines

kernel/trace/Kconfig +10 -0

+	help
+	  User trace events are user-defined trace events that
+	  can be used like an existing kernel trace event.  User trace
+	  events are generated by writing to a tracefs file.  User
+	  processes can determine if their tracing events should be
+	  generated by memory mapping a tracefs file and checking for
+	  an associated byte being non-zero.
+
+	  If in doubt, say N.
+
 config HIST_TRIGGERS
 	bool "Histogram triggers"
 	depends on ARCH_HAVE_NMI_SAFE_CMPXCHG

kernel/trace/Makefile +11 -0

index bedc5caceec7..19ef3758da95 100644
 
 
 
 
+/*
+ * Handles the final close of the file from user mode.
+ */
+static int user_events_release(struct inode *node, struct file *file)
+{
+	struct user_event_refs *refs;
+	struct user_event *user;
+	int i;
+
+	/*
+	 * refs is protected by RCU and could in theory change immediately

kernel/trace/Makefile

Quoted 6 lines

kernel/trace/Makefile +6 -0

+	 * before this call on another core. To ensure we read the latest
+	 * version of refs we acquire the RCU read lock again.
+	 */
+	rcu_read_lock_sched();
+	refs = rcu_dereference_sched(file->private_data);
+	rcu_read_unlock_sched();

This still bothers me. Can another CPU call an ioctl here?

  user_events_ioctl_reg() {
    user_events_ref_add() {
      refs = rcu_dereference_protected(file->private_data, ..);
      new_refs = kzalloc(size, GFP_KERNEL);
      rcu_assign_pointer(file->private_data, new_refs);
      if (refs)
        kfree_rcu(refs, rcu);

refs now freed.

kernel/trace/Makefile

Quoted 11 lines

kernel/trace/Makefile +11 -0

+
+	if (!refs)
+		goto out;
+
+	/*
+	 * Do not need RCU while enumerating the events that were used.
+	 * The lifetime of refs has reached an end, it's tied to this file.
+	 * The underlying user_events are ref counted, and cannot be freed.
+	 * After this decrement, the user_events may be freed elsewhere.
+	 */
+	for (i = 0; i < refs->count; ++i) {

Fault on refs->count

??

Quoted 12 lines

kernel/trace/Makefile +11 -0

+		user = refs->events[i];
+
+		if (user)
+			atomic_dec(&user->refcnt);
+	}
+
+	kfree_rcu(refs, rcu);
+out:
+	return 0;
+}
+

Quoted 6 lines

kernel/trace/Makefile +6 -0

+static const struct file_operations user_status_fops = {
+	.mmap = user_status_mmap,
+	.read = user_status_read,
+	.llseek  = user_status_seek,
+	.release = user_status_release,
+};

kernel/trace/Makefile

Quoted 10 lines

kernel/trace/Makefile +10 -0

+
+/*
+ * Creates a set of tracefs files to allow user mode interactions.
+ */
+static int create_user_tracefs(void)
+{
+	struct dentry *edata, *emmap;
+
+	edata = tracefs_create_file("user_events_data", 0644, NULL,
+				    NULL, &user_data_fops);

BTW, I now define:

TRACE_MODE_WRITE for files to be written to, and TRACE_MODE_READ for files that are read only.

And soon tracefs will honor the gid mount option to define what group all the tracefs files should belong to on mount.

-- Steve

Quoted 14 lines

Quoted 14 lines

kernel/trace/trace_events_user.c new file +4 -0

+/*
+ * Handles the final close of the file from user mode.
+ */
+static int user_events_release(struct inode *node, struct file

*file) +{

kernel/trace/trace_events_user.c new file +6 -0

+	struct user_event_refs *refs;
+	struct user_event *user;
+	int i;
+
+	/*
+	 * refs is protected by RCU and could in theory change

immediately

(quoted diff) +1 -0

+	 * before this call on another core. To ensure we read

Quoted 8 lines

kernel/trace/trace_events_user.c line 983

Quoted 6 lines

the latest

kernel/trace/trace_events_user.c new file +5 -0

+	 * version of refs we acquire the RCU read lock again.
+	 */
+	rcu_read_lock_sched();
+	refs = rcu_dereference_sched(file->private_data);
+	rcu_read_unlock_sched();  

This still bothers me. Can another CPU call an ioctl here?

Sorry :)

No, another CPU cannot call the ioctl on the file, since if another CPU had a reference to this file release couldn't be called.

user_events_release is only called when the final reference to the file has been closed, so there cannot be another ioctl pending, starting or finishing for this file at the time it is called.

The last user mode program to call close() on the file will end up invoking user_events_release.

The user_event_refs is only accessible via the file's private_data, which now has zero references when release is executing. This means the private_data can no longer change and the rcu deref ensures we have the latest version.

refs is per-file, so while there can be other ioctl's occurring for other files, they are completely different ref objects than the one being cleaned up in the release of the file, it's not shared outside of this file lifetime, which has now ended.

Quoted 10 lines

  user_events_ioctl_reg() {
    user_events_ref_add() {
      refs = rcu_dereference_protected(file->private_data, ..);
      new_refs = kzalloc(size, GFP_KERNEL);
      rcu_assign_pointer(file->private_data, new_refs);
      if (refs)
        kfree_rcu(refs, rcu);

Quoted 2 lines

refs now freed.

If user_events_ioctl is executing for that same file, user_events_release could not have been called due to the file being in use to issue the ioctl.

Quoted 19 lines

Quoted 14 lines

kernel/trace/trace_events_user.c new file +6 -0

+
+	if (!refs)
+		goto out;
+
+	/*
+	 * Do not need RCU while enumerating the events that

were used.

(quoted diff) +1 -0

+	 * The lifetime of refs has reached an end, it's tied to

this file.

(quoted diff) +1 -0

+	 * The underlying user_events are ref counted, and

cannot be freed.

(quoted diff) +1 -0

+	 * After this decrement, the user_events may be freed

elsewhere.

Review

Quoted 2 lines

(quoted diff) +2 -0

+	 */
+	for (i = 0; i < refs->count; ++i) {  

Fault on refs->count

Quoted 1 line

??

refs after rcu_dereference is checked for null before accessing.

refs cannot be changed when release is being executed, since that would mean the ioctl ran without a file reference (not sure how that could happen).

This is why it's important that release get the latest version of refs, an ioctl could have JUST happened before the final close() in user mode, and if it jumped CPUs we could (in theory) get an old value. If we got an old value, then yes, the fault could occur.

This code uses the file ops release method as a final sync point to clean up everything for that file only after there are no more references to it at all, so nobody can do this kind of thing.

Is there some case I am missing where an ioctl on a file can be performed without a reference to that file?

Are you worried about a malicious user calling close on the file and then immediately issuing an ioctl on the now closed file?

If so, wouldn't ioctl just reject that file reference being used as not in the processes file table / invalid and not let the ioctl go through?

Quoted 25 lines · On Wed, 8 Dec 2021 16:58:23 -0800 Beau Belgrave <[email protected]>…

Quoted 14 lines

Quoted 14 lines

kernel/trace/trace_events_user.c new file +4 -0

+/*
+ * Handles the final close of the file from user mode.
+ */
+static int user_events_release(struct inode *node, struct file

*file) +{

kernel/trace/trace_events_user.c new file +6 -0

+	struct user_event_refs *refs;
+	struct user_event *user;
+	int i;
+
+	/*
+	 * refs is protected by RCU and could in theory change

immediately

(quoted diff) +1 -0

+	 * before this call on another core. To ensure we read

Quoted 8 lines

kernel/trace/trace_events_user.c line 983

Quoted 6 lines

the latest

kernel/trace/trace_events_user.c new file +5 -0

+	 * version of refs we acquire the RCU read lock again.
+	 */
+	rcu_read_lock_sched();
+	refs = rcu_dereference_sched(file->private_data);
+	rcu_read_unlock_sched();  

This still bothers me. Can another CPU call an ioctl here?

Sorry :)

Quoted 2 lines

No, another CPU cannot call the ioctl on the file, since if another CPU had a reference to this file release couldn't be called.

OK, so it should be good, as the last fdput() will call this (and the ioctl should keep that from happening until its done). But this could also be done with less confusion and less paranoia if we simply take the reg_mutex, as that should keep everything from changing, and we wouldn't need to do any rcu_read_lock*() from the release function.

Quoted 5 lines

user_events_release is only called when the final reference to the file has been closed, so there cannot be another ioctl pending, starting or finishing for this file at the time it is called.

Quoted 2 lines

The last user mode program to call close() on the file will end up invoking user_events_release.

It doesn't work like that. There's only one close(). But you are correct that it is protected, and that's by the fdget() and fdput() that is done within the ioctl (and other) system call.

Quoted 10 lines

The user_event_refs is only accessible via the file's private_data, which now has zero references when release is executing. This means the private_data can no longer change and the rcu deref ensures we have the latest version.

refs is per-file, so while there can be other ioctl's occurring for other files, they are completely different ref objects than the one being cleaned up in the release of the file, it's not shared outside of this file lifetime, which has now ended.

Right, but I'm still paranoid ;-)

Quoted 13 lines

Quoted 12 lines

  user_events_ioctl_reg() {
    user_events_ref_add() {
      refs = rcu_dereference_protected(file->private_data, ..);
      new_refs = kzalloc(size, GFP_KERNEL);
      rcu_assign_pointer(file->private_data, new_refs);
      if (refs)
        kfree_rcu(refs, rcu);

refs now freed.

Quoted 3 lines

If user_events_ioctl is executing for that same file, user_events_release could not have been called due to the file being in use to issue the ioctl.

The only thing protecting against this is the fdget/put logic in the system calls.

Quoted 32 lines

Quoted 19 lines

Quoted 14 lines

kernel/trace/trace_events_user.c new file +6 -0

+
+	if (!refs)
+		goto out;
+
+	/*
+	 * Do not need RCU while enumerating the events that

were used.

(quoted diff) +1 -0

+	 * The lifetime of refs has reached an end, it's tied to

this file.

(quoted diff) +1 -0

+	 * The underlying user_events are ref counted, and

cannot be freed.

(quoted diff) +1 -0

+	 * After this decrement, the user_events may be freed

elsewhere.

Review

Quoted 2 lines

(quoted diff) +2 -0

+	 */
+	for (i = 0; i < refs->count; ++i) {  

Fault on refs->count

Quoted 1 line

??

refs after rcu_dereference is checked for null before accessing.

refs cannot be changed when release is being executed, since that would mean the ioctl ran without a file reference (not sure how that could happen).

This is why it's important that release get the latest version of refs, an ioctl could have JUST happened before the final close() in user mode, and if it jumped CPUs we could (in theory) get an old value. If we got an old value, then yes, the fault could occur.

Quoted 6 lines

This code uses the file ops release method as a final sync point to clean up everything for that file only after there are no more references to it at all, so nobody can do this kind of thing.

Is there some case I am missing where an ioctl on a file can be performed without a reference to that file?

Well, the ioctl can be called as the close happens, but it's the internal working of fdget/put that protects it. If the ioctl is called at the same time as the close, the fdget in the ioctl will keep the close from calling the release. And as soon as the ioctl is finished, it will call the fdput() which then calls the release logic.

Quoted 8 lines

Are you worried about a malicious user calling close on the file and then immediately issuing an ioctl on the now closed file?

If so, wouldn't ioctl just reject that file reference being used as not in the processes file table / invalid and not let the ioctl go through?

I think it seems less confusing and saner to just use the mutex. It's not a fast path is it?

-- Steve

No, this is not a fast path, and I don't have a problem moving to a mutex if you feel that is better. I've likely become too close to this code to know when things are confusing for others.

Quoted 3 lines · On Thu, 9 Dec 2021 09:40:50 -0800 Beau Belgrave <[email protected]>…

No, this is not a fast path, and I don't have a problem moving to a mutex if you feel that is better. I've likely become too close to this code to know when things are confusing for others.

Yeah. I really dislike the "protection by algorithms" then protection by locking unless it is a fast path.

If this was a fast path then I'd be more concerned. I dislike global locks as well, but unless contention becomes a concern, I don't think we should worry about it.

Also, for this comment:

kernel/trace/trace_events_user.c new file +14 -0

+static int user_events_release(struct inode *node, struct file *file)
+{
+	struct user_event_refs *refs;
+	struct user_event *user;
+	int i;
+
+	/*
+	 * refs is protected by RCU and could in theory change immediately
+	 * before this call on another core. To ensure we read the latest
+	 * version of refs we acquire the RCU read lock again.
+	 */
+	rcu_read_lock_sched();
+	refs = rcu_dereference_sched(file->private_data);
+	rcu_read_unlock_sched();

How do you see refs changing on another core if this can only be called when nothing has a reference to it?

I think this comment and grabbing the rcu locks is what is causing me concern.

-- Steve

User program task:
CPU0: ioctl(fd, REG)
CPU1: close(fd)

IE: Some program registers and then immediately calls close on the file. If the CPU migrates right between the 2 and the close swaps, it is possible this could occur.

This could be attempted in tight loops maliciously as well.

I assume with a mutex there that some barrier is imposed to ensure correct reads in this condition? (By virtue of the mutex acquire/check)

Quoted 4 lines · On Thu, 9 Dec 2021 11:42:35 -0800 Beau Belgrave <[email protected]>…

User program task:
CPU0: ioctl(fd, REG)
CPU1: close(fd)

Quoted 8 lines

IE: Some program registers and then immediately calls close on the file. If the CPU migrates right between the 2 and the close swaps, it is possible this could occur.

This could be attempted in tight loops maliciously as well.

I assume with a mutex there that some barrier is imposed to ensure correct reads in this condition? (By virtue of the mutex acquire/check)

But as I stated before, the ioctl() uses fdget() which will prevent he close from calling the release. Even if they get swapped. If close goes first and starts down the path of the release, then the ioctl is guaranteed to return -EBADF. If it gets the fd, then close will be a nop, and the exit of the ioctl will call the release.

If this wasn't the case, then the race I was concerned about would be an issue.

Because we are both confused by this, add the mutex! :-)

-- Steve

I guess I am being paranoid about an architecture that does not have automatic cache consistency and while the write / read don't happen at the exact time, they happen close together. Close enough that one CPU reads the old value from a cache line and gets it wrong.

I don't believe that is possible on Intel, but I don't know if it's possible on other architectures (especially older ones).