Received-SPF: pass (google.com: domain of linux-kernel+bounces-49005-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: "Theo de Raadt" <deraadt@openbsd.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
cc: Jeff Xu <jeffxu@chromium.org>,
    "Liam R. Howlett" <Liam.Howlett@oracle.com>,
    Jonathan Corbet <corbet@lwn.net>, akpm@linux-foundation.org,
    keescook@chromium.org, jannh@google.com, sroettger@google.com,
    willy@infradead.org, gregkh@linuxfoundation.org,
    usama.anjum@collabora.com, rdunlap@infradead.org, jeffxu@google.com,
    jorgelo@chromium.org, groeck@chromium.org,
    linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
    linux-mm@kvack.org, pedro.falcato@gmail.com, dave.hansen@intel.com,
    linux-hardening@vger.kernel.org
Subject: Re: [PATCH v8 0/4] Introduce mseal
In-reply-to: <CAHk-=wjqozic6JuRimXD=RamnJmD6FoaQki7RtNYrezzx_OfOg@mail.gmail.com>
References: <20240131175027.3287009-1-jeffxu@chromium.org> <20240131193411.opisg5yoyxkwoyil@revolver> <CABi2SkXOX4SRMs0y8FYccoj+XrEiPCJk2seqT+sgO7Na7NWwLg@mail.gmail.com> <20240201204512.ht3e33yj77kkxi4q@revolver> <CABi2SkWB2eV24LBJtgJ73zEwaAWuFhAwrfqx3Rs=tqnpcJ0qRw@mail.gmail.com> <58408.1706828083@cvs.openbsd.org> <CAHk-=wjqozic6JuRimXD=RamnJmD6FoaQki7RtNYrezzx_OfOg@mail.gmail.com>
Comments: In-reply-to Linus Torvalds <torvalds@linux-foundation.org>
   message dated "Thu, 01 Feb 2024 15:15:27 -0800."
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <25907.1706830997.1@cvs.openbsd.org>
Date: Thu, 01 Feb 2024 16:43:17 -0700
Message-ID: <99920.1706830997@cvs.openbsd.org>

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> So yes, to my mind
> 
>     mprotect(addr, len, PROT_READ);
>     mseal(addr, len, 0);
> 
> should basically give identical results to
> 
>     mprotect(addr, len, PROT_READ | PROT_SEAL);
> 
> and using PROT_SEAL at mmap() time is similarly the same obvious
> notion of "map this, and then seal that mapping".

I think that isn't easy to do.  Let's expand it to show error checking.

    if (mprotect(addr, len, PROT_READ) == -1)
       react to the errno value
    if (mseal(addr, len, 0) == -1)
       react to the errno value

and

    if (mprotect(addr, len, PROT_READ | PROT_SEAL) == -1)
       react to the errno value

For current mprotect(), the errno values are mostly related to range
issues with the parameters.

After sealing a region, mprotect() also has the new errno EPERM.

But what is the return value supposed to be from "PROT_READ | PROT_SEAL"
over various sub-region types?

Say I have a region 3 pages long.  One page is unmapped, one page is
regular, and one page is sealed.  Re-arrange those 3 pages in all 6
permutations.  Try them all.

Does the returned errno change, based upon the order?
Does it do part of the operation, or all of the operation?

If the sealed page is first, the regular page is second, and the unmapped
page is 3rd, does it return an error or return 0?  Does it change the
permission on the 3rd page?  If it returns an error, has it changed any
permissions?

I don't think the diff follows the principle of

if an error is returned --> we know nothing was changed.
if success is returned --> we know all the requests were satisfied

> The reason for having "mseal()" as a separate call at all from the
> PROT_SEAL bit is that it does allow possible future expansion (while
> PROT_SEAL is just a single bit, and it won't change semantics) but
> also so that you can do whatever prep-work in stages if you want to,
> and then just go "now we seal it all".


How about you add basic mseal() that is maximum compatible with mimmutable(),
and then we can all talk about whether PROT_SEAL makes sense once there
are applications that demand it, and can prove they need it?