Re: [PATCH v6 7/7] mseal, system mappings: update mseal.rst

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Tue, Feb 25, 2025 at 02:31:15PM -0800, Jeff Xu wrote:
> On Mon, Feb 24, 2025 at 10:07 PM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > On Mon, Feb 24, 2025 at 05:45:13PM +0000, jeffxu@chromium.org wrote:
> > > From: Jeff Xu <jeffxu@chromium.org>
> > >
> > > Update memory sealing documentation to include details about system
> > > mappings.
> > >
> > > Signed-off-by: Jeff Xu <jeffxu@chromium.org>
> > > ---
> > >  Documentation/userspace-api/mseal.rst | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/Documentation/userspace-api/mseal.rst b/Documentation/userspace-api/mseal.rst
> > > index 41102f74c5e2..10147281bf2d 100644
> > > --- a/Documentation/userspace-api/mseal.rst
> > > +++ b/Documentation/userspace-api/mseal.rst
> > > @@ -130,6 +130,13 @@ Use cases
> > >
> > >  - Chrome browser: protect some security sensitive data structures.
> > >
> > > +- System mappings:
> > > +  If supported by an architecture (via CONFIG_ARCH_HAS_MSEAL_SYSTEM_MAPPINGS),
> > > +  the CONFIG_MSEAL_SYSTEM_MAPPINGS seals system mappings, e.g. vdso, vvar,
> > > +  uprobes, sigpage, vectors, etc. CHECKPOINT_RESTORE, UML, gVisor, rr are
> > > +  known to relocate or unmap system mapping, therefore this config can't be
> > > +  enabled universally.
> >
> > Thanks for adding this.
> >
> > Similar comments to the Kconfig update - you are listing features that do not
> > exist yet, please just list what you're doing, specifically, and avoid the vague
> > 'etc.', we don't need to be vague.
> >
> OK, I will remove etc and list the known mappings here.
>
> > As per the Kconfig comment - you need to be a lot more clear, I think you're
> > duplicating the text from there to here, so again I suggest something like:
> >
> > 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.
> >
> Sure.
>
> > You also seem to be writing very little here, it's a documentation page, you can
> > be as verbose as you like :)
> >
> > You really need to add some more detail here in general - you aren't explaining
> > why people would want to enable this, what you're mitigating, etc. from that you
> > explain _why_ it doesn't work for some things.
> >
> The mseal.rst already includes below regarding the protection/mitigation.

Not for system sealing. Please add it even if you think it'd be
duplicative. Thanks.

>
>
>
>
>
> > You're also not mentioning architectural limitations here, for instance that you
> > can only do this on arches that don't require VDSO relocation and listing
> > known-good arches.
> >
> > This is a documentation file, you can go wild :) the more information here the
> > better.
> >
> > WARNING
> > =======
> >
> > > +
> > >  When not to use mseal
> > >  =====================
> > >  Applications can apply sealing to any virtual memory region from userspace,
> > > --
> > > 2.48.1.601.g30ceb7b040-goog
> > >