On 10/17/24 12:16 AM, Andrii Nakryiko wrote:
From memfd_secret(2) manpage:
The memory areas backing the file created with memfd_secret(2) are
visible only to the processes that have access to the file descriptor.
The memory region is removed from the kernel page tables and only the
page tables of the processes holding the file descriptor map the
corresponding physical memory. (Thus, the pages in the region can't be
accessed by the kernel itself, so that, for example, pointers to the
region can't be passed to system calls.)
So folios backed by such secretmem files are not mapped into kernel
address space and shouldn't be accessed, in general.
To make this a bit more generic of a fix and prevent regression in the
future for similar special mappings, do a generic check of whether the
folio we got is mapped with kernel_page_present(), as suggested in [1].
This will handle secretmem, and any future special cases that use
a similar approach.
Original report and repro can be found in [0].
[0] https://lore.kernel.org/bpf/ZwyG8Uro%2FSyTXAni@ly-workstation/
[1] https://lore.kernel.org/bpf/CAJD7tkbpEMx-eC4A-z8Jm1ikrY_KJVjWO+mhhz1_fni4x+COKw@xxxxxxxxxxxxxx/
Reported-by: Yi Lai <yi1.lai@xxxxxxxxx>
Suggested-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx>
Fixes: de3ec364c3c3 ("lib/buildid: add single folio-based file reader abstraction")
Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
---
lib/buildid.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/buildid.c b/lib/buildid.c
index 290641d92ac1..90df64fd64c1 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/set_memory.h>
#define BUILD_ID 3
@@ -74,7 +75,9 @@ static int freader_get_folio(struct freader *r, loff_t file_off)
filemap_invalidate_unlock_shared(r->file->f_mapping);
}
- if (IS_ERR(r->folio) || !folio_test_uptodate(r->folio)) {
+ if (IS_ERR(r->folio) ||
+ !kernel_page_present(&r->folio->page) ||
+ !folio_test_uptodate(r->folio)) {
BPF CI fails to build this on s390 (+ Ilya & others):
[...]
CC crypto/ctr.o
../lib/buildid.c: In function ‘freader_get_folio’:
../lib/buildid.c:79:14: error: implicit declaration of function ‘kernel_page_present’ [-Werror=implicit-function-declaration]
79 | !kernel_page_present(&r->folio->page) ||
| ^~~~~~~~~~~~~~~~~~~
CC net/sched/cls_bpf.o
[...]
Interestingly, the generic kernel_page_present() which returns true under
!CONFIG_ARCH_HAS_SET_DIRECT_MAP is not used here since s390 selects the
CONFIG_ARCH_HAS_SET_DIRECT_MAP, but does not provide an implementation of
the function compared to the others which select it (x86, arm64, riscv).
Relevant commit is 0490d6d7ba0a ("s390/mm: enable ARCH_HAS_SET_DIRECT_MAP").
if (!IS_ERR(r->folio))
folio_put(r->folio);
r->folio = NULL;
