Re: [PATCH] tracing: Fix double free of event_trigger_data

[Steven Rostedt]

On Wed, 25 Jul 2018 12:40:08 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> Hmm, looks to me that event_enable_trigger_func() suffers the same
> issue. Perhaps we should add this patch too:
> 
> -- Steve
> 
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index d18ec0e58be2..2681d917f896 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -1457,6 +1457,10 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
>  	ret = trace_event_enable_disable(event_enable_file, 1, 1);
>  	if (ret < 0)
>  		goto out_put;
> +
> +	/* Up the trigger_data count to make sure reg doesn't free it on failure */
> +	event_trigger_init(trigger_ops, trigger_data);
> +
>  	ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
>  	/*
>  	 * The above returns on success the # of functions enabled,
> @@ -1464,12 +1468,13 @@ int event_enable_trigger_func(struct event_command *cmd_ops,
>  	 * Consider no functions a failure too.
>  	 */
>  	if (!ret) {
> +		cmd_ops->unreg(glob, trigger_ops, trigger_data, file);
>  		ret = -ENOENT;
> -		goto out_disable;
> -	} else if (ret < 0)
> -		goto out_disable;
> -	/* Just return zero, not the number of enabled functions */
> -	ret = 0;
> +	} else if (ret > 0)
> +		ret = 0;
> +
> +	/* Down the counter of trigger_data or free it if not used anymore */
> +	event_trigger_free(trigger_ops, trigger_data);

Nope, this doesn't work. It's a little more complex than the other one.
I'll just leave it, and fix the reg code for 4.19.

-- Steve

>   out:
>  	return ret;
>