linux-kernel - Re: [PATCH stable 6.6 v2] lib/buildid: Handle memfd_secret() files in build_id

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAC1kPDN-cwLyJgXY2DUZuhUf+guFS7u8OWJx_3G8s6RMua1NJQ@mail.gmail.com>
Date: Mon, 17 Mar 2025 13:38:36 +0800
From: Chen Linxuan <chenlinxuan@...pin.org>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: Chen Linxuan <chenlinxuan@...pin.org>, Andrii Nakryiko <andrii@...nel.org>, 
	Jiri Olsa <jolsa@...nel.org>, Sasha Levin <sashal@...nel.org>, 
	Eduard Zingerman <eddyz87@...il.com>, Jann Horn <jannh@...gle.com>, 
	Alexey Dobriyan <adobriyan@...il.com>, Alexei Starovoitov <ast@...nel.org>, 
	Shakeel Butt <shakeel.butt@...ux.dev>, "Peter Zijlstra (Intel)" <peterz@...radead.org>, 
	Yi Lai <yi1.lai@...el.com>, Daniel Borkmann <daniel@...earbox.net>, stable@...r.kernel.org, 
	linux-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH stable 6.6 v2] lib/buildid: Handle memfd_secret() files in build_id_parse()

Greg KH <gregkh@...uxfoundation.org> 于2025年3月17日周一 13:15写道：
>
> On Mon, Mar 17, 2025 at 01:04:41PM +0800, Chen Linxuan wrote:
> > Greg KH <greg@...ah.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@...el.com>
> > > > Suggested-by: Shakeel Butt <shakeel.butt@...ux.dev>
> > > > Signed-off-by: Andrii Nakryiko <andrii@...nel.org>
> > > > Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> > > > Acked-by: Shakeel Butt <shakeel.butt@...ux.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@...r.kernel.org
> > > > Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event")
> > > > Signed-off-by: Chen Linxuan <chenlinxuan@...pin.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