diff mbox series

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

Message ID 05D0A9F7DE394601+20250311100555.310788-2-chenlinxuan@deepin.org (mailing list archive)
State Superseded
Headers show
Series [stable,6.6] lib/buildid: Handle memfd_secret() files in build_id_parse() | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Chen Linxuan March 11, 2025, 10:05 a.m. UTC
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(+)

Comments

Greg KH March 11, 2025, 11:14 a.m. UTC | #1
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
Chen Linxuan March 12, 2025, 3:03 a.m. UTC | #2
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
>
>
Andrii Nakryiko March 12, 2025, 6:43 p.m. UTC | #3
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
> >
> >
Greg KH March 13, 2025, 4:04 p.m. UTC | #4
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
diff mbox series

Patch

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 */