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

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

On Tue, Feb 25, 2025 at 02:36:52PM -0800, Jeff Xu wrote:
> On Tue, Feb 25, 2025 at 2:31 PM Jeff Xu <jeffxu@chromium.org> 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.

But not specifically why you'd want to do that for system mappings.

I guess you mean it's _implied_ that it would be sensible to do this for
system mappings and I suppose again I'm asking for emphasis on this

>
> Memory sealing additionally protects the mapping itself against
> modifications. This is useful to mitigate memory corruption issues where a
> corrupted pointer is passed to a memory management system. For example,
> such an attacker primitive can break control-flow integrity guarantees
> since read-only memory that is supposed to be trusted can become writable
> or .text pages can get remapped. Memory sealing can automatically be
> applied by the runtime loader to seal .text and .rodata pages and
> applications can additionally seal security critical data at runtime.

Right, this is exactly the kind of thing you need but obviously adjusted to
mention system mappings, and why they are especially problematic (again -at
risk of sounding like a security idiot here :) - I'm guessing sand box
breakout, rop, syscall, find a way to write, and un-sandboxed code now does
unexpected stuff).

>
> I could copy  some sections from cover-letter to here, specifically
> for special mappings.

yeah that'd be good, while the cover letter will be copied in automatically
by Andrew to the series, it's really nice to have it at a glance in docs
for users who go to docs.kernel.org etc.

>
> >
> >
> >
> >
> >
> > > 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.
> > >
> Sure, I will mention the architecture that has this enabled
> (x86,arm,uml) -- I don't think there is an architecture limitation
> though. mseal is a software feature. The reason why other
> architectures don't have it is due to the fact that I don't have the
> HW for testing

Yeah I mean just list the arches that you've tested, more or less.

Thanks!

>
>
>
> > > 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
> > > >