Re: [PATCH v6 bpf-next 00/10] Harden and extend ELF build ID parsing logic

[Eduard Zingerman <eddyz87@gmail.com>]

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@gmail.com>

[...]