Received-SPF: pass (google.com: domain of linux-kernel+bounces-49150-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240131175027.3287009-1-jeffxu@chromium.org> <20240131175027.3287009-3-jeffxu@chromium.org>
 <20240201231151.GA41472@sol.localdomain>
In-Reply-To: <20240201231151.GA41472@sol.localdomain>
From: Jeff Xu <jeffxu@chromium.org>
Date: Thu, 1 Feb 2024 19:30:29 -0800
Message-ID: <CABi2SkWW78n3PK3Qk5cCzpjb57ZCoLmybA1ds3=rHrGMams7sw@mail.gmail.com>
Subject: Re: [PATCH v8 2/4] mseal: add mseal syscall
To: Eric Biggers <ebiggers@kernel.org>
Cc: akpm@linux-foundation.org, keescook@chromium.org, jannh@google.com, 
	sroettger@google.com, willy@infradead.org, gregkh@linuxfoundation.org, 
	torvalds@linux-foundation.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, deraadt@openbsd.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 1, 2024 at 3:11=E2=80=AFPM Eric Biggers <ebiggers@kernel.org> w=
rote:
>
> On Wed, Jan 31, 2024 at 05:50:24PM +0000, jeffxu@chromium.org wrote:
> > [PATCH v8 2/4] mseal: add mseal syscall
> [...]
> > +/*
> > + * The PROT_SEAL defines memory sealing in the prot argument of mmap()=
.
> > + */
> > +#define PROT_SEAL    0x04000000      /* _BITUL(26) */
> > +
> >  /* 0x01 - 0x03 are defined in linux/mman.h */
> >  #define MAP_TYPE     0x0f            /* Mask for type of mapping */
> >  #define MAP_FIXED    0x10            /* Interpret addr exactly */
> > @@ -33,6 +38,9 @@
> >  #define MAP_UNINITIALIZED 0x4000000  /* For anonymous mmap, memory cou=
ld be
> >                                        * uninitialized */
> >
> > +/* map is sealable */
> > +#define MAP_SEALABLE 0x8000000       /* _BITUL(27) */
>
> IMO this patch is misleading, as it claims to just be adding a new syscal=
l, but
> it actually adds three new UAPIs, only one of which is the new syscall.  =
The
> other two new UAPIs are new flags to the mmap syscall.
>
The description does include all three. I could update the patch title.

> Based on recent discussions, it seems the usefulness of the new mmap flag=
s has
> not yet been established.  Note also that there are only a limited number=
 of
> mmap flags remaining, so we should be careful about allocating them.
>
> Therefore, why not start by just adding the mseal syscall, without the ne=
w mmap
> flags alongside it?
>
> I'll also note that the existing PROT_* flags seem to be conventionally u=
sed for
> the CPU page protections, as opposed to kernel-specific properties of the=
 VMA
> object.  As such, PROT_SEAL feels a bit out of place anyway.  If it's add=
ed at
> all it perhaps should be a MAP_* flag, not PROT_*.  I'm not sure this asp=
ect has
> been properly discussed yet, seeing as the patchset is presented as just =
adding
> sys_mseal().  Some reviewers may not have noticed or considered the new f=
lags.
>
MAP_ flags is more used for type of mapping, such as MAP_FIXED_NOREPLACE.

The PROT_SEAL might make more sense because sealing the protection bit
is the main functionality of the sealing at this moment.

Thanks
-Jeff


> - Eric