On Tue, 31 Oct 2023 00:00:31 -0400
Steven Rostedt <rost...@goodmis.org> wrote:

> But the I applied:
> 
>   f5ca233e2e66d ("tracing: Increase trace array ref count on enable and 
> filter files")
> 
> And do the above commands again and BOOM! it crashes with:

The below patch appears to solve this. Care to test it?

-- Steve

Signed-off-by: Steven Rostedt (Google) <rost...@goodmis.org>
---
 include/linux/trace_events.h       |  4 ++++
 kernel/trace/trace.c               | 15 +++++++++++++++
 kernel/trace/trace.h               |  4 ++++
 kernel/trace/trace_events.c        | 29 +++++++++++++++++++++++++----
 kernel/trace/trace_events_filter.c |  3 +++
 5 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 12207dc6722d..696f8dc4aa53 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -492,6 +492,7 @@ enum {
        EVENT_FILE_FL_TRIGGER_COND_BIT,
        EVENT_FILE_FL_PID_FILTER_BIT,
        EVENT_FILE_FL_WAS_ENABLED_BIT,
+       EVENT_FILE_FL_FREED_BIT,
 };
 
 extern struct trace_event_file *trace_get_event_file(const char *instance,
@@ -630,6 +631,7 @@ extern int __kprobe_event_add_fields(struct dynevent_cmd 
*cmd, ...);
  *  TRIGGER_COND  - When set, one or more triggers has an associated filter
  *  PID_FILTER    - When set, the event is filtered based on pid
  *  WAS_ENABLED   - Set when enabled to know to clear trace on module removal
+ *  FREED         - File descriptor is freed, all fields should be considered 
invalid
  */
 enum {
        EVENT_FILE_FL_ENABLED           = (1 << EVENT_FILE_FL_ENABLED_BIT),
@@ -643,6 +645,7 @@ enum {
        EVENT_FILE_FL_TRIGGER_COND      = (1 << EVENT_FILE_FL_TRIGGER_COND_BIT),
        EVENT_FILE_FL_PID_FILTER        = (1 << EVENT_FILE_FL_PID_FILTER_BIT),
        EVENT_FILE_FL_WAS_ENABLED       = (1 << EVENT_FILE_FL_WAS_ENABLED_BIT),
+       EVENT_FILE_FL_FREED             = (1 << EVENT_FILE_FL_FREED_BIT),
 };
 
 struct trace_event_file {
@@ -671,6 +674,7 @@ struct trace_event_file {
         * caching and such. Which is mostly OK ;-)
         */
        unsigned long           flags;
+       atomic_t                ref;    /* ref count for opened files */
        atomic_t                sm_ref; /* soft-mode reference counter */
        atomic_t                tm_ref; /* trigger-mode reference counter */
 };
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2539cfc20a97..9aebf904ff97 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4978,6 +4978,20 @@ int tracing_open_file_tr(struct inode *inode, struct 
file *filp)
        if (ret)
                return ret;
 
+       mutex_lock(&event_mutex);
+
+       /* Fail if the file is marked for removal */
+       if (file->flags & EVENT_FILE_FL_FREED) {
+               trace_array_put(file->tr);
+               ret = -ENODEV;
+       } else {
+               event_file_get(file);
+       }
+
+       mutex_unlock(&event_mutex);
+       if (ret)
+               return ret;
+
        filp->private_data = inode->i_private;
 
        return 0;
@@ -4988,6 +5002,7 @@ int tracing_release_file_tr(struct inode *inode, struct 
file *filp)
        struct trace_event_file *file = inode->i_private;
 
        trace_array_put(file->tr);
+       event_file_put(file);
 
        return 0;
 }
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 0e1405abf4f7..4fcd293ecbd4 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1669,6 +1669,10 @@ extern void event_trigger_unregister(struct 
event_command *cmd_ops,
                                     char *glob,
                                     struct event_trigger_data *trigger_data);
 
+extern void event_file_get(struct trace_event_file *file);
+extern void event_file_put(struct trace_event_file *file);
+
+
 /**
  * struct event_trigger_ops - callbacks for trace event triggers
  *
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 70a029ae3c65..ec86f4894455 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -990,13 +990,33 @@ static void remove_subsystem(struct trace_subsystem_dir 
*dir)
        }
 }
 
+void event_file_get(struct trace_event_file *file)
+{
+       atomic_inc(&file->ref);
+}
+
+void event_file_put(struct trace_event_file *file)
+{
+       if (WARN_ON_ONCE(!atomic_read(&file->ref))) {
+               if (file->flags & EVENT_FILE_FL_FREED)
+                       kmem_cache_free(file_cachep, file);
+               return;
+       }
+
+       if (atomic_dec_and_test(&file->ref) &&
+           (file->flags & EVENT_FILE_FL_FREED)) {
+               kmem_cache_free(file_cachep, file);
+       }
+}
+
 static void remove_event_file_dir(struct trace_event_file *file)
 {
        eventfs_remove_dir(file->ei);
        list_del(&file->list);
        remove_subsystem(file->system);
        free_event_filter(file->filter);
-       kmem_cache_free(file_cachep, file);
+       file->flags |= EVENT_FILE_FL_FREED;
+       event_file_put(file);
 }
 
 /*
@@ -1369,7 +1389,7 @@ event_enable_read(struct file *filp, char __user *ubuf, 
size_t cnt,
                flags = file->flags;
        mutex_unlock(&event_mutex);
 
-       if (!file)
+       if (!file || flags & EVENT_FILE_FL_FREED)
                return -ENODEV;
 
        if (flags & EVENT_FILE_FL_ENABLED &&
@@ -1404,7 +1424,7 @@ event_enable_write(struct file *filp, const char __user 
*ubuf, size_t cnt,
                ret = -ENODEV;
                mutex_lock(&event_mutex);
                file = event_file_data(filp);
-               if (likely(file)) {
+               if (likely(file && !(file->flags & EVENT_FILE_FL_FREED))) {
                        printk("update file = %px\n", file);
                        printk("update tr = %px\n", file->tr);
                        ret = tracing_update_buffers(file->tr);
@@ -1686,7 +1706,7 @@ event_filter_read(struct file *filp, char __user *ubuf, 
size_t cnt,
 
        mutex_lock(&event_mutex);
        file = event_file_data(filp);
-       if (file)
+       if (file && !(file->flags & EVENT_FILE_FL_FREED))
                print_event_filter(file, s);
        mutex_unlock(&event_mutex);
 
@@ -2905,6 +2925,7 @@ trace_create_new_event(struct trace_event_call *call,
        atomic_set(&file->tm_ref, 0);
        INIT_LIST_HEAD(&file->triggers);
        list_add(&file->list, &tr->events);
+       event_file_get(file);
 
        return file;
 }
diff --git a/kernel/trace/trace_events_filter.c 
b/kernel/trace/trace_events_filter.c
index 33264e510d16..0c611b281a5b 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2349,6 +2349,9 @@ int apply_event_filter(struct trace_event_file *file, 
char *filter_string)
        struct event_filter *filter = NULL;
        int err;
 
+       if (file->flags & EVENT_FILE_FL_FREED)
+               return -ENODEV;
+
        if (!strcmp(strstrip(filter_string), "0")) {
                filter_disable(file);
                filter = event_filter(file);
-- 
2.42.0

Reply via email to