public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Jeremy Linton <jeremy.linton@arm.com>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Brown <broonie@kernel.org>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>,
	Will Deacon <will@kernel.org>, "H . J . Lu" <hjl.tools@gmail.com>,
	Yu-cheng Yu <yu-cheng.yu@intel.com>,
	linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	libc-alpha@sourceware.org, Mark Rutland <mark.rutland@arm.com>
Subject: Re: [PATCH v7 0/4] arm64: Enable BTI for the executable as well as the interpreter
Date: Thu, 6 Jan 2022 10:09:35 -0600	[thread overview]
Message-ID: <8550afd2-268d-a25f-88fd-0dd0b184ca23@arm.com> (raw)
In-Reply-To: <YdbL5kIzi0xqVTVd@arm.com>

Hi,

On 1/6/22 05:00, Catalin Marinas wrote:
> Hi Jeremy,
> 
> On Wed, Jan 05, 2022 at 04:42:01PM -0600, Jeremy Linton wrote:
>> On 1/4/22 11:32, Mark Brown wrote:
>>> On Thu, Dec 09, 2021 at 11:10:48AM +0000, Szabolcs Nagy wrote:
>>>> The 12/08/2021 18:23, Catalin Marinas wrote:
>>>>> On Mon, Nov 15, 2021 at 03:27:10PM +0000, Mark Brown wrote:
>>>>>> memory is already mapped with PROT_EXEC.  This series resolves this by
>>>>>> handling the BTI property for both the interpreter and the main
>>>>>> executable.
>>>>>
>>>>> Given the silence on this series over the past months, I propose we drop
>>>>> it. It's a bit unfortunate that systemd's MemoryDenyWriteExecute cannot
>>>>> work with BTI but I also think the former is a pretty blunt hardening
>>>>> mechanism (rejecting any mprotect(PROT_EXEC) regardless of the previous
>>>>> attributes).
>>>>
>>>> i still think it would be better if the kernel dealt with
>>>> PROT_BTI for the exe loaded by the kernel.
>>>
>>> The above message from Catalin isn't quite the full story here - my
>>> understanding from backchannel is that there's concern from others that
>>> we might be creating future issues by enabling PROT_BTI, especially in
>>> the case where the same permissions issue prevents the dynamic linker
>>> disabling PROT_BTI.  They'd therefore rather stick with the status quo
>>> and not create any new ABI.  Unfortunately that's not something people
>>> have been willing to say on the list, hopefully the above captures the
>>> thinking well enough.
>>>
>>> Personally I'm a bit ambivalent on this, I do see the potential issue
>>> but I'm having trouble constructing an actual scenario and my instinct
>>> is that since we handle PROT_EXEC we should also handle PROT_BTI for
>>> consistency.
>>
>> I'm hardly a security expert, but it seems to me that BTI hardens against a
>> wider set of possible exploits than MDWE.
> 
> They are complementary features.
> 
>> Yet, we are silently turning it
>> off for systemd services which are considered some of the most security
>> critical things in the machine right now (ex:logind, etc). So despite
>> 'systemd-analyze secuirty` flagging those services as the most secure ones
>> on a system, they might actually be less secure.
> 
> Well, that's a distro decision. MDWE/MDWX is not something imposed by
> the kernel but rather a seccomp bpf filter set up by systemd.
> 
>> It also seems that getting BTI turned on earlier, as this patch is doing is
>> itself a win.
>>
>> So, mentally i'm having a hard time balancing the hypothetical problem laid
>> out, as it should only really exist in an environment similar to the MDWE
>> one, since AFAIK, its possible today to just flip it back off unless MDWE
>> stops that from happening.
> 
> That's a user ABI change and given that the first attempt was shown to
> break with some combination of old loader and new main executable (or
> the other way around), I'd rather keep things as they are.

This should only change the behavior for for binaries which conform to 
the new ABI containing the BTI note. So outside of the tiny window of 
things built with BTI, but run on !BTI hardware or older kernel+glibc, 
this shouldn't be a problem. (Unless i'm missing something) Put another 
way, now is the time to make a change, before there is a legacy BTI 
ecosystem we have to deal with.


