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, jef...@chromium.org wrote:
> > > > From: Jeff Xu <jef...@chromium.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.

Benjamin

> 
> > 
> > Benjamin
> > 
> > > 
> > > > 
> > > > Signed-off-by: Jeff Xu <jef...@chromium.org>
> > > > Tested-by: Benjamin Berg <benjamin.b...@intel.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.stoa...@oracle.com>
> 
> OK guess drop this tag please until we can figure this out, sorry Jeff.
> 
> > > 
> > > > ---
> > > >  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


Reply via email to