netdev - Re: [PATCH bpf-next 3/4] bpf: fix lockdep false positive in bpf_prog

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: <20190130103754.GA3103@hirez.programming.kicks-ass.net>
Date:   Wed, 30 Jan 2019 11:37:54 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Alexei Starovoitov <ast@...nel.org>
Cc:     davem@...emloft.net, daniel@...earbox.net, edumazet@...gle.com,
        jannh@...gle.com, netdev@...r.kernel.org, kernel-team@...com
Subject: Re: [PATCH bpf-next 3/4] bpf: fix lockdep false positive in
 bpf_prog_register

On Wed, Jan 30, 2019 at 11:10:58AM +0100, Peter Zijlstra wrote:
> On Tue, Jan 29, 2019 at 08:04:57PM -0800, Alexei Starovoitov wrote:
> > Lockdep warns about false positive:
> 
> The report reads like:
> 
> 	tracepoint_probe_register()
> #0	  mutex_lock(&tracepoint_mutex)
> 	  tracepoint_add_func()
> 	    static_key_slow_inc()
> #1	      cpus_read_lock();
> 
> 
> 	_cpu_up()
> #1	  cpus_write_lock();
> 	  ...
> 	  perf_event_init_cpu()
> #2	    mutex_lock(&pmus_lock);
> #3	    mutex_lock(&ctx->mutex);
> 
> 
> 	perf_ioctl()
> #4	  perf_event_ctx_lock();

Sorry, that's #3, and then do s/#5/#4/ on the rest of the text.

> 	  _perf_ioctl(IOC_QUERY_BPF)
> 	    perf_event_query_prog_array()
> #5	      mutex_lock(&bpf_event_mutex);
> 
> 
> 	bpf_probe_register()
> #5	  mutex_lock(&bpf_event_mutex);
> 	  __bpf_probe_register()
> 	    tracepoint_probe_register()
> #0	      mutex_lock(&tracepoint_mutex);
> 
> Which to me reads like an entirely valid deadlock scenario.
> 
> And note that the first and last can be combined to give:
> 
> 	bpf_probe_register()
> #5	  mutex_lock(&bpf_event_mutex);
> 	  __bpf_probe_register()
> 	    tracepoint_probe_register()
> #0	      mutex_lock(&tracepoint_mutex);
> 	      tracepoint_add_func()
> 	        static_key_slow_inc()
> #1		  cpus_read_lock();
> 
> 
> Which generates a deadlock even without #0.
> 
> Why do you say this is not possible? All you need is 3 CPUs, one doing a
> CPU online, one doing a perf ioctl() and one doing that
> bpf_probe_register().
> 
>