On 5/28/19 10:31 PM, Steven Rostedt wrote: > On Tue, 28 May 2019 17:43:38 +0200 > Tomas Bortoli <tomasbort...@gmail.com> wrote: > >> @@ -578,6 +578,8 @@ predicate_parse(const char *str, int nr_parens, int >> nr_preds, >> out_free: >> kfree(op_stack); >> kfree(inverts); >> + for (i = 0; prog_stack[i].pred; i++) >> + kfree(prog_stack[i].pred); >> kfree(prog_stack); >> return ERR_PTR(ret); >> } > > I should have caught this, but thanks to the zero day bot, it found it > first: > > kernel/trace/trace_events_filter.c:582:27-31: ERROR: prog_stack is NULL but > dereferenced. > > I changed the patch with the following: > > From dfb4a6f2191a80c8b790117d0ff592fd712d3296 Mon Sep 17 00:00:00 2001 > From: Tomas Bortoli <tomasbort...@gmail.com> > Date: Tue, 28 May 2019 17:43:38 +0200 > Subject: [PATCH] tracing: Avoid memory leak in predicate_parse() > > In case of errors, predicate_parse() goes to the out_free label > to free memory and to return an error code. > > However, predicate_parse() does not free the predicates of the > temporary prog_stack array, thence leaking them. > > Link: http://lkml.kernel.org/r/20190528154338.29976-1-tomasbort...@gmail.com > > Cc: sta...@vger.kernel.org > Fixes: 80765597bc587 ("tracing: Rewrite filter logic to be simpler and > faster") > Reported-by: syzbot+6b8e0fb820e570c59...@syzkaller.appspotmail.com > Signed-off-by: Tomas Bortoli <tomasbort...@gmail.com> > [ Added protection around freeing prog_stack[i].pred ] > Signed-off-by: Steven Rostedt (VMware) <rost...@goodmis.org> > --- > kernel/trace/trace_events_filter.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/kernel/trace/trace_events_filter.c > b/kernel/trace/trace_events_filter.c > index d3e59312ef40..5079d1db3754 100644 > --- a/kernel/trace/trace_events_filter.c > +++ b/kernel/trace/trace_events_filter.c > @@ -428,7 +428,7 @@ predicate_parse(const char *str, int nr_parens, int > nr_preds, > op_stack = kmalloc_array(nr_parens, sizeof(*op_stack), GFP_KERNEL); > if (!op_stack) > return ERR_PTR(-ENOMEM); > - prog_stack = kmalloc_array(nr_preds, sizeof(*prog_stack), GFP_KERNEL); > + prog_stack = kcalloc(nr_preds, sizeof(*prog_stack), GFP_KERNEL); > if (!prog_stack) { > parse_error(pe, -ENOMEM, 0); > goto out_free; > @@ -579,7 +579,11 @@ predicate_parse(const char *str, int nr_parens, int > nr_preds, > out_free: > kfree(op_stack); > kfree(inverts); > - kfree(prog_stack); > + if (prog_stack) { > + for (i = 0; prog_stack[i].pred; i++) > + kfree(prog_stack[i].pred); > + kfree(prog_stack); > + } > return ERR_PTR(ret); > } > >
Oops again, I should have been more careful. Thanks.
