On Wed, Aug 7, 2024 at 5:51 PM Jann Horn <jannh@xxxxxxxxxx> wrote:
>
> On Thu, Aug 8, 2024 at 1:40 AM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote:
> > Add freader abstraction that transparently manages fetching and local
> > mapping of the underlying file page(s) and provides a simple direct data
> > access interface.
> >
> > freader_fetch() is the only and single interface necessary. It accepts
> > file offset and desired number of bytes that should be accessed, and
> > will return a kernel mapped pointer that caller can use to dereference
> > data up to requested size. Requested size can't be bigger than the size
> > of the extra buffer provided during initialization (because, worst case,
> > all requested data has to be copied into it, so it's better to flag
> > wrongly sized buffer unconditionally, regardless if requested data range
> > is crossing page boundaries or not).
> >
> > If folio is not paged in, or some of the conditions are not satisfied,
> > NULL is returned and more detailed error code can be accessed through
> > freader->err field. This approach makes the usage of freader_fetch()
> > cleaner.
> >
> > To accommodate accessing file data that crosses folio boundaries, user
> > has to provide an extra buffer that will be used to make a local copy,
> > if necessary. This is done to maintain a simple linear pointer data
> > access interface.
> >
> > We switch existing build ID parsing logic to it, without changing or
> > lifting any of the existing constraints, yet. This will be done
> > separately.
> >
> > Given existing code was written with the assumption that it's always
> > working with a single (first) page of the underlying ELF file, logic
> > passes direct pointers around, which doesn't really work well with
> > freader approach and would be limiting when removing the single page (folio)
> > limitation. So we adjust all the logic to work in terms of file offsets.
> >
> > There is also a memory buffer-based version (freader_init_from_mem())
> > for cases when desired data is already available in kernel memory. This
> > is used for parsing vmlinux's own build ID note. In this mode assumption
> > is that provided data starts at "file offset" zero, which works great
> > when parsing ELF notes sections, as all the parsing logic is relative to
> > note section's start.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> [...]
> > +static int freader_get_folio(struct freader *r, loff_t file_off)
> > +{
> > + /* check if we can just reuse current folio */
> > + if (r->folio && file_off >= r->folio_off &&
> > + file_off < r->folio_off + folio_size(r->folio))
> > + return 0;
> > +
> > + freader_put_folio(r);
> > +
> > + r->folio = filemap_get_folio(r->mapping, file_off >> PAGE_SHIFT);
> > + if (IS_ERR(r->folio) || !folio_test_uptodate(r->folio)) {
>
> Can you also fix the uptodate stuff in patch 1, or in another
> stable-backportable patch? (I guess alternatively we can fix it with a
> custom patch in stable after it's been fixed in mainline, but that
> feels kinda hacky to me.)
Ah, right, damn, forgot to do that also in patch #1... I'll fix it
locally so I don't forget, will give others time to review and provide
feedback or acks, so I don't spam the list too much.
