On Tue, 25 Jun 2024 20:39:49 -0700 Namhyung Kim <namhyung@xxxxxxxxx> wrote: > When process_cond() failed, it freed the token but didn't reset the > arg->op.op to NULL. So it tried to free the arg->op.op again from > free_arg() from the caller and resulted in a double free. > > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx> > --- > src/event-parse.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/src/event-parse.c b/src/event-parse.c > index 9f0522c..c327917 100644 > --- a/src/event-parse.c > +++ b/src/event-parse.c > @@ -2375,8 +2375,11 @@ process_op(struct tep_event *event, struct tep_print_arg *arg, char **tok) > > /* it will set arg->op.right */ > type = process_cond(event, arg, tok); > - if (type == TEP_EVENT_ERROR) > - free(token); > + if (type == TEP_EVENT_ERROR) { > + /* arg->op.op (= token) will be freed at out_free */ > + arg->op.op = NULL; > + goto out_free; > + } > > } else if (strcmp(token, ">>") == 0 || > strcmp(token, "<<") == 0 || Hmm, so this fixes: 76a0eb8d5a20c ("libtraceevent: Fix event-parse memory leak in process_cond") Which added: + if (type == TEP_EVENT_ERROR) + free(token); because of a memory leak. But if token is arg->op, and we should be freeing that arg, how did it end up being a leak? Because args->op.left is allocated. Whatever frees that, should also have freed args->op.op. I'm getting a feeling that we are missing the real bug. Perhaps the real fix was: diff --git a/src/event-parse.c b/src/event-parse.c index 9f0522c8d9e4..c7ffad116c3f 100644 --- a/src/event-parse.c +++ b/src/event-parse.c @@ -2375,8 +2375,6 @@ process_op(struct tep_event *event, struct tep_print_arg *arg, char **tok) /* it will set arg->op.right */ type = process_cond(event, arg, tok); - if (type == TEP_EVENT_ERROR) - free(token); } else if (strcmp(token, ">>") == 0 || strcmp(token, "<<") == 0 || @@ -2587,6 +2585,9 @@ static int alloc_and_process_delim(struct tep_event *event, char *next_token, if (type == TEP_EVENT_OP) { type = process_op(event, field, &token); + if (type == TEP_EVENT_ERROR) + goto out_error; + if (consolidate_op_arg(field) < 0) type = TEP_EVENT_ERROR; -- Steve
![]() |