linux-kernel - Re: [PATCH v3 tip/perf/core 4/4] uprobes: add speculative lockless VMA-to-inode-to-uprobe resolution

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: <20241023201917.GG11151@noisy.programming.kicks-ass.net>
Date: Wed, 23 Oct 2024 22:19:17 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Andrii Nakryiko <andrii@...nel.org>, linux-trace-kernel@...r.kernel.org,
	linux-mm@...ck.org, oleg@...hat.com, rostedt@...dmis.org,
	mhiramat@...nel.org, bpf@...r.kernel.org,
	linux-kernel@...r.kernel.org, jolsa@...nel.org, paulmck@...nel.org,
	willy@...radead.org, surenb@...gle.com, akpm@...ux-foundation.org,
	mjguzik@...il.com, brauner@...nel.org, jannh@...gle.com,
	mhocko@...nel.org, vbabka@...e.cz, shakeel.butt@...ux.dev,
	hannes@...xchg.org, Liam.Howlett@...cle.com,
	lorenzo.stoakes@...cle.com
Subject: Re: [PATCH v3 tip/perf/core 4/4] uprobes: add speculative lockless
 VMA-to-inode-to-uprobe resolution

On Wed, Oct 23, 2024 at 01:02:53PM -0700, Andrii Nakryiko wrote:

> > > +      * but can't be freed from under us, so it's safe to read fields from
> > > +      * it, even if the values are some garbage values; ultimately
> > > +      * find_uprobe_rcu() + mmap_lock_speculation_end() check will ensure
> > > +      * that whatever we speculatively found is correct
> > > +      */
> > > +     vm_file = READ_ONCE(vma->vm_file);
> > > +     if (!vm_file)
> > > +             return NULL;
> > > +
> > > +     vm_pgoff = data_race(vma->vm_pgoff);
> > > +     vm_start = data_race(vma->vm_start);
> > > +     vm_inode = data_race(vm_file->f_inode);
> >
> > So... seqcount has kcsan annotations other than data_race(). I suppose
> > this works, but it all feels like a bad copy with random changes.
> 
> I'm not sure what this means... Do I need to change anything? Drop
> data_race()? Use READ_ONCE()? Do nothing?

Keep for now. I've ranted at 1/n a bit, but unless the response is:
yeah, obviously this should be seqcount (unlikely) this is something
that can be fixed later (*sigh* always later... this todo list is a
problem).