Message-ID: <f22c3d6a-1468-74d0-1bca-74091f0c1eff@linux.intel.com>
Date:   Mon, 6 Mar 2023 15:31:10 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v7 01/41] Documentation/x86: Add CET shadow stack
 description
Content-Language: en-US
To:     "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>,
        "david@redhat.com" <david@redhat.com>,
        "bsingharora@gmail.com" <bsingharora@gmail.com>,
        "hpa@zytor.com" <hpa@zytor.com>,
        "Syromiatnikov, Eugene" <esyr@redhat.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "rdunlap@infradead.org" <rdunlap@infradead.org>,
        "keescook@chromium.org" <keescook@chromium.org>,
        "Eranian, Stephane" <eranian@google.com>,
        "kirill.shutemov@linux.intel.com" <kirill.shutemov@linux.intel.com>,
        "szabolcs.nagy@arm.com" <szabolcs.nagy@arm.com>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "fweimer@redhat.com" <fweimer@redhat.com>,
        "nadav.amit@gmail.com" <nadav.amit@gmail.com>,
        "jannh@google.com" <jannh@google.com>,
        "dethoma@microsoft.com" <dethoma@microsoft.com>,
        "broonie@kernel.org" <broonie@kernel.org>,
        "kcc@google.com" <kcc@google.com>,
        "linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
        "bp@alien8.de" <bp@alien8.de>, "oleg@redhat.com" <oleg@redhat.com>,
        "hjl.tools@gmail.com" <hjl.tools@gmail.com>,
        "Yang, Weijiang" <weijiang.yang@intel.com>,
        "Lutomirski, Andy" <luto@kernel.org>,
        "pavel@ucw.cz" <pavel@ucw.cz>, "arnd@arndb.de" <arnd@arndb.de>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "Schimpe, Christina" <christina.schimpe@intel.com>,
        "mike.kravetz@oracle.com" <mike.kravetz@oracle.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "debug@rivosinc.com" <debug@rivosinc.com>,
        "jamorris@linux.microsoft.com" <jamorris@linux.microsoft.com>,
        "john.allen@amd.com" <john.allen@amd.com>,
        "rppt@kernel.org" <rppt@kernel.org>,
        "andrew.cooper3@citrix.com" <andrew.cooper3@citrix.com>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "corbet@lwn.net" <corbet@lwn.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
        "gorcunov@gmail.com" <gorcunov@gmail.com>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>
Cc:     "Liang, Kan" <kan.liang@intel.com>,
        "Yu, Yu-cheng" <yu-cheng.yu@intel.com>, "nd@arm.com" <nd@arm.com>
References: <Y/9fdYQ8Cd0GI+8C@arm.com>
 <636de4a28a42a082f182e940fbd8e63ea23895cc.camel@intel.com>
 <df8ef3a9e5139655a223589c16a68393ab3f6d1d.camel@intel.com>
 <ZADQISkczejfgdoS@arm.com>
 <9714f724b53b04fdf69302c6850885f5dfbf3af5.camel@intel.com>
 <ZAYS6CHuZ0MiFvmE@arm.com>
 <7b571c394839073cdc338b6718a363f44c9ba072.camel@intel.com>
From:   "Liang, Kan" <kan.liang@linux.intel.com>
In-Reply-To: <7b571c394839073cdc338b6718a363f44c9ba072.camel@intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2023-03-06 1:05 p.m., Edgecombe, Rick P wrote:
> +Kan for shadow stack perf discussion.
> 
> On Mon, 2023-03-06 at 16:20 +0000, szabolcs.nagy@arm.com wrote:
>> The 03/03/2023 22:35, Edgecombe, Rick P wrote:
>>> I think I slightly prefer the former arch_prctl() based solution
>>> for a
>>> few reasons:
>>>   - When you need to find the start or end of the shadow stack can
>>> you
>>> can just ask for it instead of searching. It can be faster and
>>> simpler.
>>>   - It saves 8 bytes of memory per shadow stack.
>>>
>>> If this turns out to be wrong and we want to do the marker solution
>>> much later at some point, the safest option would probably be to
>>> create
>>> new flags.
>>
>> i see two problems with a get bounds syscall:
>>
>> - syscall overhead.
>>
>> - discontinous shadow stack (e.g. alt shadow stack ends with a
>>   pointer to the interrupted thread shadow stack, so stack trace
>>   can continue there, except you don't know the bounds of that).
>>
>>> But just discussing this with HJ, can you share more on what the
>>> usage
>>> is? Like which backtracing operation specifically needs the marker?
>>> How
>>> much does it care about the ucontext case?
>>
>> it could be an option for perf or ptracers to sample the stack trace.
>>
>> in-process collection of stack trace for profiling or crash reporting
>> (e.g. when stack is corrupted) or cross checking stack integrity may
>> use it too.
>>
>> sometimes parsing /proc/self/smaps maybe enough, but the idea was to
>> enable light-weight backtrace collection in an async-signal-safe way.
>>
>> syscall overhead in case of frequent stack trace collection can be
>> avoided by caching (in tls) when ssp falls within the thread shadow
>> stack bounds. otherwise caching does not work as the shadow stack may
>> be reused (alt shadow stack or ucontext case).
>>
>> unfortunately i don't know if syscall overhead is actually a problem
>> (probably not) or if backtrace across signal handlers need to work
>> with alt shadow stack (i guess it should work for crash reporting).
> 
> There was a POC done of perf integration. I'm not too knowledgeable on
> perf, but the patch itself didn't need any new shadow stack bounds ABI.
> Since it was implemented in the kernel, it could just refer to the
> kernel's internal data for the thread's shadow stack bounds.
> 
> I asked about ucontext (similar to alt shadow stacks in regards to lack
> of bounds ABI), and apparently perf usually focuses on the thread
> stacks. Hopefully Kan can lend some more confidence to that assertion.

The POC perf patch I implemented tries to use the shadow stack to
replace the frame pointer to construct a callchain of a user space
thread. Yes, it's in the kernel, perf_callchain_user(). I don't think
the current X86 perf implementation handle the alt stack either. So the
kernel internal data for the thread's shadow stack bounds should be good
enough for the perf case.

Thanks,
Kan