Re: [PATCH v8 bpf-next 03/10] libbpf: use kind layout to compute an unknown kind size

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2025-12-16 at 13:11 -0800, Andrii Nakryiko wrote:
> On Tue, Dec 16, 2025 at 11:58 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Tue, 2025-12-16 at 11:42 -0800, Andrii Nakryiko wrote:
> > > On Tue, Dec 16, 2025 at 7:00 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> > > > 
> > > > On 16/12/2025 06:07, Eduard Zingerman wrote:
> > > > > On Mon, 2025-12-15 at 09:17 +0000, Alan Maguire wrote:
> > > > > 
> > > > > [...]
> > > > > 
> > > > > > @@ -395,8 +416,7 @@ static int btf_type_size(const struct btf_type *t)
> > > > > >      case BTF_KIND_DECL_TAG:
> > > > > >              return base_size + sizeof(struct btf_decl_tag);
> > > > > >      default:
> > > > > > -            pr_debug("Unsupported BTF_KIND:%u\n", btf_kind(t));
> > > > > > -            return -EINVAL;
> > > > > > +            return btf_type_size_unknown(btf, t);
> > > > > >      }
> > > > > >  }
> > > > > > 
> > > > > 
> > > > > That's a matter of personal preference, of-course, but it seems to me
> > > > > that using `kind_layouts` table from your next patch for size
> > > > > computation for all kinds would be a bit more elegant.
> > > > > 
> > > > > Also, a question, should BTF validation check sizes for known kinds
> > > > > and reject kind layout sections if those sizes differ from expected?
> > > > > 
> > > > 
> > > > yeah, I'd say we'd need your second suggestion for the first to be safe,
> > > > and it seems worthwhile doing both I think. Thanks!
> > > 
> > > ... but we will just blindly trust layout for unknown kinds, though?
> > > So it's a bit inconsistent. I'd say let's keep it simple and don't
> > > overdo the checking? btf_sanity_check() will validate that all known
> > > kinds are well-formed, isn't that sufficient to ensure that subsequent
> > > use of BTF data in libbpf won't crash? If some tool generated a subtly
> > > invalid layout section which otherwise preserves BTF data
> > > correctness... I don't know, this seems fine. The goal of sanity
> > > checking is just to prevent more checks in all different places that
> > > will subsequently rely on IDs being valid, and stuff like that. If
> > > layout info is wrong for known kinds, so be it, we are not using that
> > > information anyways.
> > 
> > Ignoring layout information for known kinds can lead to weird
> > scenarios: e.g. suppose type size is N, but kind layout specifies that
> > it is M > N, and the tool generating BTF uses M to actually layout the
> > binary data. We are being a bit inconsistent with such encoding.
> 
> Who are "we" here? The tool that emitted incorrect layout information
> -- yep, for sure. (But that shouldn't happen for correct BTF.)
> 
> We do btf_sanity_check() upfront to minimize various sanity checks
> spread out in libbpf code when using BTF data later on. But the goal
> there is not really to check "100% standards conformance" of whatever
> BTF we are working with. Kernel is way more concerned about validity
> and not letting anything unexpected get through, but libbpf is in user
> space and it's a bit different approach there.
> 
> As long as BTF looks structurally sound (btf_sanity_check), it should
> be fine for our needs. If BTF is corrupted or just uses invalid ID, or
> whatnot, it will eventually fail somewhere, most probably. But the
> goal is not to have NULL derefs and stuff like that.

Introducing layout info into format provides an alternative definition
for structural soundness. E.g. some types or vlen elements can have
padding in the end all of a sudden.

Using this info for some types but not the others is inconsistent.
Given that BTF rewrites would only be unsound in presence of unknown
types the whole feature looks questionable to me.

> Basically what I'm trying to say is that if some tool intentionally or
> unintentionally generates invalid layout information, but the rest of
> data is valid, and libbpf doesn't look at layout and otherwise makes
> fine use of BTF. Then that's fine with me, that BTF fulfills its duty
> as far as libbpf is concerned. Libbpf here is just a consumer that
> tries to be as permissive (unlike kernel) as possible, while not
> leading to a process crash.
> 
> With double checking that layout info matches our implicit knowledge
> of type layout we are starting to move into BTF verification a bit,
> IMO. And I think that's misleading and unnecessary.