Message-ID: <ZzVRbCZP9N4Os8Bj@debug.ba.rivosinc.com>
Date: Wed, 13 Nov 2024 17:25:00 -0800
From: Deepak Gupta <debug@...osinc.com>
To: Nick Hu <nick.hu@...ive.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Liam R. Howlett" <Liam.Howlett@...cle.com>,
	Vlastimil Babka <vbabka@...e.cz>,
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Albert Ou <aou@...s.berkeley.edu>, Conor Dooley <conor@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Arnd Bergmann <arnd@...db.de>,
	Christian Brauner <brauner@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Eric Biederman <ebiederm@...ssion.com>, Kees Cook <kees@...nel.org>,
	Jonathan Corbet <corbet@....net>, Shuah Khan <shuah@...nel.org>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-mm@...ck.org, linux-riscv@...ts.infradead.org,
	devicetree@...r.kernel.org, linux-arch@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-kselftest@...r.kernel.org,
	alistair.francis@....com, richard.henderson@...aro.org,
	jim.shu@...ive.com, andybnac@...il.com, kito.cheng@...ive.com,
	charlie@...osinc.com, atishp@...osinc.com, evan@...osinc.com,
	cleger@...osinc.com, alexghiti@...osinc.com,
	samitolvanen@...gle.com, broonie@...nel.org,
	rick.p.edgecombe@...el.com
Subject: Re: [PATCH v8 24/29] riscv: enable kernel access to shadow stack
 memory via FWFT sbi call

On Thu, Nov 14, 2024 at 09:20:14AM +0800, Nick Hu wrote:
>Hi Deepak
>
>On Thu, Nov 14, 2024 at 9:06 AM Deepak Gupta <debug@...osinc.com> wrote:
>>
>> On Thu, Nov 14, 2024 at 12:13:38AM +0800, Nick Hu wrote:
>> >Hi Deepak
>> >
>> >On Tue, Nov 12, 2024 at 5:08 AM Deepak Gupta <debug@...osinc.com> wrote:
>> >>
>> >> Kernel will have to perform shadow stack operations on user shadow stack.
>> >> Like during signal delivery and sigreturn, shadow stack token must be
>> >> created and validated respectively. Thus shadow stack access for kernel
>> >> must be enabled.
>> >>
>> >> In future when kernel shadow stacks are enabled for linux kernel, it must
>> >> be enabled as early as possible for better coverage and prevent imbalance
>> >> between regular stack and shadow stack. After `relocate_enable_mmu` has
>> >> been done, this is as early as possible it can enabled.
>> >>
>> >> Signed-off-by: Deepak Gupta <debug@...osinc.com>
>> >> ---
>> >>  arch/riscv/kernel/asm-offsets.c |  4 ++++
>> >>  arch/riscv/kernel/head.S        | 12 ++++++++++++
>> >>  2 files changed, 16 insertions(+)
>> >>
>> >> diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
>> >> index 766bd33f10cb..a22ab8a41672 100644
>> >> --- a/arch/riscv/kernel/asm-offsets.c
>> >> +++ b/arch/riscv/kernel/asm-offsets.c
>> >> @@ -517,4 +517,8 @@ void asm_offsets(void)
>> >>         DEFINE(FREGS_A6,            offsetof(struct ftrace_regs, a6));
>> >>         DEFINE(FREGS_A7,            offsetof(struct ftrace_regs, a7));
>> >>  #endif
>> >> +       DEFINE(SBI_EXT_FWFT, SBI_EXT_FWFT);
>> >> +       DEFINE(SBI_EXT_FWFT_SET, SBI_EXT_FWFT_SET);
>> >> +       DEFINE(SBI_FWFT_SHADOW_STACK, SBI_FWFT_SHADOW_STACK);
>> >> +       DEFINE(SBI_FWFT_SET_FLAG_LOCK, SBI_FWFT_SET_FLAG_LOCK);
>> >>  }
>> >> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>> >> index 356d5397b2a2..6244408ca917 100644
>> >> --- a/arch/riscv/kernel/head.S
>> >> +++ b/arch/riscv/kernel/head.S
>> >> @@ -164,6 +164,12 @@ secondary_start_sbi:
>> >>         call relocate_enable_mmu
>> >>  #endif
>> >>         call .Lsetup_trap_vector
>> >> +       li a7, SBI_EXT_FWFT
>> >> +       li a6, SBI_EXT_FWFT_SET
>> >> +       li a0, SBI_FWFT_SHADOW_STACK
>> >> +       li a1, 1 /* enable supervisor to access shadow stack access */
>> >> +       li a2, SBI_FWFT_SET_FLAG_LOCK
>> >> +       ecall
>> >>         scs_load_current
>> >>         call smp_callin
>> >>  #endif /* CONFIG_SMP */
>> >> @@ -320,6 +326,12 @@ SYM_CODE_START(_start_kernel)
>> >>         la tp, init_task
>> >>         la sp, init_thread_union + THREAD_SIZE
>> >>         addi sp, sp, -PT_SIZE_ON_STACK
>> >> +       li a7, SBI_EXT_FWFT
>> >> +       li a6, SBI_EXT_FWFT_SET
>> >> +       li a0, SBI_FWFT_SHADOW_STACK
>> >> +       li a1, 1 /* enable supervisor to access shadow stack access */
>> >> +       li a2, SBI_FWFT_SET_FLAG_LOCK
>> >> +       ecall
>> >>         scs_load_current
>> >>
>> >>  #ifdef CONFIG_KASAN
>> >>
>> >> --
>> >> 2.45.0
>> >>
>> >Should we clear the SBI_FWFT_SET_FLAG_LOCK before the cpu hotplug
>> >otherwise the menvcfg.sse won't be set by the fwft set sbi call when
>> >the hotplug cpu back to kernel?
>>
>> Hmm...
>>
>> An incoming hotplug CPU has no features setup on it.
>> I see that `sbi_cpu_start` will supply `secondary_start_sbi` as start
>> up code for incoming CPU. `secondary_start_sbi` is in head.S which converges
>> in `.Lsecondary_start_common`. And thus hotplugged CPU should be
>> issuing shadow stack set FWFT sbi as well.
>>
>> Am I missing something ?
>>
>This is the correct flow. However the opensbi will deny it due to the
>SBI_FWFT_SET_FLAG_LOCK already being set.
>So the menvcfg.sse will not set by this flow.
>
>if (conf->flags & SBI_FWFT_SET_FLAG_LOCK)
>                return SBI_EDENIED;
>

hmm... Why?

`conf` is pointing to per-hart state in firmware.

On this incoming cpu, opensbi (or equivalent) firmware must have
ensured that this per-hart state doesn't have lock set.

Am I missing something?

>Regards,
>Nick
>> >
>> >Regards,
>> >Nick
>> >>
>> >> _______________________________________________
>> >> linux-riscv mailing list
>> >> linux-riscv@...ts.infradead.org
>> >> http://lists.infradead.org/mailman/listinfo/linux-riscv

Powered by blists - more mailing lists