netdev - Re: [RFC PATCH bpf-next 0/2] Improve prog array uref semantics

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: <eb20fd2c-0fb7-48f7-9fd0-4d654363f4da@app.fastmail.com>
Date: Wed, 30 Oct 2024 15:36:24 +0900
From: "Daniel Xu" <dxu@...uu.xyz>
To: "bpf@...r.kernel.org" <bpf@...r.kernel.org>, linux-kernel@...r.kernel.org,
 netdev@...r.kernel.org, "Daniel Borkmann" <daniel@...earbox.net>
Subject: Re: [RFC PATCH bpf-next 0/2] Improve prog array uref semantics

Hey Daniel,

On Wed, Aug 23, 2023, at 9:08 AM, Daniel Xu wrote:
> This patchset changes the behavior of TC and XDP hooks during attachment
> such that any BPF_MAP_TYPE_PROG_ARRAY that the prog uses has an extra
> uref taken.
>
> The goal behind this change is to try and prevent confusion for the
> majority of use cases. The current behavior where when the last uref is
> dropped the prog array map is emptied is quite confusing. Confusing
> enough for there to be multiple references to it in ebpf-go [0][1].
>
> Completely solving the problem is difficult. As stated in c9da161c6517
> ("bpf: fix clearing on persistent program array maps"), it is
> difficult-to-impossible to walk the full dependency graph b/c it is too
> dynamic.
>
> However in practice, I've found that all progs in a tailcall chain
> share the same prog array map. Knowing that, if we take a uref on any
> used prog array map when the program is attached, we can simplify the
> majority use case and make it more ergonomic.
>
> I'll be the first to admit this is not a very clean solution. It does
> not fully solve the problem. Nor does it make overall logic any simpler.
> But I do think it makes a pretty big usability hole slightly smaller.
>
> I've done some basic testing using a repro program [3] I wrote to debug
> the original issue that eventually led me to this patchset. If we wanna
> move forward with this approach, I'll resend with selftests.
>
> [0]: 
> https://github.com/cilium/ebpf/blob/01ebd4c1e2b9f8b3dd4fd2382aa1092c3c9bfc9d/doc.go#L22-L24
> [1]: 
> https://github.com/cilium/ebpf/blob/d1a52333f2c0fed085f8d742a5a3c164795d8492/collection.go#L320-L321
> [2]: https://github.com/danobi/tc_tailcall_repro

I recently remembered about this again. Was suggested I poke you in case you're interested.
I looked again and I think this is kinda a neat hack. I probably won't have time to pick this back
up either way.