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

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

[Shakeel Butt]

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);
+		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

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

[Christoph Hellwig]

On Wed, Dec 17, 2025 at 04:58:18PM -0800, Shakeel Butt 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.

Getting the code further away from messing with internals is good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

I do not think making IOCB_NOWAIT never wait is feasily, though for the
next step.

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

[Andrii Nakryiko]

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
>