linux-kernel - Re: [PATCH v3 21/36] arm64/mm: Implement map_shadow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a9ea33d31aad0c45eab41b0dcbd4913d863cc930.camel@intel.com>
Date:   Tue, 1 Aug 2023 17:07:00 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "broonie@...nel.org" <broonie@...nel.org>
CC:     "corbet@....net" <corbet@....net>,
        "ardb@...nel.org" <ardb@...nel.org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "Szabolcs.Nagy@....com" <Szabolcs.Nagy@....com>,
        "shuah@...nel.org" <shuah@...nel.org>,
        "maz@...nel.org" <maz@...nel.org>,
        "james.morse@....com" <james.morse@....com>,
        "debug@...osinc.com" <debug@...osinc.com>,
        "aou@...s.berkeley.edu" <aou@...s.berkeley.edu>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "hjl.tools@...il.com" <hjl.tools@...il.com>,
        "paul.walmsley@...ive.com" <paul.walmsley@...ive.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "oleg@...hat.com" <oleg@...hat.com>,
        "arnd@...db.de" <arnd@...db.de>,
        "ebiederm@...ssion.com" <ebiederm@...ssion.com>,
        "will@...nel.org" <will@...nel.org>,
        "suzuki.poulose@....com" <suzuki.poulose@....com>,
        "kvmarm@...ts.linux.dev" <kvmarm@...ts.linux.dev>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        "oliver.upton@...ux.dev" <oliver.upton@...ux.dev>,
        "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
        "palmer@...belt.com" <palmer@...belt.com>
Subject: Re: [PATCH v3 21/36] arm64/mm: Implement map_shadow_stack()

On Tue, 2023-08-01 at 15:01 +0100, Mark Brown wrote:
> On Mon, Jul 31, 2023 at 11:19:34PM +0000, Edgecombe, Rick P wrote:
> 
> > The thing I was trying to get at was, we have this shared syscall
> > that
> > means create shadow stack memory and prepopulate it like this flag
> > says. On x86 we optionally support SHADOW_STACK_SET_TOKEN which
> > means
> > put a token right at the end of size. So maybe arm should have a
> > different flag value that includes putting the marker and then the
> > token, and x86 could match it someday if we get markers too.
> 
> Oh, I see.  My mental model was that this was controlling the whole
> thing we put at the top rather than treating the terminator and the
> cap
> separately.
> 
> > It could be a different flag, like SHADOW_STACK_SET_TOKEN_MARKER,
> > or it
> > could be SHADOW_STACK_SET_MARKER, and callers could pass
> > (SHADOW_STACK_SET_TOKEN | SHADOW_STACK_SET_MARKER) to get what you
> > have
> > implemented here. What do you think?
> 
> For arm64 code this would mean that it would be possible (and fairly
> easy) to create stacks which don't have a termination record which
> would
> make life harder for unwinders to rely on.  I don't think this is
> insurmountable, creating manually shouldn't be the standard and it'll
> already be an issue on x86 anyway.

If you are going to support optionally writing to shadow stacks (which
x86 needed for CRIU, and also seems like a nice thing for several other
reasons), you are already at that point. Can't you also do a bunch of
gcspopm's to the top of the GCS stack, and have no marker to hit before
the end of the stack? (maybe not in GCS, I don't know...)

> 
> The other minor issue is that the current arm64 marker is all bits 0
> so by itself for arm64 _MARKER would have no perceptible impact, it
> would only serve to push the token down a slot in the stack (I'm
> guessing that's the intended meaning?).

Pushing the token down a frame is what flags==0 does in this patch,
right?

You don't have to support all the flags actually, you could just
support the one mode you already have and reject all other
combinations... Then it matches between arch's, and you still have the
guaranteed-ish end marker.

So the question is not what mode should arm support, but should we have
the flags match between x86 and ARM?

>   I'm not sure that's a
> particularly big deal though.

Yea, it's not a big problem either way.