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

[Jeff Xu <jeffxu@chromium.org>]

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.

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

-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