linux-kernel - Re: [PATCH trace/for-next 1/3] bpf: put bpf_link's program when link is safe to be deallocated

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: <20241101065558.4be28057@gandalf.local.home>
Date: Fri, 1 Nov 2024 06:55:58 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Andrii Nakryiko <andrii@...nel.org>
Cc: linux-trace-kernel@...r.kernel.org, bpf@...r.kernel.org, ast@...nel.org,
 daniel@...earbox.net, martin.lau@...nel.org,
 mathieu.desnoyers@...icios.com, linux-kernel@...r.kernel.org,
 mhiramat@...nel.org, peterz@...radead.org, paulmck@...nel.org,
 jrife@...gle.com
Subject: Re: [PATCH trace/for-next 1/3] bpf: put bpf_link's program when
 link is safe to be deallocated

On Thu, 31 Oct 2024 14:09:36 -0700
Andrii Nakryiko <andrii@...nel.org> wrote:

> In general, BPF link's underlying BPF program should be considered to be
> reachable through attach hook -> link -> prog chain, and, pessimistically,
> we have to assume that as long as link's memory is not safe to free,
> attach hook's code might hold a pointer to BPF program and use it.
> 
> As such, it's not (generally) correct to put link's program early before
> waiting for RCU GPs to go through. More eager bpf_prog_put() that we
> currently do is mostly correct due to BPF program's release code doing
> similar RCU GP waiting, but as will be shown in the following patches,
> BPF program can be non-sleepable (and, thus, reliant on only "classic"
> RCU GP), while BPF link's attach hook can have sleepable semantics and
> needs to be protected by RCU Tasks Trace, and for such cases BPF link
> has to go through RCU Tasks Trace + "classic" RCU GPs before being
> deallocated. And so, if we put BPF program early, we might free BPF
> program before we free BPF link, leading to use-after-free situation.
> 
> So, this patch defers bpf_prog_put() until we are ready to perform
> bpf_link's deallocation. At worst, this delays BPF program freeing by
> one extra RCU GP, but that seems completely acceptable. Alternatively,
> we'd need more elaborate ways to determine BPF hook, BPF link, and BPF
> program lifetimes, and how they relate to each other, which seems like
> an unnecessary complication.
> 
> Note, for most BPF links we still will perform eager bpf_prog_put() and
> link dealloc, so for those BPF links there are no observable changes
> whatsoever. Only BPF links that use deferred dealloc might notice
> slightly delayed freeing of BPF programs.
> 
> Also, to reduce code and logic duplication, extract program put + link
> dealloc logic into bpf_link_dealloc() helper.
> 
> Signed-off-by: Andrii Nakryiko <andrii@...nel.org>


Hi Andrii,

Do you want me to add this on top of my queue? If so, would it be possible
that I can get a tested-by from someone? As I don't do much to test BPF
patches.

-- Steve