On Thu, Feb 1, 2024 at 3:11 PM Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > > On Wed, Jan 31, 2024 at 05:50:24PM +0000, jeffxu@xxxxxxxxxxxx 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 could 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 syscall, 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 flags 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 new mmap > flags alongside it? > > I'll also note that the existing PROT_* flags seem to be conventionally used 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 added at > all it perhaps should be a MAP_* flag, not PROT_*. I'm not sure this aspect 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 flags. > 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
