linux-kernel - Re: [PATCH] tracing: Fix double free of event_trigger

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180725152904.11edde2b@gandalf.local.home>
Date:   Wed, 25 Jul 2018 15:29:04 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Tom Zanussi <tom.zanussi@...ux.intel.com>,
        Hiraku Toyooka <hiraku.toyooka@...ertrust.co.jp>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] tracing: Fix double free of event_trigger_data

On Wed, 25 Jul 2018 12:40:08 -0400
Steven Rostedt <rostedt@...dmis.org> 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;
>