linux-mm.kvack.org archive mirror
 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 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox