Re: bpf: Question about odd BPF verifier behaviour

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 27, 2023 at 9:48 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Mon, 2023-02-27 at 11:31 -0800, Andrii Nakryiko wrote:
> [...]
> > > > I'd start with understanding what BTF and DWARF differences are
> > > > causing the issue before trying to come up with the fix. For that we
> > > > don't even need config or repro steps, it should be enough to share
> > > > vmlinux with BTF and DWARF, and start from there.
> > > >
> > >
> > > Yes, I suspect that there is some kind of unanticipated
> > > anomaly for some DWARF encoding for some kind of objects,
> > > just need to find the root for the diverging type hierarchies.
> > >
> > > > But I'm sure Eduard is on top of this already (especially that he can
> > > > repro the issue now).
> > >
> > > I'm working on it, nothing to report yet, but I'm working on it.
> > >
> >
> > Thanks, please keep us posted!
>
> It is interesting how everything is interconnected. The patch for
> pahole below happens to help. I prepared it last week while working on
> new DWARF encoding scheme for btf_type_tag.
>
> I still need to track down which "unspecified_type" entries caused the
> issue in this particular case. Will post an update tomorrow.
>
> Meanwhile, Matt, KP, could you please verify the patch on your side?
> It is for the "next" branch of pahole.
>
> ---
>
> From 09fac63ca08e25aea499f827283b07cc87a7daab Mon Sep 17 00:00:00 2001
> From: Eduard Zingerman <eddyz87@xxxxxxxxx>
> Date: Tue, 21 Feb 2023 19:23:00 +0200
> Subject: [PATCH] dwarf_loader: Fix for BTF id drift caused by adding
>  unspecified types
>
> Recent changes to handle unspecified types (see [1]) cause BTF ID drift.
>
> Specifically, the intent of commits [2], [3] and [4] is to render
> references to unspecified types as void type.
> However, as a consequence:
> - in `die__process_unit()` call to `cu__add_tag()` allocates `small_id`
>   for unspecified type tags and adds these tags to `cu->types_table`;
> - `btf_encoder__encode_tag()` skips generation of BTF entries for
>   `DW_TAG_unspecified_type` tags.
>
> Such logic causes ID drift if unspecified type is not the last type
> processed for compilation unit. `small_id` of each type following
> unspecified type in the `cu->types_table` would have its BTF id off by -1.
> Thus renders references established on recode phase invalid.
>
> This commit reverts `unspecified_type` id/tag tracking, instead:
> - `small_id` for unspecified type tags is set to 0, thus reference to
>   unspecified type tag would render BTF id of a `void` on recode phase;
> - unspecified type tags are not added to `cu->types_table`.
>
> [1] https://lore.kernel.org/all/Y0R7uu3s%2FimnvPzM@xxxxxxxxxx/
> [2] bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> [3] cffe5e1f75e1 ("core: Record if a CU has a DW_TAG_unspecified_type")
> [4] 75e0fe28bb02 ("core: Add DW_TAG_unspecified_type to tag__is_tag_type() set")
>
> Fixes: bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>

Tested-by: KP Singh <kpsingh@xxxxxxxxxx>
Reported-by: Matt Bobrowski <mattbobrowski@xxxxxxxxxx>

Thank you so much Eduard, this worked:

* No duplicate BTF ID warnings
* No 15 minute BTF ID generation
* Matt's reproducer loads successfully.

I had a sneaky suspicion that it was these unspecified types, which is
why my hacky patch which got unspecified types out of the way got
things to *mostly* work.




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux