lib/buildid: use __kernel_read() for sleepable context
Subsystems (5)
PATCH v3 1/1 lib/buildid: use __kernel_read() for sleepable context
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.
---
Changes since v2:
- Single call to __kernel_read instead of looping (Matthew Wilcox).
Changes since v1:
- Fix handling of buf in freader_fetch_sync as pointed out by Andrii.
lib/buildid.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-)
lib/buildid.c +20 -12
@@ -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
@@ -46,20 +47,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 +87,24 @@ 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) { +ssize_t ret; + + ret =__kernel_read (r->file, r->buf, sz, &file_off); +if (ret != sz) { + r->err = (ret <0 ) ? ret : -EIO ; +return NULL ; + } +return r->buf; + } +/* fetch or reuse folio for given file offset */ r->err =freader_get_folio (r, file_off);if (r->err)
Signature · 1 line
2.47.3
Quoted 18 lines · Shakeel Butt
For the sleepable context, convert freader to use
__kernel_read()instead of direct page cache access viaread_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()withIOCB_NOWAITwork 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.
Quoted 6 lines
Reported-by [email protected]Fixes ad41251c290d ("lib/buildid: implement sleepable build_id_parse() API")Reviewed-by Christoph Hellwig <[email protected]>Signed-off-by Shakeel Butt <[email protected]>---