Re: [PATCH] lib/buildid: use __kernel_read() for sleepable context

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Wed, Dec 17, 2025 at 4:59 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> For the sleepable context, convert freader to use __kernel_read()
> instead of direct page cache access via read_cache_folio(). This
> simplifies the faultable code path by using the standard kernel file
> reading interface which handles all the complexity of reading file data.
>
> At the moment we are not changing the code for non-sleepable context
> which uses filemap_get_folio() and only succeeds if the target folios
> are already in memory and up-to-date. The reason is to keep the patch
> simple and easier to backport to stable kernels.
>
> Syzbot repro does not crash the kernel anymore and the selftests run
> successfully.
>
> In the follow up we will make __kernel_read() with IOCB_NOWAIT work for
> non-sleepable contexts. In addition, I would like to replace the
> secretmem check with a more generic approach and will add fstest for the
> buildid code.
>
> Reported-by: syzbot+09b7d050e4806540153d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=09b7d050e4806540153d
> Fixes: ad41251c290d ("lib/buildid: implement sleepable build_id_parse() API")
> Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
> ---
>  lib/buildid.c | 47 +++++++++++++++++++++++++++++++++++------------
>  1 file changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/lib/buildid.c b/lib/buildid.c
> index aaf61dfc0919..e7e258532720 100644
> --- a/lib/buildid.c
> +++ b/lib/buildid.c
> @@ -5,6 +5,7 @@
>  #include <linux/elf.h>
>  #include <linux/kernel.h>
>  #include <linux/pagemap.h>
> +#include <linux/fs.h>
>  #include <linux/secretmem.h>
>
>  #define BUILD_ID 3
> @@ -37,6 +38,29 @@ static void freader_put_folio(struct freader *r)
>         r->folio = NULL;
>  }
>
> +/*
> + * Data is read directly into r->buf. Returns pointer to the buffer
> + * on success, NULL on failure with r->err set.
> + */
> +static const void *freader_fetch_sync(struct freader *r, loff_t file_off, size_t sz)
> +{
> +       ssize_t ret;
> +       loff_t pos = file_off;
> +       char *buf = r->buf;
> +
> +       do {
> +               ret = __kernel_read(r->file, r->buf, sz, &pos);

r->buf -> buf

(and please add [PATCH bpf] for next revision)

pw-bot: cr


> +               if (ret <= 0) {
> +                       r->err = ret ?: -EIO;
> +                       return NULL;
> +               }
> +               buf += ret;
> +               sz -= ret;
> +       } while (sz > 0);
> +
> +       return r->buf;
> +}
> +
>  static int freader_get_folio(struct freader *r, loff_t file_off)
>  {
>         /* check if we can just reuse current folio */
> @@ -46,20 +70,9 @@ static int freader_get_folio(struct freader *r, loff_t file_off)
>
>         freader_put_folio(r);
>
> -       /* reject secretmem folios created with memfd_secret() */
> -       if (secretmem_mapping(r->file->f_mapping))
> -               return -EFAULT;
> -
> +       /* only use page cache lookup - fail if not already cached */
>         r->folio = filemap_get_folio(r->file->f_mapping, file_off >> PAGE_SHIFT);
>
> -       /* if sleeping is allowed, wait for the page, if necessary */
> -       if (r->may_fault && (IS_ERR(r->folio) || !folio_test_uptodate(r->folio))) {
> -               filemap_invalidate_lock_shared(r->file->f_mapping);
> -               r->folio = read_cache_folio(r->file->f_mapping, file_off >> PAGE_SHIFT,
> -                                           NULL, r->file);
> -               filemap_invalidate_unlock_shared(r->file->f_mapping);
> -       }
> -
>         if (IS_ERR(r->folio) || !folio_test_uptodate(r->folio)) {
>                 if (!IS_ERR(r->folio))
>                         folio_put(r->folio);
> @@ -97,6 +110,16 @@ const void *freader_fetch(struct freader *r, loff_t file_off, size_t sz)
>                 return r->data + file_off;
>         }
>
> +       /* reject secretmem folios created with memfd_secret() */
> +       if (secretmem_mapping(r->file->f_mapping)) {
> +               r->err = -EFAULT;
> +               return NULL;
> +       }
> +
> +       /* use __kernel_read() for sleepable context */
> +       if (r->may_fault)
> +               return freader_fetch_sync(r, file_off, sz);
> +
>         /* fetch or reuse folio for given file offset */
>         r->err = freader_get_folio(r, file_off);
>         if (r->err)
> --
> 2.47.3
>