[PATCH stable 6.6] lib/buildid: Handle memfd_secret() files in build_id_parse()

[PATCH stable 6.6] lib/buildid: Handle memfd_secret() files in build_id_parse()

[Chen Linxuan]

Backport of a similar change from commit 5ac9b4e935df ("lib/buildid:
Handle memfd_secret() files in build_id_parse()") to address an issue
where accessing secret memfd contents through build_id_parse() would
trigger faults.

Original report and repro can be found in [0].

  [0] https://lore.kernel.org/bpf/ZwyG8Uro%2FSyTXAni@ly-workstation/

This repro will cause BUG: unable to handle kernel paging request in
build_id_parse in 5.15/6.1/6.6.

Some other discussions can be found in [1].

  [1] https://lore.kernel.org/bpf/20241104175256.2327164-1-jolsa@kernel.org/T/#u

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 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/buildid.c b/lib/buildid.c
index 9fc46366597e..9db35305f257 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,12 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
 	if (!vma->vm_file)
 		return -EINVAL;
 
+#ifdef CONFIG_SECRETMEM
+	/* reject secretmem folios created with memfd_secret() */
+	if (vma->vm_file->f_mapping->a_ops == &secretmem_aops)
+		return -EFAULT;
+#endif
+
 	page = find_get_page(vma->vm_file->f_mapping, 0);
 	if (!page)
 		return -EFAULT;	/* page not mapped */
-- 
2.48.1

Re: [PATCH stable 6.6] lib/buildid: Handle memfd_secret() files in build_id_parse()

[Greg KH]

On Tue, Mar 11, 2025 at 06:05:55PM +0800, Chen Linxuan wrote:
> Backport of a similar change from commit 5ac9b4e935df ("lib/buildid:
> Handle memfd_secret() files in build_id_parse()") to address an issue
> where accessing secret memfd contents through build_id_parse() would
> trigger faults.
> 
> Original report and repro can be found in [0].
> 
>   [0] https://lore.kernel.org/bpf/ZwyG8Uro%2FSyTXAni@ly-workstation/
> 
> This repro will cause BUG: unable to handle kernel paging request in
> build_id_parse in 5.15/6.1/6.6.
> 
> Some other discussions can be found in [1].
> 
>   [1] https://lore.kernel.org/bpf/20241104175256.2327164-1-jolsa@kernel.org/T/#u
> 
> Cc: stable@vger.kernel.org
> Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event")
> Signed-off-by: Chen Linxuan <chenlinxuan@deepin.org>

You dropped all the original signed-off-by and changelog text.  Just
provide a backport with all of the original information, and then if you
had to do something "different", put that in the signed-off-by area.
THere are loads of examples on the list for how that was done.

thanks,

greg k-h

Re: [PATCH stable 6.6] lib/buildid: Handle memfd_secret() files in build_id_parse()

[Chen Linxuan]

Greg KH <gregkh@linuxfoundation.org> 于2025年3月11日周二 19:14写道：
>
> On Tue, Mar 11, 2025 at 06:05:55PM +0800, Chen Linxuan wrote:
> > Backport of a similar change from commit 5ac9b4e935df ("lib/buildid:
> > Handle memfd_secret() files in build_id_parse()") to address an issue
> > where accessing secret memfd contents through build_id_parse() would
> > trigger faults.
> >
> > Original report and repro can be found in [0].
> >
> >   [0] https://lore.kernel.org/bpf/ZwyG8Uro%2FSyTXAni@ly-workstation/
> >
> > This repro will cause BUG: unable to handle kernel paging request in
> > build_id_parse in 5.15/6.1/6.6.
> >
> > Some other discussions can be found in [1].
> >
> >   [1] https://lore.kernel.org/bpf/20241104175256.2327164-1-jolsa@kernel.org/T/#u
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event")
> > Signed-off-by: Chen Linxuan <chenlinxuan@deepin.org>
>
> You dropped all the original signed-off-by and changelog text.  Just

The original commit is based on commit de3ec364c3c3 ("lib/buildid: add
single folio-based file reader abstraction"). `git cherry-pick` result lots of
conflicts. So I rewrite same logic on old code.

> provide a backport with all of the original information, and then if you
> had to do something "different", put that in the signed-off-by area.
> THere are loads of examples on the list for how that was done.

Do you means that I should:

1. Run git cherry-pick 5ac9b4e935df on stable branches;
2. Resolve conflicts by drop all changes then apply changes
   as I send in this email;
3. Note why content of this patch is different from the original
   one after original signed-off-by area, but before the --- separator.

I am not familiar with contributing to stable kernel tree.
Sorry for bothering.

>
> thanks,
>
> greg k-h
>
>

Re: [PATCH stable 6.6] lib/buildid: Handle memfd_secret() files in build_id_parse()

[Andrii Nakryiko]

On Tue, Mar 11, 2025 at 8:03 PM Chen Linxuan <chenlinxuan@deepin.org> wrote:
>
> Greg KH <gregkh@linuxfoundation.org> 于2025年3月11日周二 19:14写道：
> >
> > On Tue, Mar 11, 2025 at 06:05:55PM +0800, Chen Linxuan wrote:
> > > Backport of a similar change from commit 5ac9b4e935df ("lib/buildid:
> > > Handle memfd_secret() files in build_id_parse()") to address an issue
> > > where accessing secret memfd contents through build_id_parse() would
> > > trigger faults.
> > >
> > > Original report and repro can be found in [0].
> > >
> > >   [0] https://lore.kernel.org/bpf/ZwyG8Uro%2FSyTXAni@ly-workstation/
> > >
> > > This repro will cause BUG: unable to handle kernel paging request in
> > > build_id_parse in 5.15/6.1/6.6.
> > >
> > > Some other discussions can be found in [1].
> > >
> > >   [1] https://lore.kernel.org/bpf/20241104175256.2327164-1-jolsa@kernel.org/T/#u
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event")
> > > Signed-off-by: Chen Linxuan <chenlinxuan@deepin.org>
> >
> > You dropped all the original signed-off-by and changelog text.  Just
>
> The original commit is based on commit de3ec364c3c3 ("lib/buildid: add
> single folio-based file reader abstraction"). `git cherry-pick` result lots of
> conflicts. So I rewrite same logic on old code.
>

Yep, for the purpose of fixing the issue, I wouldn't try to backport
my folio-based changes to lib/buildid. What you are doing here (an
equivalent direct check for secretmem) makes sense to me.

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> > provide a backport with all of the original information, and then if you
> > had to do something "different", put that in the signed-off-by area.
> > THere are loads of examples on the list for how that was done.
>
> Do you means that I should:
>
> 1. Run git cherry-pick 5ac9b4e935df on stable branches;
> 2. Resolve conflicts by drop all changes then apply changes
>    as I send in this email;
> 3. Note why content of this patch is different from the original
>    one after original signed-off-by area, but before the --- separator.
>
> I am not familiar with contributing to stable kernel tree.
> Sorry for bothering.
>
> >
> > thanks,
> >
> > greg k-h
> >
> >

Re: [PATCH stable 6.6] lib/buildid: Handle memfd_secret() files in build_id_parse()

[Greg KH]

On Wed, Mar 12, 2025 at 11:03:18AM +0800, Chen Linxuan wrote:
> Greg KH <gregkh@linuxfoundation.org> 于2025年3月11日周二 19:14写道：
> >
> > On Tue, Mar 11, 2025 at 06:05:55PM +0800, Chen Linxuan wrote:
> > > Backport of a similar change from commit 5ac9b4e935df ("lib/buildid:
> > > Handle memfd_secret() files in build_id_parse()") to address an issue
> > > where accessing secret memfd contents through build_id_parse() would
> > > trigger faults.
> > >
> > > Original report and repro can be found in [0].
> > >
> > >   [0] https://lore.kernel.org/bpf/ZwyG8Uro%2FSyTXAni@ly-workstation/
> > >
> > > This repro will cause BUG: unable to handle kernel paging request in
> > > build_id_parse in 5.15/6.1/6.6.
> > >
> > > Some other discussions can be found in [1].
> > >
> > >   [1] https://lore.kernel.org/bpf/20241104175256.2327164-1-jolsa@kernel.org/T/#u
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event")
> > > Signed-off-by: Chen Linxuan <chenlinxuan@deepin.org>
> >
> > You dropped all the original signed-off-by and changelog text.  Just
> 
> The original commit is based on commit de3ec364c3c3 ("lib/buildid: add
> single folio-based file reader abstraction"). `git cherry-pick` result lots of
> conflicts. So I rewrite same logic on old code.

Then keep the original commit message, tell us what the original commit
id was, and then put in the signed-off-by area what you changed.

There's loads of examples of backports in this format on the stable
mailing list, look at them for what to do.

thanks,

greg k-h

[PATCH stable 6.6] lib/buildid: Handle memfd_secret() files in build_id_parse()

[Chen Linxuan]

[ 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
[ Linxuan: perform an equivalent direct check without folio-based changes ]
Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event")
Signed-off-by: Chen Linxuan <chenlinxuan@deepin.org>
---

Some previous discussions can be found in the following links:
https://lore.kernel.org/stable/05D0A9F7DE394601+20250311100555.310788-2-chenlinxuan@deepin.org/

---
 lib/buildid.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/buildid.c b/lib/buildid.c
index 9fc46366597e..6249bd47fb0b 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,12 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
 	if (!vma->vm_file)
 		return -EINVAL;
 
+#ifdef CONFIG_SECRETMEM
+       /* reject secretmem folios created with memfd_secret() */
+       if (vma->vm_file->f_mapping->a_ops == &secretmem_aops)
+               return -EFAULT;
+#endif
+
 	page = find_get_page(vma->vm_file->f_mapping, 0);
 	if (!page)
 		return -EFAULT;	/* page not mapped */
-- 
2.48.1

[PATCH stable 6.6] lib/buildid: Handle memfd_secret() files in build_id_parse()

[Chen Linxuan]

[ 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
[ Linxuan: perform an equivalent direct check 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>
---

Some previous discussions can be found in the following links:
https://lore.kernel.org/stable/05D0A9F7DE394601+20250311100555.310788-2-chenlinxuan@deepin.org/

---
 lib/buildid.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/buildid.c b/lib/buildid.c
index 9fc46366597e..6249bd47fb0b 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,12 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
 	if (!vma->vm_file)
 		return -EINVAL;
 
+#ifdef CONFIG_SECRETMEM
+       /* reject secretmem folios created with memfd_secret() */
+       if (vma->vm_file->f_mapping->a_ops == &secretmem_aops)
+               return -EFAULT;
+#endif
+
 	page = find_get_page(vma->vm_file->f_mapping, 0);
 	if (!page)
 		return -EFAULT;	/* page not mapped */
-- 
2.48.1

Re: [PATCH stable 6.6] lib/buildid: Handle memfd_secret() files in build_id_parse()

[Greg Kroah-Hartman]

On Fri, Mar 14, 2025 at 02:40:39PM +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
> [ Linxuan: perform an equivalent direct check without folio-based changes ]
> Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event")
> Signed-off-by: Chen Linxuan <chenlinxuan@deepin.org>
> ---
> 
> Some previous discussions can be found in the following links:
> https://lore.kernel.org/stable/05D0A9F7DE394601+20250311100555.310788-2-chenlinxuan@deepin.org/
> 
> ---
>  lib/buildid.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/lib/buildid.c b/lib/buildid.c
> index 9fc46366597e..6249bd47fb0b 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,12 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
>  	if (!vma->vm_file)
>  		return -EINVAL;
>  
> +#ifdef CONFIG_SECRETMEM
> +       /* reject secretmem folios created with memfd_secret() */
> +       if (vma->vm_file->f_mapping->a_ops == &secretmem_aops)
> +               return -EFAULT;
> +#endif

We really do not like #ifdef in .c files.  Why can't this use much the
same call it does in the original commit?  Just put that in the correct
.h file so that the #ifdef is not needed here, to make the backport
match much more cleanly.

thanks,

greg k-h