Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>
To: Solar Designer <solar@openwall.com>, Pavel Machek <pavel@ucw.cz>
Cc: kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-fsdevel@vger.kernel.org, linux-integrity@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org, oleg@redhat.com,
	x86@kernel.org, luto@kernel.org, David.Laight@ACULAB.COM,
	fweimer@redhat.com, mark.rutland@arm.com, mic@digikod.net,
	Rich Felker <dalias@libc.org>
Subject: Re: [PATCH v2 0/4] [RFC] Implement Trampoline File Descriptor
Date: Wed, 23 Sep 2020 14:41:04 -0500	[thread overview]
Message-ID: <1c3acf03-94af-1d5e-00c6-d874ee0ef330@linux.microsoft.com> (raw)
In-Reply-To: <20200923091456.GA6177@openwall.com>



On 9/23/20 4:14 AM, Solar Designer wrote:
>>> The W^X implementation today is not complete. There exist many user level
>>> tricks that can be used to load and execute dynamic code. E.g.,
>>>
>>> - Load the code into a file and map the file with R-X.
>>>
>>> - Load the code in an RW- page. Change the permissions to R--. Then,
>>>   change the permissions to R-X.
>>>
>>> - Load the code in an RW- page. Remap the page with R-X to get a separate
>>>   mapping to the same underlying physical page.
>>>
>>> IMO, these are all security holes as an attacker can exploit them to inject
>>> his own code.
>> IMO, you are smoking crack^H^H very seriously misunderstanding what
>> W^X is supposed to protect from.
>>
>> W^X is not supposed to protect you from attackers that can already do
>> system calls. So loading code into a file then mapping the file as R-X
>> is in no way security hole in W^X.
>>
>> If you want to provide protection from attackers that _can_ do system
>> calls, fine, but please don't talk about W^X and please specify what
>> types of attacks you want to prevent and why that's good thing.
> On one hand, Pavel is absolutely right.  It is ridiculous to say that
> "these are all security holes as an attacker can exploit them to inject
> his own code."
> 

Why? Isn't it possible that an attacker can exploit some vulnerability such
as buffer overflow and overwrite the buffer that contains the dynamic code?


> On the other hand, "what W^X is supposed to protect from" depends on how
> the term W^X is defined (historically, by PaX and OpenBSD).  It may be
> that W^X is partially not a feature to defeat attacks per se, but also a
> policy enforcement feature preventing use of dangerous techniques (JIT).
> 
> Such policy might or might not make sense.  It might make sense for ease
> of reasoning, e.g. "I've flipped this setting, and now I'm certain the
> system doesn't have JIT within a process (can still have it through
> dynamically creating and invoking an entire new program), so there are
> no opportunities for an attacker to inject code nor generate previously
> non-existing ROP gadgets into an executable mapping within a process."
> 
> I do find it questionable whether such policy and such reasoning make
> sense beyond academia.
> 
> Then, there might be even more ways in which W^X is not perfect enough
> to enable such reasoning.  What about using ptrace(2) to inject code?
> Should enabling W^X also disable ability to debug programs by non-root?
> We already have Yama ptrace_scope, which can achieve that at the highest
> setting, although that's rather inconvenient and is probably unexpected
> by most to be a requirement for having (ridiculously?) full W^X allowing
> for the academic reasoning.
> 

I am not suggesting that W^X be fixed. That is up to the maintainers of that
code. I am saying that if the security subsystem is enhanced in the future with
policies and settings that prevent the user tricks I mentioned, then it becomes
impossible to execute dynamic code except by making security exceptions on a case
by case basis.

As an alternative to making security exceptions, one could convert dynamic code
to static code which can then be authenticated.

