Message-ID: <67da8e11-c9b1-3445-bf7d-3854cea72b7e@amd.com>
Date:   Thu, 3 Nov 2022 12:39:30 -0500
From:   John Allen <john.allen@....com>
To:     "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "nathan@...nel.org" <nathan@...nel.org>
Cc:     "bsingharora@...il.com" <bsingharora@...il.com>,
        "Syromiatnikov, Eugene" <esyr@...hat.com>,
        "babu.moger@....com" <babu.moger@....com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "Eranian, Stephane" <eranian@...gle.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "fweimer@...hat.com" <fweimer@...hat.com>,
        "nadav.amit@...il.com" <nadav.amit@...il.com>,
        "jannh@...gle.com" <jannh@...gle.com>,
        "dethoma@...rosoft.com" <dethoma@...rosoft.com>,
        "kcc@...gle.com" <kcc@...gle.com>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        "bp@...en8.de" <bp@...en8.de>, "oleg@...hat.com" <oleg@...hat.com>,
        "hjl.tools@...il.com" <hjl.tools@...il.com>,
        "pavel@....cz" <pavel@....cz>,
        "Lutomirski, Andy" <luto@...nel.org>,
        "thomas.lendacky@....com" <thomas.lendacky@....com>,
        "jamorris@...ux.microsoft.com" <jamorris@...ux.microsoft.com>,
        "arnd@...db.de" <arnd@...db.de>,
        "Moreira, Joao" <joao.moreira@...el.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mike.kravetz@...cle.com" <mike.kravetz@...cle.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "gustavoars@...nel.org" <gustavoars@...nel.org>,
        "rppt@...nel.org" <rppt@...nel.org>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        "ndesaulniers@...gle.com" <ndesaulniers@...gle.com>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "corbet@....net" <corbet@....net>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Yang, Weijiang" <weijiang.yang@...el.com>,
        "gorcunov@...il.com" <gorcunov@...il.com>
Subject: Re: [PATCH v2 33/39] x86/cpufeatures: Limit shadow stack to Intel
 CPUs

On 10/4/22 6:24 PM, Edgecombe, Rick P wrote:
> On Tue, 2022-10-04 at 14:17 -0700, H. Peter Anvin wrote:
>> On October 4, 2022 1:50:20 PM PDT, Nathan Chancellor <
>> nathan@...nel.org> wrote:
>>> On Tue, Oct 04, 2022 at 08:34:54PM +0000, Edgecombe, Rick P wrote:
>>>> On Tue, 2022-10-04 at 14:43 -0500, John Allen wrote:
>>>>> On 10/4/22 10:47 AM, Nathan Chancellor wrote:
>>>>>> Hi Kees,
>>>>>>
>>>>>> On Mon, Oct 03, 2022 at 09:54:26PM -0700, Kees Cook wrote:
>>>>>>> On Mon, Oct 03, 2022 at 05:09:04PM -0700, Dave Hansen
>>>>>>> wrote:
>>>>>>>> On 10/3/22 16:57, Kees Cook wrote:
>>>>>>>>> On Thu, Sep 29, 2022 at 03:29:30PM -0700, Rick
>>>>>>>>> Edgecombe
>>>>>>>>> wrote:
>>>>>>>>>> Shadow stack is supported on newer AMD processors,
>>>>>>>>>> but the
>>>>>>>>>> kernel
>>>>>>>>>> implementation has not been tested on them. Prevent
>>>>>>>>>> basic
>>>>>>>>>> issues from
>>>>>>>>>> showing up for normal users by disabling shadow stack
>>>>>>>>>> on
>>>>>>>>>> all CPUs except
>>>>>>>>>> Intel until it has been tested. At which point the
>>>>>>>>>> limitation should be
>>>>>>>>>> removed.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Rick Edgecombe <
>>>>>>>>>> rick.p.edgecombe@...el.com>
>>>>>>>>>
>>>>>>>>> So running the selftests on an AMD system is sufficient
>>>>>>>>> to
>>>>>>>>> drop this
>>>>>>>>> patch?
>>>>>>>>
>>>>>>>> Yes, that's enough.
>>>>>>>>
>>>>>>>> I _thought_ the AMD folks provided some tested-by's at
>>>>>>>> some
>>>>>>>> point in the
>>>>>>>> past.  But, maybe I'm confusing this for one of the other
>>>>>>>> shared
>>>>>>>> features.  Either way, I'm sure no tested-by's were
>>>>>>>> dropped on
>>>>>>>> purpose.
>>>>>>>>
>>>>>>>> I'm sure Rick is eager to trim down his series and this
>>>>>>>> would
>>>>>>>> be a great
>>>>>>>> patch to drop.  Does anyone want to make that easy for
>>>>>>>> Rick?
>>>>>>>>
>>>>>>>> <hint> <hint>
>>>>>>>
>>>>>>> Hey Gustavo, Nathan, or Nick! I know y'all have some fancy
>>>>>>> AMD
>>>>>>> testing
>>>>>>> rigs. Got a moment to spin up this series and run the
>>>>>>> selftests?
>>>>>>> :)
>>>>>>
>>>>>> I do have access to a system with an EPYC 7513, which does
>>>>>> have
>>>>>> Shadow
>>>>>> Stack support (I can see 'shstk' in the "Flags" section of
>>>>>> lscpu
>>>>>> with
>>>>>> this series). As far as I understand it, AMD only added
>>>>>> Shadow
>>>>>> Stack
>>>>>> with Zen 3; my regular AMD test system is Zen 2 (probably
>>>>>> should
>>>>>> look at
>>>>>> procurring a Zen 3 or Zen 4 one at some point).
>>>>>>
>>>>>> I applied this series on top of 6.0 and reverted this change
>>>>>> then
>>>>>> booted
>>>>>> it on that system. After building the selftest (which did
>>>>>> require
>>>>>> 'make headers_install' and a small addition to make it build
>>>>>> beyond
>>>>>> that, see below), I ran it and this was the result. I am not
>>>>>> sure
>>>>>> if
>>>>>> that is expected or not but the other results seem promising
>>>>>> for
>>>>>> dropping this patch.
>>>>>>
>>>>>>    $ ./test_shadow_stack_64
>>>>>>    [INFO]  new_ssp = 7f8a36c9fff8, *new_ssp = 7f8a36ca0001
>>>>>>    [INFO]  changing ssp from 7f8a374a0ff0 to 7f8a36c9fff8
>>>>>>    [INFO]  ssp is now 7f8a36ca0000
>>>>>>    [OK]    Shadow stack pivot
>>>>>>    [OK]    Shadow stack faults
>>>>>>    [INFO]  Corrupting shadow stack
>>>>>>    [INFO]  Generated shadow stack violation successfully
>>>>>>    [OK]    Shadow stack violation test
>>>>>>    [INFO]  Gup read -> shstk access success
>>>>>>    [INFO]  Gup write -> shstk access success
>>>>>>    [INFO]  Violation from normal write
>>>>>>    [INFO]  Gup read -> write access success
>>>>>>    [INFO]  Violation from normal write
>>>>>>    [INFO]  Gup write -> write access success
>>>>>>    [INFO]  Cow gup write -> write access success
>>>>>>    [OK]    Shadow gup test
>>>>>>    [INFO]  Violation from shstk access
>>>>>>    [OK]    mprotect() test
>>>>>>    [OK]    Userfaultfd test
>>>>>>    [FAIL]  Alt shadow stack test
>>>>>
>>>>> The selftest is looking OK on my system (Dell PowerEdge R6515
>>>>> w/ EPYC
>>>>> 7713). I also just pulled a fresh 6.0 kernel and applied the
>>>>> series
>>>>> including the fix Nathan mentions below.
>>>>>
>>>>> $ tools/testing/selftests/x86/test_shadow_stack_64
>>>>> [INFO]  new_ssp = 7f30cccc5ff8, *new_ssp = 7f30cccc6001
>>>>> [INFO]  changing ssp from 7f30cd4c6ff0 to 7f30cccc5ff8
>>>>> [INFO]  ssp is now 7f30cccc6000
>>>>> [OK]    Shadow stack pivot
>>>>> [OK]    Shadow stack faults
>>>>> [INFO]  Corrupting shadow stack
>>>>> [INFO]  Generated shadow stack violation successfully
>>>>> [OK]    Shadow stack violation test
>>>>> [INFO]  Gup read -> shstk access success
>>>>> [INFO]  Gup write -> shstk access success
>>>>> [INFO]  Violation from normal write
>>>>> [INFO]  Gup read -> write access success
>>>>> [INFO]  Violation from normal write
>>>>> [INFO]  Gup write -> write access success
>>>>> [INFO]  Cow gup write -> write access success
>>>>> [OK]    Shadow gup test
>>>>> [INFO]  Violation from shstk access
>>>>> [OK]    mprotect() test
>>>>> [OK]    Userfaultfd test
>>>>> [OK]    Alt shadow stack test.
>>>>
>>>> Thanks for the testing. Based on the test, I wonder if this could
>>>> be a
>>>> SW bug. Nathan, could I send you a tweaked test with some more
>>>> debug
>>>> information?
>>>
>>> Yes, more than happy to help you look into this further!
>>>
>>>> John, are we sure AMD and Intel systems behave the same with
>>>> respect to
>>>> CPUs not creating Dirty=1,Write=0 PTEs in rare situations? Or any
>>>> other
>>>> CET related differences we should hash out? Otherwise I'll drop
>>>> the
>>>> patch for the next version. (and assuming the issue Nathan hit
>>>> doesn't
>>>> turn up anything HW related).
>>
>> I have to admit to being a bit confused here... in general, we trust
>> CPUID bits unless they are *known* to be wrong, in which case we
>> blacklist them.
>>
>> If AMD advertises the feature but it doesn't work or they didn't
>> validate it, that would be a (serious!) bug on their part that we can
>> address by blacklisting, but they should also fix with a
>> microcode/BIOS patch.
>>
>> What am I missing?
> 
> I have not read anything about the AMD implementation except hearing
> that it is supported. But there are some microarchitectual-like aspects
> to this CET Linux implementation, around requiring CPUs to not create
> Dirty=1,Write=0 PTEs in some cases, where they did in the past. In
> another thread Jann asked how the IOMMU works with respect to this edge
> case and I'm currently trying to chase down that answer for even Intel
> HW. So I just wanted to double check that we expect that everything
> should be the same. In either case we still have time to iron things
> out before anything gets upstream.

Hi Rick,

Sorry for the delayed reply. After asking around, I think you can safely
assume that AMD will not create Dirty=1,Write=0 PTEs in rare
circumstances and shadow stack should behave the same as Intel in that
regard.

Thanks,
John

Powered by blists - more mailing lists