linux-kernel - Re: [RFC PATCH v4 1/7] libbpf: Extract BTF type remapping logic into helper function

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: <CAEf4BzZffw1sTJUBxwUnhx8XjQNMRf2-e+vUzOfyMqgMTpYsdA@mail.gmail.com>
Date: Wed, 5 Nov 2025 10:20:08 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Eduard Zingerman <eddyz87@...il.com>
Cc: Donglin Peng <dolinux.peng@...il.com>, ast@...nel.org, linux-kernel@...r.kernel.org, 
	bpf@...r.kernel.org, Alan Maguire <alan.maguire@...cle.com>, Song Liu <song@...nel.org>, 
	pengdonglin <pengdonglin@...omi.com>
Subject: Re: [RFC PATCH v4 1/7] libbpf: Extract BTF type remapping logic into
 helper function

On Tue, Nov 4, 2025 at 5:23 PM Eduard Zingerman <eddyz87@...il.com> wrote:
>
> On Tue, 2025-11-04 at 16:57 -0800, Andrii Nakryiko wrote:
> > On Tue, Nov 4, 2025 at 4:36 PM Eduard Zingerman <eddyz87@...il.com> wrote:
> > >
> > > On Tue, 2025-11-04 at 16:11 -0800, Andrii Nakryiko wrote:
> > >
> > > [...]
> > >
> > > > > @@ -3400,6 +3400,37 @@ int btf_ext__set_endianness(struct btf_ext *btf_ext, enum btf_endianness endian)
> > > > >         return 0;
> > > > >  }
> > > > >
> > > > > +static int btf_remap_types(struct btf *btf, struct btf_ext *btf_ext,
> > > > > +                          btf_remap_type_fn visit, void *ctx)
> > > >
> > > > tbh, my goal is to reduce the amount of callback usage within libbpf,
> > > > not add more of it...
> > > >
> > > > I don't like this refactoring. We should convert
> > > > btf_ext_visit_type_ids() into iterators, have btf_field_iter_init +
> > > > btf_field_iter_next usable in for_each() form, and not try to reuse 5
> > > > lines of code. See my comments in the next patch.
> > >
> > > Remapping types is a concept.
> > > I hate duplicating code for concepts.
> > > Similarly, having patch #3 == patch #5 and patch #4 == patch #6 is
> > > plain ugly. Just waiting for a bug because we changed the one but
> > > forgot to change another in a year or two.
> >
> > Tricky and evolving part (iterating all type ID fields) is abstracted
> > behind the iterator (and we should do the same for btf_ext). Iterating
> > types is not something tricky or requiring constant maintenance.
> >
> > Same for binary search, I don't see why we'd need to adjust it. So no,
> > I don't want to share code between kernel and libbpf just to reuse
> > binary search implementation, sorry.
>
> <rant>
>
> Sure binary search is trivial, but did you count how many times you
> asked people to re-implement binary search as in [1]?

Exact match binary search can be called trivial, yes. Lower/upper
bound binary search looks deceivingly simple, but it requires
attention to every single line of code. But the end result is simple
and straightforward, yes.

I'm not sure what point you are trying to make, though. Yes, I've
asked people many times to implement upper/lower bound binary search
similarly to the one in find_linfo(), because usually people have
various unnecessary checks, keeping track not just of bounds, but also
remembering some element that we know satisfied the condition at some
point before, etc. It's not elegant, harder to reason about, and can
be done more succinctly.

You don't like that I ask people to improve implementation? You don't
like the implementation itself? Or are you suggesting that we should
add a "generic" C implementation of lower_bound/upper_bound and use
callbacks for comparison logic? What are you ranting about, exactly?

As I said, once binary search (of whatever kind, bounds or exact) is
written for something like this, it doesn't have to ever be modified.
I don't see this as a maintainability hurdle at all. But sharing code
between libbpf and kernel is something to be avoided. Look at #ifdef
__KERNEL__ sections of relo_core.c as one reason why.

>
> [1] https://elixir.bootlin.com/linux/v6.18-rc4/source/kernel/bpf/verifier.c#L2952
>
> </rant>