> Personally, I am for policies that make more practical sense.  For
> example, years ago I advocated here on kernel-hardening that we should
> have a mode where ELF flags enabling/disabling executable stack are
> ignored, and non-executable stack is always enforced.  This should also
> be extended to default (at program startup) permissions on more than
> just stack (but also on .bss, typical libcs' heap allocations, etc.)
> However, I am not convinced there's enough value in extending the policy
> to restricting explicit uses of mprotect(2).
> 
> Yes, PaX did that, and its emutramp.txt said "runtime code generation is
> by its nature incompatible with PaX's PAGEEXEC/SEGMEXEC and MPROTECT
> features, therefore the real solution is not in emulation but by
> designing a kernel API for runtime code generation and modifying
> userland to make use of it."  However, not being convinced in the
> MPROTECT feature having enough practical value, I am also not convinced
> "a kernel API for runtime code generation and modifying userland to make
> use of it" is the way to go.
> 

In a separate email, I will try to answer this and provide justification
for why it is better to do it in the kernel.

> Having static instead of dynamically-generated trampolines in userland
> code where possible (and making other userland/ABI changes to make that
> possible in more/all cases) is an obvious improvement, and IMO should be
> a priority over the above.
>

> While I share my opinion here, I don't mean that to block Madhavan's
> work.  I'd rather defer to people more knowledgeable in current userland
> and ABI issues/limitations and plans on dealing with those, especially
> to Florian Weimer.  I haven't seen Florian say anything specific for or
> against Madhavan's proposal, and I'd like to.  (Have I missed that?)
> It'd be wrong to introduce a kernel API that userland doesn't need, and
> it'd be right to introduce one that userland actually intends to use.
> 
> I've also added Rich Felker to CC here, for musl libc and its possible
> intent to use the proposed API.  (My guess is there's no such need, and
> thus no intent, but Rich might want to confirm that or correct me.)
> 
> Alexander

Madhavan

  parent reply	other threads:[~2020-09-23 19:41 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <210d7cd762d5307c2aa1676705b392bd445f1baa>
2020-09-16 15:08 ` madvenka
2020-09-16 15:08   ` [PATCH v2 1/4] [RFC] fs/trampfd: Implement the trampoline file descriptor API madvenka
2020-09-16 15:08   ` [PATCH v2 2/4] [RFC] x86/trampfd: Provide support for the trampoline file descriptor madvenka
2020-09-16 15:08   ` [PATCH v2 3/4] [RFC] arm64/trampfd: " madvenka
2020-09-16 15:08   ` [PATCH v2 4/4] [RFC] arm/trampfd: " madvenka
2020-09-17  1:04   ` [PATCH v2 0/4] [RFC] Implement Trampoline File Descriptor Florian Weimer
2020-09-17 15:36     ` Madhavan T. Venkataraman
2020-09-17 15:57       ` Madhavan T. Venkataraman
2020-09-17 16:01         ` Florian Weimer
2020-09-23  1:46       ` Arvind Sankar
2020-09-23  9:11         ` Arvind Sankar
2020-09-23 19:17           ` Madhavan T. Venkataraman
2020-09-23 19:51             ` Arvind Sankar
2020-09-23 23:51               ` Madhavan T. Venkataraman
2020-09-24 20:23               ` Madhavan T. Venkataraman
2020-09-24 20:52                 ` Florian Weimer
2020-09-25 22:22                   ` Madhavan T. Venkataraman
2020-09-27 18:25                     ` Madhavan T. Venkataraman
2020-09-24 22:13                 ` Pavel Machek
2020-09-24 23:43                 ` Arvind Sankar
2020-09-25 22:44                   ` Madhavan T. Venkataraman
2020-09-26 15:55                     ` Arvind Sankar
2020-09-27 17:59                       ` Madhavan T. Venkataraman
2020-09-22 21:53 ` madvenka
2020-09-22 21:53   ` [PATCH v2 1/4] [RFC] fs/trampfd: Implement the trampoline file descriptor API madvenka
2020-09-22 21:53   ` [PATCH v2 2/4] [RFC] x86/trampfd: Provide support for the trampoline file descriptor madvenka
2020-09-22 21:53   ` [PATCH v2 3/4] [RFC] arm64/trampfd: " madvenka
2020-09-22 21:53   ` [PATCH v2 4/4] [RFC] arm/trampfd: " madvenka
2020-09-22 21:54   ` [PATCH v2 0/4] [RFC] Implement Trampoline File Descriptor Madhavan T. Venkataraman
2020-09-23  8:14   ` Pavel Machek
2020-09-23  9:14     ` Solar Designer
2020-09-23 14:11       ` Solar Designer
2020-09-23 15:18         ` Pavel Machek
2020-09-23 18:00           ` Solar Designer
2020-09-23 18:21             ` Solar Designer
2020-09-23 14:39       ` Florian Weimer
2020-09-23 18:09         ` Andy Lutomirski
2020-09-23 18:11         ` Solar Designer
2020-09-23 18:49           ` Arvind Sankar
2020-09-23 23:53         ` Madhavan T. Venkataraman
2020-09-23 19:41       ` Madhavan T. Venkataraman [this message]
2020-09-23 18:10     ` James Morris
2020-09-23 18:32     ` Madhavan T. Venkataraman
2020-09-23  8:42   ` Pavel Machek
2020-09-23 18:56     ` Madhavan T. Venkataraman
2020-09-23 20:51       ` Pavel Machek
2020-09-23 23:04         ` Madhavan T. Venkataraman
2020-09-24 16:44         ` Mickaël Salaün
2020-09-24 22:05           ` Pavel Machek
2020-09-25 10:12             ` Mickaël Salaün

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=1c3acf03-94af-1d5e-00c6-d874ee0ef330@linux.microsoft.com \
    --to=madvenka@linux.microsoft.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=dalias@libc.org \
    --cc=fweimer@redhat.com \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mic@digikod.net \
    --cc=oleg@redhat.com \
    --cc=pavel@ucw.cz \
    --cc=solar@openwall.com \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH v2 0/4] [RFC] Implement Trampoline File Descriptor' \
    /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

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).