Re: [PATCH v6 16/33] riscv/shstk: If needed allocate a new shadow stack on clone

From: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
To: "debug@rivosinc.com" <debug@rivosinc.com>
Cc: "kito.cheng@sifive.com" <kito.cheng@sifive.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"lorenzo.stoakes@oracle.com" <lorenzo.stoakes@oracle.com>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"charlie@rivosinc.com" <charlie@rivosinc.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"samitolvanen@google.com" <samitolvanen@google.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"kees@kernel.org" <kees@kernel.org>,
	"alistair.francis@wdc.com" <alistair.francis@wdc.com>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"andybnac@gmail.com" <andybnac@gmail.com>,
	"krzk+dt@kernel.org" <krzk+dt@kernel.org>,
	"palmer@dabbelt.com" <palmer@dabbelt.com>,
	"x86@kernel.org" <x86@kernel.org>, "bp@alien8.de" <bp@alien8.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>,
	"aou@eecs.berkeley.edu" <aou@eecs.berkeley.edu>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"jim.shu@sifive.com" <jim.shu@sifive.com>,
	"vbabka@suse.cz" <vbabka@suse.cz>,
	"shuah@kernel.org" <shuah@kernel.org>,
	"Liam.Howlett@oracle.com" <Liam.Howlett@oracle.com>,
	"oleg@redhat.com" <oleg@redhat.com>,
	"alexghiti@rivosinc.com" <alexghiti@rivosinc.com>,
	"ebiederm@xmission.com" <ebiederm@xmission.com>,
	"atishp@rivosinc.com" <atishp@rivosinc.com>,
	"richard.henderson@linaro.org" <richard.henderson@linaro.org>,
	"cleger@rivosinc.com" <cleger@rivosinc.com>,
	"brauner@kernel.org" <brauner@kernel.org>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"robh@kernel.org" <robh@kernel.org>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>,
	"paul.walmsley@sifive.com" <paul.walmsley@sifive.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"evan@rivosinc.com" <evan@rivosinc.com>,
	"conor@kernel.org" <conor@kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>
Subject: Re: [PATCH v6 16/33] riscv/shstk: If needed allocate a new shadow stack on clone
Date: Tue, 8 Oct 2024 23:31:16 +0000	[thread overview]
Message-ID: <93a3315b3acd8a0585fe266bdfdbd44e54aabaee.camel@intel.com> (raw)
In-Reply-To: <ZwW9m6pqcTFBovuG@debug.ba.rivosinc.com>

On Tue, 2024-10-08 at 16:17 -0700, Deepak Gupta wrote:
> Yeah you're right. Honestly, I've been shameless in adapting most of the flows
> from x86 `shstk.c` for risc-v. So thank you for that.

All good, glad we ended up with similar behavior.

> 
> Now that we've `ARCH_HAS_USER_SHADOW_STACK` part of multiple patch series (riscv
> shadowstack, clone3 and I think arm64 gcs series as well). It's probably the
> appropriate time to find common grounds.

There have been bugs in the similar bits of code. So will be nice to not have to
fix them in each arch too.

> 
> This is what I suggest
> 
> - move most of the common/arch agnostic shadow stack stuff in kernel/shstk.c
>    This gets part of compile if `ARCH_HAS_USER_SHADOW_STACK` is enabled/selected.

Yea, I guess we have commonality for (in x86 naming):
 - map_shadow_stack()
 - shstk_free()
 - shstk_alloc_thread_stack()
 - shstk_setup()

The signal part starts to diverge. Then I guess x86 has a different prctl
interface.

> 
> - allow arch specific branch out guard checks for "if cpu supports", "is shadow stack
>    enabled on the task_struct" (I expect each arch layout of task_struct will be
>    different, no point finding common ground there), etc.

Sure.

> 
> I think it's worth a try. 
> If you already don't have patches, I'll spend some time to see what it takes to
> converge in my next version. If I end up into some roadblock, will use this thread
> for further discussion.

Sounds good. I have not looked at it too much.