All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florent Revest <revest@chromium.org>
To: Peter Xu <peterx@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	akpm@linux-foundation.org, catalin.marinas@arm.com,
	anshuman.khandual@arm.com, joey.gouly@arm.com, mhocko@suse.com,
	keescook@chromium.org, david@redhat.com, izbyshev@ispras.ru,
	nd@arm.com, broonie@kernel.org, szabolcs.nagy@arm.com
Subject: Re: [PATCH 0/4] MDWE without inheritance
Date: Fri, 5 May 2023 18:42:08 +0200	[thread overview]
Message-ID: <CABRcYmJFcUs=3QYXz8iq7qvu2orJ4HL-cHdBKg9o7=Ma=nfPLw@mail.gmail.com> (raw)
In-Reply-To: <ZFQQSKijXQHWlYaI@x1n>

On Thu, May 4, 2023 at 10:06 PM Peter Xu <peterx@redhat.com> wrote:
>
> On Thu, May 04, 2023 at 07:09:38PM +0200, Florent Revest wrote:
> > Joey recently introduced a Memory-Deny-Write-Executable (MDWE) prctl which tags
> > current with a flag that prevents pages that were previously not executable from
> > becoming executable.
> > This tag always gets inherited by children tasks. (it's in MMF_INIT_MASK)
> >
> > At Google, we've been using a somewhat similar downstream patch for a few years
> > now. To make the adoption of this feature easier, we've had it support a mode in
> > which the W^X flag does not propagate to children. For example, this is handy if
> > a C process which wants W^X protection suspects it could start children
> > processes that would use a JIT.
> >
> > I'd like to align our features with the upstream prctl. This series proposes a
> > new NO_INHERIT flag to the MDWE prctl to make this kind of adoption easier. It
> > sets a different flag in current that is not in MMF_INIT_MASK and which does not
> > propagate.
>
> I don't think I have enough context, so sorry if I'm going to ask a naive
> question..

Not at all! :) You're absolutely right, it's important to address these points.

> I can understand how current MDWE helps on not allowing any modifi-able
> content from becoming executable.  How could NO_INHERIT help if it won't
> inherit and not in MMF_INIT_MASK?

The way I see it, enabling MDWE is just a small step towards hardening
a binary anyway. It can possibly make exploitation a bit harder in the
case where the attacker has _just_: a write primitive they can use to
write a shellcode somewhere and a primitive to make that page
executable later. It's a fairly narrow protection already and I think
it only really helps as part of a broader "defense in depth" strategy.

> IIUC it means the restriction will only apply to the current process.  Then
> I assume the process can escape from this rule simply by a fork().  If so,
> what's the point to protect at all?

If we assume enough control from the attacker, then MDWE is already
useless since it can be bypassed by writing to a file and then
mmapping that file with PROT_EXEC. I think that's a good example of
how "perfect can be the enemy of good" in security hardening. MDWE
isn't a silver-bullet but it's a cheap trick and it makes a small dent
in reducing the attack surface so it seems worth having anyway ?

But indeed, to address your question, if you choose to use this
NO_INHERIT flag: you're no longer protected if the attacker can fork()
as part of their exploitation. I think it's been a useful trade-off
for our internal users since, on the other hand, it also makes
adoption a lot easier: our C++ services developers can trivially opt
into a potpourri of hardening features without having to think too
much about how they work under-the-hood. The default behavior has been
to use a NO_INHERIT strategy so users don't get bad surprises the day
when they try to spawn a JITted subcommand. In the meantime, their C++
service still gets a little bit of extra protection.

> And, what's the difference of this comparing to disabling MDWE after being
> enabled (which seems to be forbidden for now, but it seems fork() can play
> a similar role of disabling it)?

That would be functionally somewhat similar, yes. I think it mostly
comes down to ease of adoption. I imagine that users who would opt
into NO_INHERIT are those who are interested in MDWE for the binary
they are writing but aren't 100% confident in what subprocesses they
will run and so they don't have to think about disabling it after
every fork.

  reply	other threads:[~2023-05-05 16:42 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-04 17:09 [PATCH 0/4] MDWE without inheritance Florent Revest
2023-05-04 17:09 ` [PATCH 1/4] kselftest: vm: Fix tabs/spaces inconsistency in the mdwe test Florent Revest
2023-05-04 17:09 ` [PATCH 2/4] kselftest: vm: Fix mdwe's mmap_FIXED test case Florent Revest
2023-05-04 17:13   ` Florent Revest
2023-05-04 17:09 ` [PATCH 3/4] mm: Add a NO_INHERIT flag to the PR_SET_MDWE prctl Florent Revest
2023-05-05 18:34   ` Catalin Marinas
2023-05-08 12:11     ` Florent Revest
2023-05-04 17:09 ` [PATCH 4/4] kselftest: vm: Add tests for no-inherit memory-deny-write-execute Florent Revest
2023-05-04 20:29   ` Alexey Izbyshev
2023-05-05 16:42     ` Florent Revest
2023-05-05 21:26       ` Alexey Izbyshev
2023-05-08 12:12         ` Florent Revest
2023-05-04 20:06 ` [PATCH 0/4] MDWE without inheritance Peter Xu
2023-05-05 16:42   ` Florent Revest [this message]
2023-05-08  1:29     ` Peter Xu
2023-05-08 12:12       ` Florent Revest
2023-05-08 14:10         ` Catalin Marinas
2023-05-08 17:21           ` Topi Miettinen
2023-05-09 10:04             ` Catalin Marinas

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='CABRcYmJFcUs=3QYXz8iq7qvu2orJ4HL-cHdBKg9o7=Ma=nfPLw@mail.gmail.com' \
    --to=revest@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=anshuman.khandual@arm.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=david@redhat.com \
    --cc=izbyshev@ispras.ru \
    --cc=joey.gouly@arm.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=nd@arm.com \
    --cc=peterx@redhat.com \
    --cc=szabolcs.nagy@arm.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.