Re: [PATCH v7 1/7] mseal, system mappings: kernel config and header change

[Jeff Xu <jeffxu@chromium.org>]

On Mon, Feb 24, 2025 at 10:05 PM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
> > +config ARCH_HAS_MSEAL_SYSTEM_MAPPINGS
> > +     bool
> > +     help
> > +       Control MSEAL_SYSTEM_MAPPINGS access based on architecture.
> > +
> > +       A 64-bit kernel is required for the memory sealing feature.
> > +       No specific hardware features from the CPU are needed.
> > +
> > +       To enable this feature, the architecture needs to update their
> > +       special mappings calls to include the sealing flag and confirm
> > +       that it doesn't unmap/remap system mappings during the life
> > +       time of the process. After the architecture enables this, a
> > +       distribution can set CONFIG_MSEAL_SYSTEM_MAPPING to manage access
> > +       to the feature.
>
> Architectures also need to be confirmed not to require any form of VDSO
> relocation, which as discussed in previous series some arches appear to
> need to do. I'd mention that here.
>
This might need clarification, the system mapping includes vdso, right
? Why the focus on vdso ?

The sentence  "... it doesn't unmap/remap system mappings during the
lifetime of the process."  already cover what you want here, I think.


> > +
> > +       For complete descriptions of memory sealing, please see
> > +       Documentation/userspace-api/mseal.rst
> > +
> >  config HAVE_PERF_EVENTS
> >       bool
> >       help
> > diff --git a/security/Kconfig b/security/Kconfig
> > index f10dbf15c294..15a86a952910 100644
> > --- a/security/Kconfig
> > +++ b/security/Kconfig
> > @@ -51,6 +51,24 @@ config PROC_MEM_NO_FORCE
> >
> >  endchoice
> >
> > +config MSEAL_SYSTEM_MAPPINGS
> > +     bool "mseal system mappings"
> > +     depends on 64BIT
> > +     depends on ARCH_HAS_MSEAL_SYSTEM_MAPPINGS
> > +     depends on !CHECKPOINT_RESTORE
> > +     help
> > +       Seal system mappings such as vdso, vvar, sigpage, uprobes, etc.
>
> Let's be specific here, 'etc.' could mean _anything_. Also you aren't
> sealing most of this, let's just list what you are _actually_ sealing
> here. Which is AFAIK VDSO only?
>
I will remove "etc" and list all the mappings.

Those mappings are:
vdso, vvar, vvar_vclock, vectors (arm compact-mode) and sigpage (arm
compact-mode), uprobe.

We seal all system mappings that x86-64 and arm64 have.

> You can update this later as time goes on if/when you expand this.
>
> > +
> > +       A 64-bit kernel is required for the memory sealing feature.
> > +       No specific hardware features from the CPU are needed.
> > +
> > +       Note: CHECKPOINT_RESTORE, UML, gVisor, rr are known to relocate or
> > +       unmap system mapping, therefore this config can't be enabled
> > +       universally.
>
> Thanks for putting this here, appreciate it!
>
> Could we tweak this though? I'd like to make it crystal clear, so I don't
> think 'note' sufficies and this sounds a little too vague.
>
> I think 'warning' is more appropriate here since you're breaking things for
> people who might be unaware. And we need to say this -breaks- programs:
>
> WARNING: This feature breaks programs which rely on relocating or
>          unmapping system mappings.
>
>          Known broken software at the time of writing includes
>          CHECKPOINT_RESTORE, UML, gVisor and rr.
>
> I think this is critical.
>
Sure.