[ Upstream commit 5ac9b4e935dfc6af41eee2ddc21deb5c36507a9f ]
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.)
We need to handle this special case gracefully in build ID fetching code. Return -EFAULT whenever secretmem file is passed to build_id_parse() family of APIs. Original report and repro can be found in [0].
[0] https://lore.kernel.org/bpf/ZwyG8Uro%2FSyTXAni@ly-workstation/
Fixes: de3ec364c3c3 ("lib/buildid: add single folio-based file reader abstraction") Reported-by: Yi Lai yi1.lai@intel.com Suggested-by: Shakeel Butt shakeel.butt@linux.dev Signed-off-by: Andrii Nakryiko andrii@kernel.org Signed-off-by: Daniel Borkmann daniel@iogearbox.net Acked-by: Shakeel Butt shakeel.butt@linux.dev Link: https://lore.kernel.org/bpf/20241017175431.6183-A-hca@linux.ibm.com Link: https://lore.kernel.org/bpf/20241017174713.2157873-1-andrii@kernel.org [ Chen Linxuan: backport same logic without folio-based changes ] Cc: stable@vger.kernel.org Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event") Signed-off-by: Chen Linxuan chenlinxuan@deepin.org --- v1 -> v2: use vma_is_secretmem() instead of directly checking vma->vm_file->f_op == &secretmem_fops --- lib/buildid.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/lib/buildid.c b/lib/buildid.c index 9fc46366597e..34315d09b544 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/secretmem.h>
#define BUILD_ID 3
@@ -157,6 +158,10 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id, if (!vma->vm_file) return -EINVAL;
+ /* reject secretmem */ + if (vma_is_secretmem(vma)) + return -EFAULT; + page = find_get_page(vma->vm_file->f_mapping, 0); if (!page) return -EFAULT; /* page not mapped */
On Mon, Mar 17, 2025 at 09:16:04AM +0800, Chen Linxuan wrote:
[ Upstream commit 5ac9b4e935dfc6af41eee2ddc21deb5c36507a9f ]
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.)
We need to handle this special case gracefully in build ID fetching code. Return -EFAULT whenever secretmem file is passed to build_id_parse() family of APIs. Original report and repro can be found in [0].
[0] https://lore.kernel.org/bpf/ZwyG8Uro%2FSyTXAni@ly-workstation/
Fixes: de3ec364c3c3 ("lib/buildid: add single folio-based file reader abstraction") Reported-by: Yi Lai yi1.lai@intel.com Suggested-by: Shakeel Butt shakeel.butt@linux.dev Signed-off-by: Andrii Nakryiko andrii@kernel.org Signed-off-by: Daniel Borkmann daniel@iogearbox.net Acked-by: Shakeel Butt shakeel.butt@linux.dev Link: https://lore.kernel.org/bpf/20241017175431.6183-A-hca@linux.ibm.com Link: https://lore.kernel.org/bpf/20241017174713.2157873-1-andrii@kernel.org [ Chen Linxuan: backport same logic without folio-based changes ] Cc: stable@vger.kernel.org Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event") Signed-off-by: Chen Linxuan chenlinxuan@deepin.org
v1 -> v2: use vma_is_secretmem() instead of directly checking vma->vm_file->f_op == &secretmem_fops
lib/buildid.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/lib/buildid.c b/lib/buildid.c index 9fc46366597e..34315d09b544 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/secretmem.h> #define BUILD_ID 3 @@ -157,6 +158,10 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id, if (!vma->vm_file) return -EINVAL;
- /* reject secretmem */
Why is this comment different from what is in the original commit? Same for your other backports. Please try to keep it as identical to the original whenever possible as we have to maintain this for a very long time.
thanks,
greg k-h
Greg KH greg@kroah.com 于2025年3月17日周一 12:20写道：
On Mon, Mar 17, 2025 at 09:16:04AM +0800, Chen Linxuan wrote:
[ Upstream commit 5ac9b4e935dfc6af41eee2ddc21deb5c36507a9f ]
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.)
We need to handle this special case gracefully in build ID fetching code. Return -EFAULT whenever secretmem file is passed to build_id_parse() family of APIs. Original report and repro can be found in [0].
[0] https://lore.kernel.org/bpf/ZwyG8Uro%2FSyTXAni@ly-workstation/
Fixes: de3ec364c3c3 ("lib/buildid: add single folio-based file reader abstraction") Reported-by: Yi Lai yi1.lai@intel.com Suggested-by: Shakeel Butt shakeel.butt@linux.dev Signed-off-by: Andrii Nakryiko andrii@kernel.org Signed-off-by: Daniel Borkmann daniel@iogearbox.net Acked-by: Shakeel Butt shakeel.butt@linux.dev Link: https://lore.kernel.org/bpf/20241017175431.6183-A-hca@linux.ibm.com Link: https://lore.kernel.org/bpf/20241017174713.2157873-1-andrii@kernel.org [ Chen Linxuan: backport same logic without folio-based changes ] Cc: stable@vger.kernel.org Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event") Signed-off-by: Chen Linxuan chenlinxuan@deepin.org
v1 -> v2: use vma_is_secretmem() instead of directly checking vma->vm_file->f_op == &secretmem_fops
lib/buildid.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/lib/buildid.c b/lib/buildid.c index 9fc46366597e..34315d09b544 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/secretmem.h>
#define BUILD_ID 3
@@ -157,6 +158,10 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id, if (!vma->vm_file) return -EINVAL;
/* reject secretmem */Why is this comment different from what is in the original commit? Same for your other backports. Please try to keep it as identical to the original whenever possible as we have to maintain this for a very long time.
thanks,
greg k-h
Original comment is in a function named freader_get_folio(), but folio related changes has not been backported yet.
thanks,
Chen Linxuan
On Mon, Mar 17, 2025 at 01:04:41PM +0800, Chen Linxuan wrote:
Greg KH greg@kroah.com 于2025年3月17日周一 12:20写道：
On Mon, Mar 17, 2025 at 09:16:04AM +0800, Chen Linxuan wrote:
[ Upstream commit 5ac9b4e935dfc6af41eee2ddc21deb5c36507a9f ]
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.)
We need to handle this special case gracefully in build ID fetching code. Return -EFAULT whenever secretmem file is passed to build_id_parse() family of APIs. Original report and repro can be found in [0].
[0] https://lore.kernel.org/bpf/ZwyG8Uro%2FSyTXAni@ly-workstation/
Fixes: de3ec364c3c3 ("lib/buildid: add single folio-based file reader abstraction") Reported-by: Yi Lai yi1.lai@intel.com Suggested-by: Shakeel Butt shakeel.butt@linux.dev Signed-off-by: Andrii Nakryiko andrii@kernel.org Signed-off-by: Daniel Borkmann daniel@iogearbox.net Acked-by: Shakeel Butt shakeel.butt@linux.dev Link: https://lore.kernel.org/bpf/20241017175431.6183-A-hca@linux.ibm.com Link: https://lore.kernel.org/bpf/20241017174713.2157873-1-andrii@kernel.org [ Chen Linxuan: backport same logic without folio-based changes ] Cc: stable@vger.kernel.org Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event") Signed-off-by: Chen Linxuan chenlinxuan@deepin.org
v1 -> v2: use vma_is_secretmem() instead of directly checking vma->vm_file->f_op == &secretmem_fops
lib/buildid.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/lib/buildid.c b/lib/buildid.c index 9fc46366597e..34315d09b544 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/secretmem.h>
#define BUILD_ID 3
@@ -157,6 +158,10 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id, if (!vma->vm_file) return -EINVAL;
/* reject secretmem */Why is this comment different from what is in the original commit? Same for your other backports. Please try to keep it as identical to the original whenever possible as we have to maintain this for a very long time.
thanks,
greg k-h
Original comment is in a function named freader_get_folio(), but folio related changes has not been backported yet.
That's fine, but the logic is the same so keep the original code as close as possible. Otherwise it looks like this is a totally different change and we would have to reject it for obvious reasons.
thanks,
greg k-h
Greg KH gregkh@linuxfoundation.org 于2025年3月17日周一 13:15写道：
On Mon, Mar 17, 2025 at 01:04:41PM +0800, Chen Linxuan wrote:
Greg KH greg@kroah.com 于2025年3月17日周一 12:20写道：
On Mon, Mar 17, 2025 at 09:16:04AM +0800, Chen Linxuan wrote:
[ Upstream commit 5ac9b4e935dfc6af41eee2ddc21deb5c36507a9f ]
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.)
We need to handle this special case gracefully in build ID fetching code. Return -EFAULT whenever secretmem file is passed to build_id_parse() family of APIs. Original report and repro can be found in [0].
[0] https://lore.kernel.org/bpf/ZwyG8Uro%2FSyTXAni@ly-workstation/
Fixes: de3ec364c3c3 ("lib/buildid: add single folio-based file reader abstraction") Reported-by: Yi Lai yi1.lai@intel.com Suggested-by: Shakeel Butt shakeel.butt@linux.dev Signed-off-by: Andrii Nakryiko andrii@kernel.org Signed-off-by: Daniel Borkmann daniel@iogearbox.net Acked-by: Shakeel Butt shakeel.butt@linux.dev Link: https://lore.kernel.org/bpf/20241017175431.6183-A-hca@linux.ibm.com Link: https://lore.kernel.org/bpf/20241017174713.2157873-1-andrii@kernel.org [ Chen Linxuan: backport same logic without folio-based changes ] Cc: stable@vger.kernel.org Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event") Signed-off-by: Chen Linxuan chenlinxuan@deepin.org
v1 -> v2: use vma_is_secretmem() instead of directly checking vma->vm_file->f_op == &secretmem_fops
lib/buildid.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/lib/buildid.c b/lib/buildid.c index 9fc46366597e..34315d09b544 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/secretmem.h>
#define BUILD_ID 3
@@ -157,6 +158,10 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id, if (!vma->vm_file) return -EINVAL;
/* reject secretmem */Why is this comment different from what is in the original commit? Same for your other backports. Please try to keep it as identical to the original whenever possible as we have to maintain this for a very long time.
thanks,
greg k-h
Original comment is in a function named freader_get_folio(), but folio related changes has not been backported yet.
That's fine, but the logic is the same so keep the original code as close as possible. Otherwise it looks like this is a totally different change and we would have to reject it for obvious reasons.
thanks,
greg k-h
V3 has been sent.
Thanks, Chen Linxuan
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected. No action required from the submitter.
The upstream commit SHA1 provided is correct: 5ac9b4e935dfc6af41eee2ddc21deb5c36507a9f
WARNING: Author mismatch between patch and upstream commit: Backport author: Chen Linxuanchenlinxuan@deepin.org Commit author: Andrii Nakryikoandrii@kernel.org
Status in newer kernel trees: 6.13.y | Present (exact SHA1) 6.12.y | Present (exact SHA1)
Note: The patch differs from the upstream commit: --- 1: 5ac9b4e935dfc ! 1: c4f7486bd3c69 lib/buildid: Handle memfd_secret() files in build_id_parse() @@ ## Metadata ## -Author: Andrii Nakryiko andrii@kernel.org +Author: Chen Linxuan chenlinxuan@deepin.org
## Commit message ## lib/buildid: Handle memfd_secret() files in build_id_parse()
+ [ Upstream commit 5ac9b4e935dfc6af41eee2ddc21deb5c36507a9f ] + >From memfd_secret(2) manpage:
The memory areas backing the file created with memfd_secret(2) are @@ Commit message Acked-by: Shakeel Butt shakeel.butt@linux.dev Link: https://lore.kernel.org/bpf/20241017175431.6183-A-hca@linux.ibm.com Link: https://lore.kernel.org/bpf/20241017174713.2157873-1-andrii@kernel.org + [ Chen Linxuan: backport same logic without folio-based changes ] + Cc: stable@vger.kernel.org + Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event") + Signed-off-by: Chen Linxuan chenlinxuan@deepin.org
## lib/buildid.c ## @@ @@ lib/buildid.c
#define BUILD_ID 3
-@@ lib/buildid.c: static int freader_get_folio(struct freader *r, loff_t file_off) - - freader_put_folio(r); +@@ lib/buildid.c: int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id, + if (!vma->vm_file) + return -EINVAL;
-+ /* reject secretmem folios created with memfd_secret() */ -+ if (secretmem_mapping(r->file->f_mapping)) ++ /* reject secretmem */ ++ if (vma_is_secretmem(vma)) + return -EFAULT; + - r->folio = filemap_get_folio(r->file->f_mapping, file_off >> PAGE_SHIFT); - - /* if sleeping is allowed, wait for the page, if necessary */ + page = find_get_page(vma->vm_file->f_mapping, 0); + if (!page) + return -EFAULT; /* page not mapped */ ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.6.y | Success | Success |
linux-stable-mirror@lists.linaro.org
-
Chen Linxuan -
Greg KH -
Greg KH -
Sasha Levin