Received-SPF: pass (google.com: domain of linux-kernel+bounces-49034-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: <31812.1706833585.1@cvs.openbsd.org>
Date: Thu, 01 Feb 2024 17:26:25 -0700
Message-ID: <7481.1706833585@cvs.openbsd.org>

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

> and using PROT_SEAL at mmap() time is similarly the same obvious
> notion of "map this, and then seal that mapping".

The usual way is:

    ptr = mmap(NULL, len PROT_READ|PROT_WRITE, ...)

    initialize region between ptr, ptr+len

    mprotect(ptr, len, PROT_READ)
    mseal(ptr, len, 0);


Our source tree contains one place where a locking happens very close
to a mmap().

It is the shared-library-linker 'hints file', this is a file that gets
mapped PROT_READ and then we lock it.

It feels like that could be one operation?  It can't be.

        addr = (void *)mmap(0, hsize, PROT_READ, MAP_PRIVATE, hfd, 0);
        if (_dl_mmap_error(addr))
                goto bad_hints;

        hheader = (struct hints_header *)addr;
        if (HH_BADMAG(*hheader) || hheader->hh_ehints > hsize)
                goto bad_hints;

	/* couple more error checks */

	mimmutable(addr, hsize);
	close(hfd);
	return (0);
bad_hints:
	munmap(addr, hsize);
	...

See the problem?  It unmaps it if the contents are broken.  So even that
case cannot use something like "PROT_SEAL".

These are not hypotheticals.  I'm grepping an entire Unix kernel and
userland source tree, and I know what 100,000+ applications do.  I found
piece of code that could almost use it, but upon inspection it can't,
and it is obvious why: it is best idiom to allow a programmer to insert
an inspection operation between two disctinct operations, and especially
critical if the 2nd operation cannot be reversed.

Noone needs PROT_SEAL as a shortcut operation in mmap() or mprotect().

Throwing around ideas without proving their use in practice is very
unscientific.