[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <19e81e87-7430-4e23-ac67-dbb987496dd4@lucifer.local>
Date: Tue, 25 Feb 2025 13:41:25 +0000
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Benjamin Berg <benjamin@...solutions.net>
Cc: "jeffxu@...omium.org" <jeffxu@...omium.org>,
"Jason@...c4.com" <Jason@...c4.com>,
"adobriyan@...il.com" <adobriyan@...il.com>,
"deller@....de" <deller@....de>, "gerg@...nel.org" <gerg@...nel.org>,
"anna-maria@...utronix.de" <anna-maria@...utronix.de>,
"davem@...emloft.net" <davem@...emloft.net>,
"avagin@...il.com" <avagin@...il.com>,
"mhocko@...e.com" <mhocko@...e.com>, "enh@...gle.com" <enh@...gle.com>,
"thomas.weissschuh@...utronix.de" <thomas.weissschuh@...utronix.de>,
"hch@....de" <hch@....de>, "hca@...ux.ibm.com" <hca@...ux.ibm.com>,
"peterz@...radead.org" <peterz@...radead.org>,
"adhemerval.zanella@...aro.org" <adhemerval.zanella@...aro.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"ojeda@...nel.org" <ojeda@...nel.org>,
"jannh@...gle.com" <jannh@...gle.com>,
"f.fainelli@...il.com" <f.fainelli@...il.com>,
"sroettger@...gle.com" <sroettger@...gle.com>,
"ardb@...gle.com" <ardb@...gle.com>,
"jorgelo@...omium.org" <jorgelo@...omium.org>,
"rdunlap@...radead.org" <rdunlap@...radead.org>,
"mark.rutland@....com" <mark.rutland@....com>,
"Liam.Howlett@...cle.com" <Liam.Howlett@...cle.com>,
"vbabka@...e.cz" <vbabka@...e.cz>,
"mpe@...erman.id.au" <mpe@...erman.id.au>,
"oleg@...hat.com" <oleg@...hat.com>,
"willy@...radead.org" <willy@...radead.org>,
"keescook@...omium.org" <keescook@...omium.org>,
"peterx@...hat.com" <peterx@...hat.com>,
"mike.rapoport@...il.com" <mike.rapoport@...il.com>,
"mingo@...nel.org" <mingo@...nel.org>,
"rientjes@...gle.com" <rientjes@...gle.com>,
"groeck@...omium.org" <groeck@...omium.org>,
"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
"pedro.falcato@...il.com" <pedro.falcato@...il.com>,
"ardb@...nel.org" <ardb@...nel.org>,
"42.hyeyoo@...il.com" <42.hyeyoo@...il.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"johannes@...solutions.net" <johannes@...solutions.net>,
"linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>,
"torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"aleksandr.mikhalitsyn@...onical.com" <aleksandr.mikhalitsyn@...onical.com>
Subject: Re: [PATCH v7 5/7] mseal, system mappings: enable uml architecture
On Tue, Feb 25, 2025 at 01:24:49PM +0100, Benjamin Berg wrote:
> Hi,
>
> On Tue, 2025-02-25 at 10:37 +0000, Lorenzo Stoakes wrote:
> > On Tue, Feb 25, 2025 at 08:45:21AM +0000, Berg, Benjamin wrote:
> > > Hi,
> > >
> > > On Tue, 2025-02-25 at 06:22 +0000, Lorenzo Stoakes wrote:
> > > > On Mon, Feb 24, 2025 at 10:52:44PM +0000, jeffxu@...omium.org wrote:
> > > > > From: Jeff Xu <jeffxu@...omium.org>
> > > > >
> > > > > Provide support for CONFIG_MSEAL_SYSTEM_MAPPINGS on UML, covering
> > > > > the vdso.
> > > > >
> > > > > Testing passes on UML.
> > > >
> > > > Maybe expand on this by stating that it has been confirmed by Benjamin (I
> > > > _believe_) that UML has no need for problematic relocation so this is known to
> > > > be good.
> > >
> > > I may well be misreading this message, but this sounds to me that this
> > > is a misinterpretation. So, just to clarify in case that is needed.
> > >
> > > CONFIG_MSEAL_SYSTEM_MAPPINGS does work fine for the UML kernel.
> > > However, the UML kernel is a normal userspace application itself and
> > > for this application to run, the host kernel must have the feature
> > > disabled.
> > >
> > > So, UML supports the feature. But it still *cannot* run on a host
> > > machine that has the feature enabled.
> >
> > Sigh ok. Apologies if I misunderstood.
> >
> > Is there any point having this for the 'guest' system? I mean security wise are
> > we concerned about sealing of system mappings?
> >
> > I feel like having this here might just add confusion and churn if it's not
> > useful.
> >
> > If this is useless for UML guest, let's just drop this patch.
>
> I figured it is not a lot of churn and there isn't really any cost to
> enabling the feature.
>
> That said, the only possible real-life use case I can see is doing MM
> subsystem testing using UML. We certainly do not need the feature to
> run our UML based wireless stack and driver tests.
OK ack - my concern is users getting confused about this ironic host
vs. client thing, must disable the security feature in the _actual kernel_
to enable it in the client.
I'm not sure this is really worth it?
I mean I agree this isn't a _huge_ amount added here and I don't want to be
difficult - Jeff, Kees are you really keen on having this? Do you have
specific use cases in mind or was this just a 'because we can':>)
I guess if intent is to slowly add architectures, it's not totally insane
since we kinda know this one is ok so if that's what it is, probably won't
oppose it _too_ badly.
>
> Benjamin
>
> >
> > >
> > > Benjamin
> > >
> > > >
> > > > >
> > > > > Signed-off-by: Jeff Xu <jeffxu@...omium.org>
> > > > > Tested-by: Benjamin Berg <benjamin.berg@...el.com>
> > > >
> > > > Anyway I know UML has at any rate been confirmed to be good to go +
> > > > patch looks
> > > > fine, so:
> > > >
> > > > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> >
> > OK guess drop this tag please until we can figure this out, sorry Jeff.
(to be clear this is just temporary while we establish what's up with this
situation! :>)
> >
> > > >
> > > > > ---
> > > > > arch/um/Kconfig | 1 +
> > > > > arch/x86/um/vdso/vma.c | 6 ++++--
> > > > > 2 files changed, 5 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/arch/um/Kconfig b/arch/um/Kconfig
> > > > > index 18051b1cfce0..eb2d439a5334 100644
> > > > > --- a/arch/um/Kconfig
> > > > > +++ b/arch/um/Kconfig
> > > > > @@ -10,6 +10,7 @@ config UML
> > > > > select ARCH_HAS_FORTIFY_SOURCE
> > > > > select ARCH_HAS_GCOV_PROFILE_ALL
> > > > > select ARCH_HAS_KCOV
> > > > > + select ARCH_HAS_MSEAL_SYSTEM_MAPPINGS
> > > > > select ARCH_HAS_STRNCPY_FROM_USER
> > > > > select ARCH_HAS_STRNLEN_USER
> > > > > select HAVE_ARCH_AUDITSYSCALL
> > > > > diff --git a/arch/x86/um/vdso/vma.c b/arch/x86/um/vdso/vma.c
> > > > > index f238f7b33cdd..fdfba858ffc9 100644
> > > > > --- a/arch/x86/um/vdso/vma.c
> > > > > +++ b/arch/x86/um/vdso/vma.c
> > > > > @@ -54,6 +54,7 @@ int arch_setup_additional_pages(struct
> > > > > linux_binprm *bprm, int uses_interp)
> > > > > {
> > > > > struct vm_area_struct *vma;
> > > > > struct mm_struct *mm = current->mm;
> > > > > + unsigned long vm_flags;
> > > > > static struct vm_special_mapping vdso_mapping = {
> > > > > .name = "[vdso]",
> > > > > };
> > > > > @@ -65,9 +66,10 @@ int arch_setup_additional_pages(struct
> > > > > linux_binprm *bprm, int uses_interp)
> > > > > return -EINTR;
> > > > >
> > > > > vdso_mapping.pages = vdsop;
> > > > > + vm_flags =
> > > > > VM_READ|VM_EXEC|VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC;
> > > > > + vm_flags |= VM_SEALED_SYSMAP;
> > > > > vma = _install_special_mapping(mm, um_vdso_addr,
> > > > > PAGE_SIZE,
> > > > > - VM_READ|VM_EXEC|
> > > > > - VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
> > > > > + vm_flags,
> > > > > &vdso_mapping);
> > > > >
> > > > > mmap_write_unlock(mm);
> > > > > --
> > > > > 2.48.1.658.g4767266eb4-goog
> > > > >
> > >
> > > Intel Deutschland GmbH
> > > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> > > Tel: +49 89 99 8853-0, www.intel.de
> > > Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
> > > Chairperson of the Supervisory Board: Nicole Lau
> > > Registered Office: Munich
> > > Commercial Register: Amtsgericht Muenchen HRB 186928
>
Powered by blists - more mailing lists