linux-kernel - 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: <CAADnVQJ5NnDqx_TMbwHOPySUaJRE-N5K7L_whDsfeyMRBNOFkA@mail.gmail.com>
Date: Wed, 20 Nov 2024 08:07:29 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Daniel Xu <dxu@...uu.xyz>
Cc: "bpf@...r.kernel.org" <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	Network Development <netdev@...r.kernel.org>, Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [RFC PATCH bpf-next 0/2] Improve prog array uref semantics

On Wed, Nov 20, 2024 at 7:55 AM Daniel Xu <dxu@...uu.xyz> wrote:
>
>
>
> On Sat, Nov 16, 2024, at 2:17 PM, Alexei Starovoitov wrote:
> > On Tue, Oct 29, 2024 at 11:36 PM Daniel Xu <dxu@...uu.xyz> wrote:
> >>
> >> 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.
> >
> > Are you proposing to inc map uref when prog is attached?
> >
> > But that re-adds the circular dependency that uref concept is solving.
> > When prog is inserted into prog array prog refcnt is incremented.
> > So if prog also incremented uref. The user space can exit
> > but prog array and progs will stay there though nothing is using them.
> > I guess I'm missing the idea.
>
> IIRC the old-style tc/xdp attachment is the one incrementing the uref.

uref is incremented when FD is given to user space and
file->release() callback decrements uref.

I don't think any of the attach operations mess with uref.
At least they shouldn't.

> Once
> whatever program there is detached the uref is dropped. So I don't think
> any circular refs can happen unless a prog can somehow prevent its own
> detachment.
>
> Could be mis-remembering though.
>
> Thanks,
> Daniel