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.stoa...@oracle.com> wrote:
> >
> > On Mon, Feb 24, 2025 at 05:45:13PM +0000, jef...@chromium.org wrote:
> > > From: Jeff Xu <jef...@chromium.org>
> > >
> > > Update memory sealing documentation to include details about system
> > > mappings.
> > >
> > > Signed-off-by: Jeff Xu <jef...@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
> > >

Reply via email to