On Tue, Sep 3, 2024 at 3:39 PM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Thu, Aug 29, 2024 at 10:42 AM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote: > > > > The goal of this patch set is to extend existing ELF build ID parsing logic, > > currently mostly used by BPF subsystem, with support for working in sleepable > > mode in which memory faults are allowed and can be relied upon to fetch > > relevant parts of ELF file to find and fetch .note.gnu.build-id information. > > > > This is useful and important for BPF subsystem itself, but also for > > PROCMAP_QUERY ioctl(), built atop of /proc/<pid>/maps functionality (see [0]), > > which makes use of the same build_id_parse() functionality. PROCMAP_QUERY is > > always called from sleepable user process context, so it doesn't have to > > suffer from current restrictions of build_id_parse() which are due to the NMI > > context assumption. > > > > Along the way, we harden the logic to avoid TOCTOU, overflow, out-of-bounds > > access problems. This is the very first patch, which can be backported to > > older releases, if necessary. > > > > We also lift existing limitations of only working as long as ELF program > > headers and build ID note section is contained strictly within the very first > > page of ELF file. > > > > We achieve all of the above without duplication of logic between sleepable and > > non-sleepable modes through freader abstraction that manages underlying folio > > from page cache (on demand) and gives a simple to use direct memory access > > interface. With that, single page restrictions and adding sleepable mode > > support is rather straightforward. > > > > We also extend existing set of BPF selftests with a few tests targeting build > > ID logic across sleepable and non-sleepabe contexts (we utilize sleepable and > > non-sleepable uprobes for that). > > > > [0] https://lore.kernel.org/linux-mm/20240627170900.1672542-4-andrii@xxxxxxxxxx/ > > > > v6->v7: > > - added filemap_invalidate_{lock,unlock}_shared() around read_cache_folio > > and kept Eduard's Reviewed-by (Eduard); > > v5->v6: > > - use local phnum variable in get_build_id_32() (Jann); > > - switch memcmp() instead of strcmp() in parse_build_id() (Jann); > > v4->v5: > > - pass proper file reference to read_cache_folio() (Shakeel); > > - fix another potential overflow due to two u32 additions (Andi); > > - add PageUptodate() check to patch #1 (Jann); > > v3->v4: > > - fix few more potential overflow and out-of-bounds access issues (Andi); > > - use purely folio-based implementation for freader (Matthew); > > Ok, so I'm not sure what one needs to do to get Matthew's attention > nowadays, but hopefully yet another ping might do the trick. > > Matthew, > > Can you please take another look and provide your ack or nack? I did > the conversion to folio as you requested. It would be nice if you can > give me a courtesy of acking my patch set, if there is nothing wrong > with it, so it can finally go in. Looks like no further comments from Matthew or anyone else. I'll take another look through the set before applying to bpf-next.
