linux-kernel - Re: Some -serious- BPF-related litmus tests

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: <20200525172154.GZ2869@paulmck-ThinkPad-P72>
Date:   Mon, 25 May 2020 10:21:54 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Andrii Nakryiko <andriin@...com>,
        Alan Stern <stern@...land.harvard.edu>, parri.andrea@...il.com,
        will@...nel.org, boqun.feng@...il.com, npiggin@...il.com,
        dhowells@...hat.com, j.alglave@....ac.uk, luc.maranget@...ia.fr,
        akiyks@...il.com, dlustig@...dia.com, joel@...lfernandes.org,
        linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
        "andrii.nakryiko@...il.com" <andrii.nakryiko@...il.com>
Subject: Re: Some -serious- BPF-related litmus tests

On Mon, May 25, 2020 at 07:02:57PM +0200, Peter Zijlstra wrote:
> On Mon, May 25, 2020 at 08:47:30AM -0700, Paul E. McKenney wrote:
> > On Mon, May 25, 2020 at 01:25:21PM +0200, Peter Zijlstra wrote:
> 
> > > That is; how can you use a spinlock on the producer side at all?
> > 
> > So even trylock is now forbidden in NMI handlers?  If so, why?
> 
> The litmus tests don't have trylock.

Fair point.

> But you made me look at the actual patch:
> 
> +static void *__bpf_ringbuf_reserve(struct bpf_ringbuf *rb, u64 size)
> +{
> +	unsigned long cons_pos, prod_pos, new_prod_pos, flags;
> +	u32 len, pg_off;
> +	struct bpf_ringbuf_hdr *hdr;
> +
> +	if (unlikely(size > RINGBUF_MAX_RECORD_SZ))
> +		return NULL;
> +
> +	len = round_up(size + BPF_RINGBUF_HDR_SZ, 8);
> +	cons_pos = smp_load_acquire(&rb->consumer_pos);
> +
> +	if (in_nmi()) {
> +		if (!spin_trylock_irqsave(&rb->spinlock, flags))
> +			return NULL;
> +	} else {
> +		spin_lock_irqsave(&rb->spinlock, flags);
> +	}
> 
> And that is of course utter crap. That's like saying you don't care
> about your NMI data.

Almost.  It is really saying that -if- there is sufficient lock
contention, printk()s will be lost.  Just as they always have been if
there is more printk() volume than can be accommodated.

							Thanx, Paul