linux-kernel - Re: [PATCH] tracing/user_events: Run BPF program if attached

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]

Date:   Wed, 17 May 2023 12:26:44 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Beau Belgrave <beaub@...ux.microsoft.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-trace-kernel@...r.kernel.org,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
        David Vernet <void@...ifault.com>, dthaler@...rosoft.com,
        brauner@...nel.org, hch@...radead.org
Subject: Re: [PATCH] tracing/user_events: Run BPF program if attached

On Wed, May 17, 2023 at 12:08 PM Beau Belgrave
<beaub@...ux.microsoft.com> wrote:
>
> user_event_mm_dup() puts a new mm into the global list before the
> enablers list is fully populated.

Then that simply needs to be fixed.

user_event_mm_dup() should not madd the mm into the global list until
it is *done*.

Because if it makes that list visible to others in a half-way state,
then it needs to use the proper locking and use event_mutex.

You can't say "this is so critical that we can't take a lock" and then
use that as an excuse to simply do buggy code.

Either take the lock in user_event_mm_dup(), or make sure that the
data structures are all completely local so that no lock is necessary.

Here's a COMPLETELY UNTESTED patch that just separates out the notion
of "allocate" and "attach".

NOTE NOTE NOTE! I am *not* claiming this patch works. It builds for
me. It looks right. It seems like it's the right thing to do. But it
might have some issues.

With this, the newly dup'ed list is attached to the process once after
it is done, so nobody can see the list being built up.

Also note that this does NOT fix the incorrect RCU walks.

           Linus

View attachment "patch.diff" of type "text/x-patch" (2688 bytes)