On Fri, Aug 23, 2024 at 4:23 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Wed, 2024-08-14 at 11:54 -0700, Andrii Nakryiko wrote:
>
> [...]
>
> > Andrii Nakryiko (10):
> > lib/buildid: harden build ID parsing logic
> > lib/buildid: add single folio-based file reader abstraction
> > lib/buildid: take into account e_phoff when fetching program headers
> > lib/buildid: remove single-page limit for PHDR search
> > lib/buildid: rename build_id_parse() into build_id_parse_nofault()
> > lib/buildid: implement sleepable build_id_parse() API
> > lib/buildid: don't limit .note.gnu.build-id to the first page in ELF
>
> Never worked with lib/buildid before, so not sure how valuable my input is.
> Anyways:
> - I compared the resulting parser with ELF specification and available
> documentation for buildid, all seems correct.
> (with a small caveat that ELF defines Elf{32,64}_Ehdr->e_ehsize field
> to encode actual size of the elf header, and e_phentsize
> to encode actual size of the program header.
> Parser uses sizeof(Elf{32,64}_{Ehdr,Phdr}) instead,
> and this is how it was before, so probably does not matter).
>
> - The `freader` abstraction nicely hides away difference between
> sleepable and non-sleepable contexts.
> (with a caveat, that freader_get_folio() uses read_cache_folio()
> which is documented as expecting mapping->invalidate_lock to be held.
> I assume that this is true for vma's passed to build_id_parse(), right?)
>
> For what it's worth, full patch-set looks good to me.
>
> Reviewed-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
Thank you for the review.
The patch set looks good to me as well, but I think it needs
a bit more Acks to land it through bpf-next.
Andrew,
since lib/ is under your supervision, please review and hopefully ack.
Matthew,
since you commented on the previous version pls double check
that patch 2 plus patch 6 make the right use of folio apis.
