Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

eventfs: Hold eventfs_mutex when calling callback functions

The callback function that is used to create inodes and dentries is not
protected by anything and the data that is passed to it could become
stale. After eventfs_remove_dir() is called by the tracing system, it is
free to remove the events that are associated to that directory.
Unfortunately, that means the callbacks must not be called after that.

CPU0 CPU1
---- ----
eventfs_root_lookup() {
eventfs_remove_dir() {
mutex_lock(&event_mutex);
ei->is_freed = set;
mutex_unlock(&event_mutex);
}
kfree(event_call);

for (...) {
entry = &ei->entries[i];
r = entry->callback() {
call = data; // call == event_call above
if (call->flags ...)

[ USE AFTER FREE BUG ]

The safest way to protect this is to wrap the callback with:

mutex_lock(&eventfs_mutex);
if (!ei->is_freed)
r = entry->callback();
else
r = -1;
mutex_unlock(&eventfs_mutex);

This will make sure that the callback will not be called after it is
freed. But now it needs to be known that the callback is called while
holding internal eventfs locks, and that it must not call back into the
eventfs / tracefs system. There's no reason it should anyway, but document
that as well.

Link: https://lore.kernel.org/all/CA+G9fYu9GOEbD=rR5eMR-=HJ8H6rMsbzDC2ZY5=Y50WpWAE7_Q@mail.gmail.com/
Link: https://lkml.kernel.org/r/20231101172649.906696613@goodmis.org

Cc: Ajay Kaher <akaher@vmware.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Fixes: 5790b1fb3d672 ("eventfs: Remove eventfs_file and just use eventfs_inode")
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

+63 -2
+20 -2
fs/tracefs/event_inode.c
··· 615 615 entry = &ei->entries[i]; 616 616 if (strcmp(name, entry->name) == 0) { 617 617 void *cdata = data; 618 - r = entry->callback(name, &mode, &cdata, &fops); 618 + mutex_lock(&eventfs_mutex); 619 + /* If ei->is_freed, then the event itself may be too */ 620 + if (!ei->is_freed) 621 + r = entry->callback(name, &mode, &cdata, &fops); 622 + else 623 + r = -1; 624 + mutex_unlock(&eventfs_mutex); 619 625 if (r <= 0) 620 626 continue; 621 627 ret = simple_lookup(dir, dentry, flags); ··· 755 749 void *cdata = data; 756 750 entry = &ei->entries[i]; 757 751 name = entry->name; 758 - r = entry->callback(name, &mode, &cdata, &fops); 752 + mutex_lock(&eventfs_mutex); 753 + /* If ei->is_freed, then the event itself may be too */ 754 + if (!ei->is_freed) 755 + r = entry->callback(name, &mode, &cdata, &fops); 756 + else 757 + r = -1; 758 + mutex_unlock(&eventfs_mutex); 759 759 if (r <= 0) 760 760 continue; 761 761 d = create_file_dentry(ei, i, parent, name, mode, cdata, fops, false); ··· 831 819 * data = A pointer to @data, and the callback may replace it, which will 832 820 * cause the file created to pass the new data to the open() call. 833 821 * fops = the fops to use for the created file. 822 + * 823 + * NB. @callback is called while holding internal locks of the eventfs 824 + * system. The callback must not call any code that might also call into 825 + * the tracefs or eventfs system or it will risk creating a deadlock. 834 826 */ 835 827 struct eventfs_inode *eventfs_create_dir(const char *name, struct eventfs_inode *parent, 836 828 const struct eventfs_entry *entries, ··· 894 878 * @data: The default data to pass to the files (an entry may override it). 895 879 * 896 880 * This function creates the top of the trace event directory. 881 + * 882 + * See eventfs_create_dir() for use of @entries. 897 883 */ 898 884 struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry *parent, 899 885 const struct eventfs_entry *entries,
+43
include/linux/tracefs.h
··· 23 23 24 24 struct eventfs_file; 25 25 26 + /** 27 + * eventfs_callback - A callback function to create dynamic files in eventfs 28 + * @name: The name of the file that is to be created 29 + * @mode: return the file mode for the file (RW access, etc) 30 + * @data: data to pass to the created file ops 31 + * @fops: the file operations of the created file 32 + * 33 + * The evetnfs files are dynamically created. The struct eventfs_entry array 34 + * is passed to eventfs_create_dir() or eventfs_create_events_dir() that will 35 + * be used to create the files within those directories. When a lookup 36 + * or access to a file within the directory is made, the struct eventfs_entry 37 + * array is used to find a callback() with the matching name that is being 38 + * referenced (for lookups, the entire array is iterated and each callback 39 + * will be called). 40 + * 41 + * The callback will be called with @name for the name of the file to create. 42 + * The callback can return less than 1 to indicate that no file should be 43 + * created. 44 + * 45 + * If a file is to be created, then @mode should be populated with the file 46 + * mode (permissions) for which the file is created for. This would be 47 + * used to set the created inode i_mode field. 48 + * 49 + * The @data should be set to the data passed to the other file operations 50 + * (read, write, etc). Note, @data will also point to the data passed in 51 + * to eventfs_create_dir() or eventfs_create_events_dir(), but the callback 52 + * can replace the data if it chooses to. Otherwise, the original data 53 + * will be used for the file operation functions. 54 + * 55 + * The @fops should be set to the file operations that will be used to create 56 + * the inode. 57 + * 58 + * NB. This callback is called while holding internal locks of the eventfs 59 + * system. The callback must not call any code that might also call into 60 + * the tracefs or eventfs system or it will risk creating a deadlock. 61 + */ 26 62 typedef int (*eventfs_callback)(const char *name, umode_t *mode, void **data, 27 63 const struct file_operations **fops); 28 64 65 + /** 66 + * struct eventfs_entry - dynamically created eventfs file call back handler 67 + * @name: Then name of the dynamic file in an eventfs directory 68 + * @callback: The callback to get the fops of the file when it is created 69 + * 70 + * See evenfs_callback() typedef for how to set up @callback. 71 + */ 29 72 struct eventfs_entry { 30 73 const char *name; 31 74 eventfs_callback callback;