Re: [RFC PATCH v1 2/2] mseal: allow noop mprotect

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

On Wed, Mar 12, 2025 at 04:29:50PM -0700, Jeff Xu wrote:
> On Wed, Mar 12, 2025 at 9:45 AM Kees Cook <kees@kernel.org> wrote:
> >
> > On Wed, Mar 12, 2025 at 03:50:40PM +0000, Lorenzo Stoakes wrote:
> > > What about madvise() with MADV_DONTNEED on a r/o VMA that's not faulted in?
> > > That's a no-op right? But it's not permitted.
> >
> Madvise's semantics are about behavior, while mprotect is about
> attributes. To me:  madvise is like "make this VMA do that" while
> mprotect is about "update this VMA's attributes to a new value".
>
> It is more difficult to determine if a behavior is no-op, so I don't
> intend to apply the same no-op concept to madvise().
>
> > Hmm, yes, that's a good example. Thank you!
> >
> > > So now we have an inconsistency between the two calls.
> >
> > Yeah, I see your concern now.
> >
> > > I don't know what you mean by 'ergonomic'?
> >
> > I was thinking about idempotent-ness. Like, some library setting up a
> > memory region, it can't call its setup routine twice if the second time
> > through (where no changes are made) it gets rejected. But I think this
> > is likely just a userspace problem: check for the VMAs before blindly
> > trying to do it again. (This is strictly an imagined situation.)
> >
> Yes.
>
>  We also don't have a system call to query the "mprotect" attributes,
> so it is understandable that userspace can rely on idempotents of the
> mprotect.

PROCMAP_QUERY ioctl, /proc/$pid/pagemap :) I mean hey - these are somewhat
diagnostic-y, racey, un-fun interfaces that we'd rather you not use in
anger when mapping stuff - but they do at least exist :)

(an aside, been playing with PROCMAP_QUERY recently and very cool - we plan
to make this useable under RCU lock rather than mmap lock which will make
it _even more_ useful in future... exciting times :)

It's possible, but it seems that it would be relying upon it purely because
in some cases it would be modifying the mapping, right?

It strikes me as very unlikely that an application would be looking to
modify the attributes of a series of VMAs including ones that have a
security feature enabled which says 'until this is unmapped do not modify
the attributes of this VMA'.

Yes it's _theoretically_ possible but that'd be quite silly no?

>
> > > My reply seemed to get truncated at the end here :) So let me ask again -
> > > do you have a practical case in mind for this?
> >
> I noticed there were idempotent mprotects last year while working on
> applying mseal on stack in Android. I assume this might not be the
> only instance since mprotect gets called a lot in general.
>
> Blocking this won't improve security, it could actually hinder the
> adoption of mseal, i.e. force apps to make code change.

Thanks for the explanation it's appreciated!

But I feel the drawbacks I mentioned previously and elucidated upon in my
reply to Kees outweigh this theoretical concern.

If we encounter actual real-world instances of this we can reconsider,
presuming we are ok with the asymmetry vs. other seal-protected calls. We
have this shipped with a uAPI already like this so there's no rush.

>
> -Jeff
>
> > Sorry, I didn't have any reply to that part, so I left it off. If Jeff
> > has a specific case in mind, I'll let him answer that part. :)
> >
> > -Kees
> >
> > --
> > Kees Cook

Cheers, Lorenzo