linux-kernel - Re: perf: perf_fuzzer quickly locks up on 4.15-rc7

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20180109180207.GM6176@hirez.programming.kicks-ass.net>
Date:   Tue, 9 Jan 2018 19:02:07 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Vince Weaver <vincent.weaver@...ne.edu>,
        Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: perf: perf_fuzzer quickly locks up on 4.15-rc7

On Tue, Jan 09, 2018 at 12:53:46PM -0500, Steven Rostedt wrote:
> Looking at ftrace_profile_set_filter(), I see it starts with:
> 
> 	mutex_lock(&event_mutex);
> 
> How much of a big deal would it be if we move taking event_mutex() into
> perf_ioctl(), and then make ftrace_profile_set_filter() not take the
> event_mutex. This is the only place that function is used. Would that
> work?

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 4df5b695bf0d..9fac7ac14b32 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -4741,9 +4741,11 @@ static long perf_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  	struct perf_event_context *ctx;
>  	long ret;
>  
> +	mutex_lock(&event_mutex);
>  	ctx = perf_event_ctx_lock(event);
>  	ret = _perf_ioctl(event, cmd, arg);
>  	perf_event_ctx_unlock(event, ctx);
> +	mutex_unlock(&event_mutex);
>  
>  	return ret;
>  }

This would globally serialize all perf_ioctl()'s, also that event_mutex
is for trace_events and really does not belong in perf.

So no, I really rather would not do this.

The alternative I was thinking of was lifting the cpuhp lock out from
under event_mutex, that would also break the chain, but would probably
be lots of work for trace bits.