> 
>> What are the the remaining alternatives? A new syscall? But that is by
>> definition a new ABI,
> 
> A new ABI is better than changing the current ABI.
> 
>> and wouldn't benefit from having BTI turned on as early as this patch
>> is doing.
> 
> In the absence of MDWX, it's not relevant how early the kernel turns BTI
> on for the main executable. The dynamic loader would do this with an
> mprotect() before actually executing any of the main code. Of course, we
> assume there are no security bugs in the dynamic loader.
> 
>> Should we disable MDWE on a BTI machine? I'm
>> not sure that is a good look, particularly if MDWE happens to successfully
>> stop some exploit. AFAIK, MDWE+BTI are a good strong combination, it seems a
>> shame if we can't get them both working together.
> 
> AFAICT MDWX wants (one of the filters) to prevent a previously writable
> mapping from becoming executable through mprotect(PROT_EXEC). How common
> is mprotect(PROT_EXEC|PROT_BTI) outside of the dynamic loader? I doubt
> it is, especially in an MDWX environment. So can we not change the
> filter to allow PROT_EXEC|PROT_BTI? If your code is already exploitable
> to allow random syscalls, all bets are off anyway.

I would expect JITs to be twittling EXEC|BTI but, those wouldn't be able 
to trivially run under MDWE anyway. So rarely?

Changing the filter to allow PROT_EXEC|PROT_BTI defeats the purpose 
because the hypothetical exploit would just add the BTI tags and turn 
BTI on as well. The filter, as is, also provides additional BTI 
protections because it makes it more difficult to disable BTI. Without 
that filter it seems likely someone could come up with a way to use an 
existing PROT_EXEC as a gadget to disable BTI anywhere they choose.

So, to your point before, BTI+MDWE are complementary, the combination 
seems considerably more robust than either by itself.

> 
>> I hesitate to suggest it, but maybe this patch should be conditional
>> somehow, that way !systemd/MDWE machines can behave as they do today, and
>> systemd/MDWE machines can request BTI be turned on by the kernel
>> automatically?
> 
> That would be some big knob sysctl but I'm still not keen on toggling
> the ABI like this.
> 


  reply	other threads:[~2022-01-06 16:09 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-15 15:27 Mark Brown
2021-11-15 15:27 ` [PATCH v7 1/4] elf: Allow architectures to parse properties on the main executable Mark Brown
2021-11-15 15:27 ` [PATCH v7 2/4] arm64: Enable BTI for main executable as well as the interpreter Mark Brown
2021-11-15 15:27 ` [PATCH v7 3/4] elf: Remove has_interp property from arch_adjust_elf_prot() Mark Brown
2021-11-15 15:27 ` [PATCH v7 4/4] elf: Remove has_interp property from arch_parse_elf_property() Mark Brown
2021-12-08 18:23 ` [PATCH v7 0/4] arm64: Enable BTI for the executable as well as the interpreter Catalin Marinas
2021-12-09 11:10   ` Szabolcs Nagy
2022-01-04 17:32     ` Mark Brown
2022-01-05 22:42       ` Jeremy Linton
2022-01-06 11:00         ` Catalin Marinas
2022-01-06 16:09           ` Jeremy Linton [this message]
2022-01-06 18:13             ` Catalin Marinas
2022-01-06 19:07               ` Mark Brown
2022-01-07 12:01                 ` Catalin Marinas
2022-01-07 13:10                   ` Mark Brown
2022-01-17 17:54                   ` Catalin Marinas
2022-01-17 18:16                     ` Adhemerval Zanella
2022-01-17 19:01                       ` H.J. Lu
2022-01-18 11:22                         ` Szabolcs Nagy
2022-01-18 12:55                           ` H.J. Lu
2022-01-18 11:02                     ` Szabolcs Nagy
2022-01-27 12:24                       ` Catalin Marinas
2022-01-27 14:48                         ` Szabolcs Nagy

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=8550afd2-268d-a25f-88fd-0dd0b184ca23@arm.com \
    --to=jeremy.linton@arm.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=hjl.tools@gmail.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=szabolcs.nagy@arm.com \
    --cc=will@kernel.org \
    --cc=yu-cheng.yu@intel.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).