Messages in this thread Patch in this message | | | Date | Wed, 8 Dec 2021 15:44:57 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH] tracing: Fix possible memory leak in __create_synth_event |
| |
On Fri, 26 Nov 2021 10:47:08 +0000 Miaoqian Lin <linmq006@gmail.com> wrote:
> Before goto err, call argv_free to handle argv in order to prevent > memory leak. > > Signed-off-by: Miaoqian Lin <linmq006@gmail.com> > --- > kernel/trace/trace_events_synth.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c > index 22db3ce95e74..fe2e37564c9b 100644 > --- a/kernel/trace/trace_events_synth.c > +++ b/kernel/trace/trace_events_synth.c > @@ -1261,6 +1261,7 @@ static int __create_synth_event(const char *name, const char *raw_fields) > */ > if (cmd_version > 1 && n_fields_this_loop >= 1) { > synth_err(SYNTH_ERR_INVALID_CMD, errpos(field_str)); > + argv_free(argv); > ret = -EINVAL; > goto err; > } > @@ -1268,6 +1269,7 @@ static int __create_synth_event(const char *name, const char *raw_fields) > fields[n_fields++] = field; > if (n_fields == SYNTH_FIELDS_MAX) { > synth_err(SYNTH_ERR_TOO_MANY_FIELDS, 0); > + argv_free(argv); > ret = -EINVAL; > goto err; > } > @@ -1277,6 +1279,7 @@ static int __create_synth_event(const char *name, const char *raw_fields) > > if (consumed < argc) { > synth_err(SYNTH_ERR_INVALID_CMD, 0); > + argv_free(argv); > ret = -EINVAL; > goto err; > }
A cleaner way is to have:
diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c index 98e002648994..a88f1f9046a6 100644 --- a/kernel/trace/trace_events_synth.c +++ b/kernel/trace/trace_events_synth.c @@ -1237,9 +1237,8 @@ static int __create_synth_event(const char *name, const char *raw_fields) argv + consumed, &consumed, &field_version); if (IS_ERR(field)) { - argv_free(argv); ret = PTR_ERR(field); - goto err; + goto err_free_arg; } /* @@ -1262,26 +1261,25 @@ static int __create_synth_event(const char *name, const char *raw_fields) if (cmd_version > 1 && n_fields_this_loop >= 1) { synth_err(SYNTH_ERR_INVALID_CMD, errpos(field_str)); ret = -EINVAL; - goto err; + goto err_free_arg; } fields[n_fields++] = field; if (n_fields == SYNTH_FIELDS_MAX) { synth_err(SYNTH_ERR_TOO_MANY_FIELDS, 0); ret = -EINVAL; - goto err; + goto err_free_arg; } n_fields_this_loop++; } + argv_free(argv); if (consumed < argc) { synth_err(SYNTH_ERR_INVALID_CMD, 0); ret = -EINVAL; goto err; } - - argv_free(argv); } if (n_fields == 0) { @@ -1307,6 +1305,8 @@ static int __create_synth_event(const char *name, const char *raw_fields) kfree(saved_fields); return ret; + err_free_arg: + argv_free(argv); err: for (i = 0; i < n_fields; i++) free_synth_field(fields[i]);
Feel free to send v2 and add:
Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
-- Steve
| |