From: Steven Rostedt <[email protected]>

The enable_data variable gets freed on most error paths in
event_enable_trigger_parse(). Use free() to free it and just before
returning normally, call retain_and_null_ptr(enable_data) just before a
successful exit to keep it from being freed. On success, the enable_data
is assigned to the trigger_data->private_data field.

Also add a comment to why event_trigger_free(trigger_data) is being called
before a successful exit.

Reviewed-by: Masami Hiramatsu (Google) <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
Changes since v1: 
https://patch.msgid.link/[email protected]

- s/no_return_ptr/retain_and_null_ptr/ as no_return_ptr() has __must_check
  (Sashiko)

- Added a comment to explain the last event_trigger_free() before a successful
  exit.

 kernel/trace/trace_events_trigger.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/trace_events_trigger.c 
b/kernel/trace/trace_events_trigger.c
index 655db2e82513..87601c2560d0 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -1739,7 +1739,7 @@ int event_enable_trigger_parse(struct event_command 
*cmd_ops,
                               char *glob, char *cmd, char *param_and_filter)
 {
        struct trace_event_file *event_enable_file;
-       struct enable_trigger_data *enable_data;
+       struct enable_trigger_data *enable_data __free(kfree) = NULL;
        struct event_trigger_data *trigger_data;
        struct trace_array *tr = file->tr;
        char *param, *filter;
@@ -1789,17 +1789,13 @@ int event_enable_trigger_parse(struct event_command 
*cmd_ops,
        enable_data->file = event_enable_file;
 
        trigger_data = trigger_data_alloc(cmd_ops, cmd, param, enable_data);
-       if (!trigger_data) {
-               kfree(enable_data);
+       if (!trigger_data)
                return ret;
-       }
 
        if (remove) {
                event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
                kfree(trigger_data);
-               kfree(enable_data);
-               ret = 0;
-               return ret;
+               return 0;
        }
 
        /* Up the trigger_data count to make sure nothing frees it on failure */
@@ -1828,7 +1824,12 @@ int event_enable_trigger_parse(struct event_command 
*cmd_ops,
        if (ret)
                goto out_disable;
 
+       /* Release the above reference taken for modules */
        event_trigger_free(trigger_data);
+
+       /* The enabled_data is assigned to trigger_data->private_data */
+       retain_and_null_ptr(enable_data);
+
        return ret;
  out_disable:
        trace_event_enable_disable(event_enable_file, 0, 1);
@@ -1837,7 +1838,6 @@ int event_enable_trigger_parse(struct event_command 
*cmd_ops,
  out_free:
        event_trigger_reset_filter(cmd_ops, trigger_data);
        event_trigger_free(trigger_data);
-       kfree(enable_data);
 
        return ret;
 }
-- 
2.53.0


Reply via email to