linux-kernel - Re: [PATCH bpf v2] bpf: Fix prog_array UAF in __uprobe_perf

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] [day] [month] [year] [list]

Message-ID: <CAADnVQJ9grJJs4s_eBBk7iL136FNKt3ahhaLLDo1igrBBscxYA@mail.gmail.com>
Date: Mon, 9 Dec 2024 16:54:57 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Jann Horn <jannh@...gle.com>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, KP Singh <kpsingh@...nel.org>, 
	Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Delyan Kratunov <delyank@...com>, 
	bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	linux-trace-kernel <linux-trace-kernel@...r.kernel.org>, stable <stable@...r.kernel.org>
Subject: Re: [PATCH bpf v2] bpf: Fix prog_array UAF in __uprobe_perf_func()

On Mon, Dec 9, 2024 at 2:45 PM Andrii Nakryiko
<andrii.nakryiko@...il.com> wrote:
>
> Ok, weeding through the perf/uprobe plumbing for BPF, I think we avoid
> this problem with uprobe BPF link because uprobe_unregister_sync()
> waits for RCU Tasks Trace GP, and so once we finish uprobe
> unregistration, we have a guarantee that there is no more uprobe that
> might dereference our BPF program. (I might have thought about this
> problem when fixing BPF link for sleepable tracepoints, but I missed
> the legacy perf_event attach/detach case).
>
> With legacy perf event perf_event_detach_bpf_prog() we don't do any of
> that, we just NULL out pointer and do bpf_prog_put(), not caring
> whether this is uprobe, kprobe, or tracepoint...
>
> So one way to solve this is to either teach
> perf_event_detach_bpf_prog() to delay bpf_prog_put() until after RCU
> Tasks Trace GP (which is what we do with bpf_prog_array, but not the
> program itself),

since this is a legacy prog detach api I would just add sync_rcu_tt
there. It's a backportable one line change.

> or add prog->aux->sleepable_hook flag in addition to
> prog->aux->sleepable, and then inside bpf_prog_put() check
> (prog->aux->sleepable || prog->aux->sleepable_hook) and do RCU Tasks
> Trace delay (in addition to normal call_rcu()).

That sounds like more work and if we introduce sleepable_hook
we would better generalize it and rely on it everywhere.
Which makes it even more work and certainly not for stable.

> Third alternative would be to have something like
> bpf_prog_put_sleepable() (just like we have
> bpf_prog_array_free_sleepable()), where this would do additional
> call_rcu_tasks_trace() even if BPF program itself isn't sleepable.

Sounds like less work than 2, but conceptually it's the same as 1.
Just call_rcu_tt vs sync_rcu_tt.
And we can wait just fine in that code path.
So I'd go with 1.