| Message ID | 20241111-v5_user_cfi_series-v8-24-dce14aa30207@rivosinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | riscv control-flow integrity for usermode | expand |
Hi Deepak On Tue, Nov 12, 2024 at 5:08 AM Deepak Gupta <debug@rivosinc.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@rivosinc.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? Regards, Nick > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
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@rivosinc.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@rivosinc.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 ? > >Regards, >Nick >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Deepak On Thu, Nov 14, 2024 at 9:06 AM Deepak Gupta <debug@rivosinc.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@rivosinc.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@rivosinc.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; Regards, Nick > > > >Regards, > >Nick > >> > >> _______________________________________________ > >> linux-riscv mailing list > >> linux-riscv@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-riscv
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@rivosinc.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@rivosinc.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@rivosinc.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@lists.infradead.org >> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Deepak On Thu, Nov 14, 2024 at 9:25 AM Deepak Gupta <debug@rivosinc.com> wrote: > > 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@rivosinc.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@rivosinc.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@rivosinc.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? > Current OpenSBI doesn't clear the lock in the warm init of the hotplug path. It seems like we need a patch to address it. Regards, Nick > >Regards, > >Nick > >> > > >> >Regards, > >> >Nick > >> >> > >> >> _______________________________________________ > >> >> linux-riscv mailing list > >> >> linux-riscv@lists.infradead.org > >> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Nick, Thanks for reviewing and helping. On Thu, Nov 14, 2024 at 02:17:30PM +0800, Nick Hu wrote: >Hi Deepak > >On Thu, Nov 14, 2024 at 9:25 AM Deepak Gupta <debug@rivosinc.com> wrote: >> >> 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@rivosinc.com> wrote: >> >> >> 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? >> >Current OpenSBI doesn't clear the lock in the warm init of the hotplug path. >It seems like we need a patch to address it. Got it thanks. Since you already know what's the problem, can you send a patch to opensbi. If you want rather have me do it, let me know. Thanks. > >Regards, >Nick >> >Regards, >> >Nick >> >> > >> >> >Regards, >> >> >Nick >> >> >> >> >> >> _______________________________________________ >> >> >> linux-riscv mailing list >> >> >> linux-riscv@lists.infradead.org >> >> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Deepak On Thu, Nov 14, 2024 at 11:50 PM Deepak Gupta <debug@rivosinc.com> wrote: > > > Hi Nick, > > Thanks for reviewing and helping. > > On Thu, Nov 14, 2024 at 02:17:30PM +0800, Nick Hu wrote: > >Hi Deepak > > > >On Thu, Nov 14, 2024 at 9:25 AM Deepak Gupta <debug@rivosinc.com> wrote: > >> > >> 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@rivosinc.com> wrote: > >> >> >> 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? > >> > >Current OpenSBI doesn't clear the lock in the warm init of the hotplug path. > >It seems like we need a patch to address it. > > Got it thanks. > Since you already know what's the problem, can you send a patch to opensbi. > If you want rather have me do it, let me know. Thanks. > No problem. I'll send a patch to opensbi. Regards, Nick > > > >Regards, > >Nick > >> >Regards, > >> >Nick > >> >> > > >> >> >Regards, > >> >> >Nick > >> >> >> > >> >> >> _______________________________________________ > >> >> >> linux-riscv mailing list > >> >> >> linux-riscv@lists.infradead.org > >> >> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
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
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@rivosinc.com> --- arch/riscv/kernel/asm-offsets.c | 4 ++++ arch/riscv/kernel/head.S | 12 ++++++++++++ 2 files changed, 16 insertions(+)