From: Jeff Xu <jeffxu@chromium.org>
To: Theo de Raadt <deraadt@openbsd.org>
Cc: Eric Biggers <ebiggers@kernel.org>,
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
Subject: Re: [PATCH v8 2/4] mseal: add mseal syscall
Date: Thu, 1 Feb 2024 20:03:45 -0800 [thread overview]
Message-ID: <CABi2SkWP=N_V9+VvH3omw3gw=VcRqhqenti-qABijo-SNqXxhQ@mail.gmail.com> (raw)
In-Reply-To: <96087.1706846050@cvs.openbsd.org>
On Thu, Feb 1, 2024 at 7:54 PM Theo de Raadt <deraadt@openbsd.org> wrote:
>
> Jeff Xu <jeffxu@chromium.org> wrote:
>
> > On Thu, Feb 1, 2024 at 3:11 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > >
> > > 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 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.
>
> Jeff, please show a piece of software that needs to do PROT_SEAL as
> mprotect() or mmap() argument.
>
I didn't propose mprotect().
for mmap() here is a potential use case:
fs/binfmt_elf.c
if (current->personality & MMAP_PAGE_ZERO) {
/* Why this, you ask??? Well SVr4 maps page 0 as read-only,
and some applications "depend" upon this behavior.
Since we do not have the power to recompile these, we
emulate the SVr4 behavior. Sigh. */
error = vm_mmap(NULL, 0, PAGE_SIZE,
PROT_READ | PROT_EXEC, <-- add PROT_SEAL
MAP_FIXED | MAP_PRIVATE, 0);
}
I don't see the benefit of RWX page 0, which might make a null
pointers error to become executable for some code.
> Please don't write it as a vague essay.
>
> Instead, take a piece of existing code, write a diff, and show your work.
>
> Then explain that diff, justify why doing the PROT_SEAL as an argument
> of mprotect() or mmap() is a required improvement, and show your Linux
> developer peers that you can do computer science.
>
> I did the same work in OpenBSD, at least 25% time over 2 years, and I
> had to prove my work inside my development community. I had to prove
> that it worked system wide, not in 1 program, with hand-waving for the
> rest. If I had said "Looks, it works in ssh, trust me it works in other
> programs", it would not have gone further.
>
> glibc is the best example to demonstrate, but smaller examples might
> convince.
next prev parent reply other threads:[~2024-02-02 4:03 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-31 17:50 [PATCH v8 0/4] Introduce mseal jeffxu
2024-01-31 17:50 ` [PATCH v8 1/4] mseal: Wire up mseal syscall jeffxu
2024-01-31 17:50 ` [PATCH v8 2/4] mseal: add " jeffxu
2024-02-01 23:11 ` Eric Biggers
2024-02-02 3:30 ` Jeff Xu
2024-02-02 3:54 ` Theo de Raadt
2024-02-02 4:03 ` Jeff Xu [this message]
2024-02-02 4:10 ` Theo de Raadt
2024-02-02 4:22 ` Jeff Xu
2024-01-31 17:50 ` [PATCH v8 3/4] selftest mm/mseal memory sealing jeffxu
2024-01-31 17:50 ` [PATCH v8 4/4] mseal:add documentation jeffxu
2024-01-31 19:34 ` [PATCH v8 0/4] Introduce mseal Liam R. Howlett
2024-02-01 1:27 ` Jeff Xu
2024-02-01 1:46 ` Theo de Raadt
2024-02-01 16:56 ` Bird, Tim
2024-02-01 1:55 ` Theo de Raadt
2024-02-01 20:45 ` Liam R. Howlett
2024-02-01 22:24 ` Theo de Raadt
2024-02-02 1:06 ` Greg KH
2024-02-02 3:24 ` Jeff Xu
2024-02-02 3:29 ` Linus Torvalds
2024-02-02 3:46 ` Jeff Xu
2024-02-02 15:18 ` Greg KH
2024-02-01 22:37 ` Jeff Xu
2024-02-01 22:54 ` Theo de Raadt
2024-02-01 23:15 ` Linus Torvalds
2024-02-01 23:43 ` Theo de Raadt
2024-02-02 0:26 ` Theo de Raadt
2024-02-02 3:20 ` Jeff Xu
2024-02-02 4:05 ` Theo de Raadt
2024-02-02 4:54 ` Jeff Xu
2024-02-02 5:00 ` Theo de Raadt
2024-02-02 17:58 ` Jeff Xu
2024-02-02 18:51 ` Pedro Falcato
2024-02-02 21:20 ` Jeff Xu
2024-02-04 19:39 ` David Laight
2024-02-02 17:05 ` Theo de Raadt
2024-02-02 21:02 ` Jeff Xu
2024-02-02 3:14 ` Jeff Xu
2024-02-02 15:13 ` Liam R. Howlett
2024-02-02 17:24 ` Jeff Xu
2024-02-02 19:21 ` Liam R. Howlett
2024-02-02 19:32 ` Theo de Raadt
2024-02-02 20:36 ` Linus Torvalds
2024-02-02 20:57 ` Jeff Xu
2024-02-02 21:18 ` Liam R. Howlett
2024-02-02 23:36 ` Linus Torvalds
2024-02-03 4:45 ` Liam R. Howlett
2024-02-05 22:13 ` Suren Baghdasaryan
2024-02-02 20:14 ` Jeff Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CABi2SkWP=N_V9+VvH3omw3gw=VcRqhqenti-qABijo-SNqXxhQ@mail.gmail.com' \
--to=jeffxu@chromium.org \
--cc=akpm@linux-foundation.org \
--cc=dave.hansen@intel.com \
--cc=deraadt@openbsd.org \
--cc=ebiggers@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=groeck@chromium.org \
--cc=jannh@google.com \
--cc=jeffxu@google.com \
--cc=jorgelo@chromium.org \
--cc=keescook@chromium.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=pedro.falcato@gmail.com \
--cc=rdunlap@infradead.org \
--cc=sroettger@google.com \
--cc=torvalds@linux-foundation.org \
--cc=usama.anjum@collabora.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).