Message-ID: <ZKw3zSKxCug0IbC1@arm.com>
Date:   Mon, 10 Jul 2023 17:54:37 +0100
From:   "szabolcs.nagy@....com" <szabolcs.nagy@....com>
To:     "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
        "Lutomirski, Andy" <luto@...nel.org>
Cc:     "Xu, Pengfei" <pengfei.xu@...el.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        "kcc@...gle.com" <kcc@...gle.com>,
        "nadav.amit@...il.com" <nadav.amit@...il.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "david@...hat.com" <david@...hat.com>,
        "Schimpe, Christina" <christina.schimpe@...el.com>,
        "Yang, Weijiang" <weijiang.yang@...el.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "corbet@....net" <corbet@....net>, "nd@....com" <nd@....com>,
        "dethoma@...rosoft.com" <dethoma@...rosoft.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>, "pavel@....cz" <pavel@....cz>,
        "bp@...en8.de" <bp@...en8.de>,
        "debug@...osinc.com" <debug@...osinc.com>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        "rppt@...nel.org" <rppt@...nel.org>,
        "jamorris@...ux.microsoft.com" <jamorris@...ux.microsoft.com>,
        "arnd@...db.de" <arnd@...db.de>,
        "john.allen@....com" <john.allen@....com>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "mike.kravetz@...cle.com" <mike.kravetz@...cle.com>,
        "jannh@...gle.com" <jannh@...gle.com>,
        "oleg@...hat.com" <oleg@...hat.com>,
        "andrew.cooper3@...rix.com" <andrew.cooper3@...rix.com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "gorcunov@...il.com" <gorcunov@...il.com>,
        "fweimer@...hat.com" <fweimer@...hat.com>,
        "Yu, Yu-cheng" <yu-cheng.yu@...el.com>,
        "bsingharora@...il.com" <bsingharora@...il.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "hjl.tools@...il.com" <hjl.tools@...il.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "Syromiatnikov, Eugene" <esyr@...hat.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "Torvalds, Linus" <torvalds@...ux-foundation.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "Eranian, Stephane" <eranian@...gle.com>
Subject: Re: [PATCH v9 23/42] Documentation/x86: Add CET shadow stack
 description

The 07/07/2023 17:37, Edgecombe, Rick P wrote:
> On Fri, 2023-07-07 at 16:25 +0100, szabolcs.nagy@....com wrote:
> > > Separate from all of this...now that all the constraints are
> > > clearer,
> > > if you have changed your mind on whether this series is ready,
> > > could
> > > you comment at the top of this thread something to that effect? I'm
> > > imagining not many are reading so far down at this point.
> > > 
> > > For my part, I think we should go forward with what we have on the
> > > kernel side, unless glibc/gcc developers would like to start by
> > > deprecating the existing binaries. I just talked with HJ, and he
> > > has
> > > not changed his plans around this. If anyone else in that community
> > > has
> > > (Florian?), please speak up. But otherwise I think it's better to
> > > start
> > > getting real world feedback and grow based on that.
> > > 
> > 
> > the x86 v1 abi tries to be compatible with existing unwinders.
> > (are there other binaries that constrains v1? portable code
> > should be fine as they rely on libc which we can still change)
> > 
> > i will have to discuss the arm plan with the arm kernel devs.
> > the ugly bit i want to avoid on arm is to have to reimplement
> > unwind and jump ops to make alt shadow stack work in a v2 abi.
> > 
> > i think the worse bit of the x86 v1 abi is that crash handlers
> > don't work reliably (e.g. a crash on a tiny makecontext stack
> > with the usual sigaltstack crash handler can unrecoverably fail
> > during crash handling). i guess this can be somewhat mitigated
> > by both linux and libc adding an extra page to the shadow stack
> > size to guarantee that alt stack handlers with certain depth
> > always work.
> 
> Some mails back, I listed the three things you might be asking for from
> the kernel side and pointedly asked you to clarify. The only one you
> still were wishing for up front was "Leave a token on switching to an
> alt shadow stack."
> 
> But how you want to use this involves a lot of details for how glibc
> will work (automatic shadow stack for sigaltstack, scan-restore-incssp,
> etc). I think you first need to get the story straight with other libc
> developers, otherwise this is just brainstorming. I'm not a glibc
> contributor, so winning me over is only half the battle.
> 
> Only after that is settled do we get to the problem of the old libgcc
> unwinders, and how it is a challenge to even add alt shadow stack given
> glibc's plans and the existing binaries.
> 
> Once that is solved we are at the overflow problem, and the current
> state of thinking on that is "i'm fairly sure this can be done (but
> indeed complicated)".
> 
> So I think we are still missing any actionable requests that should
> hold this up.
> 
> Is this a reasonable summary?

not entirely.

the high level requirement is a design that

a) does not break many existing sigaltstack uses,

b) allows implementing jump and unwind that support the
   relevant use-cases around signals and stack switches
   with minimal userspace changes.

where (b) has nothing to add to v1 abi: existing unwind
binaries mean this needs a v2 abi. (the point of discussing
v2 ahead of time is to understand the cost of v2 and the
divergence wrt targets without abi compat issue.)

for (a) my actionable suggestion was to account altstack
when sizing shadow stacks. to document an altstack call
depth limit on the libc level (e.g. fixed 100 is fine) we
need guarantees from the kernel. (consider recursive calls
overflowing the stack with altstack crash handler: for this
to be reliable shadow stack size > stack size is needed.
but the diff can be tiny e.g. 1 page is enough.)

your previous 3 actionable item list was

1. add token when handling signals on altstack.

this falls under (b). your summary is correct that this
requires sorting out many fiddly details.

2. top of stack token.

this can work differently across targets so i have nothing
against the x86 v1 abi, but on arm64 we plan to have this.

3. more shadow stack sizing policies.

this can be done in the future except the default policy
should be fixed for (a) and a smaller size introduces the
overflow issue which may require v2.

in short the only important change for v1 is shstk sizing.

Powered by blists - more mailing lists