| Message ID | 20251210151046.37177-1-wxjstz@126.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/1] firmware: Initialize stack guard via Zkr | expand |
On Wed, Dec 10, 2025 at 8:40 PM Xiang W <wxjstz@126.com> wrote: > > Try to initialize stack protection guard via the zkr extension. > > Signed-off-by: Xiang W <wxjstz@126.com> > Reviewed-by: Anup Patel <anup@brainfault.org> I think my Reviewed-by was premature previously. Please see few comments below. > --- > v2 changes: add '[PATCH]' > > firmware/fw_base.S | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/firmware/fw_base.S b/firmware/fw_base.S > index 5300ecf2..953fba95 100644 > --- a/firmware/fw_base.S > +++ b/firmware/fw_base.S > @@ -107,6 +107,30 @@ _bss_zero: > add s4, s4, __SIZEOF_POINTER__ > blt s4, s5, _bss_zero > > + /* Trying to initialize the stack guard via the Zkr extension */ > + lla t0, __stack_chk_guard_done > + csrw CSR_MTVEC, t0 > + li t0, 0 > + li t3, 2 > + li t4, 0xffff > + li t5, __SIZEOF_POINTER__ > +__stack_chk_guard_loop: > + csrrw t1, CSR_SEED, x0 > + srli t2, t1, 30 > + andi t2, t2, 3 We have various hard-coded values related to SEED CSR over here. Please introduce appropriate defines in riscv_encoding.h and use it over here. > + bgt t2, t3, __stack_chk_guard_done > + blt t2, t3, __stack_chk_guard_loop > + and t1, t1, t4 > + slli t0, t0, 16 > + or t0, t0, t1 > + addi t5, t5, -2 Should this not be "addi t5, t5, -16" since we are reading 16 bits of entropy in one loop iteration ? > + bgtz t5, __stack_chk_guard_loop > + lla t1, __stack_chk_guard > + REG_S t0, 0(t1) > + j __stack_chk_guard_done > + .align 3 > +__stack_chk_guard_done: > + > /* Setup temporary trap handler */ > lla s4, _start_hang > csrw CSR_MTVEC, s4 > -- > 2.47.3 > Regards, Anup
在 2025-12-28日的 21:04 +0530，Anup Patel写道： > On Wed, Dec 10, 2025 at 8:40 PM Xiang W <wxjstz@126.com> wrote: > > > > Try to initialize stack protection guard via the zkr extension. > > > > Signed-off-by: Xiang W <wxjstz@126.com> > > Reviewed-by: Anup Patel <anup@brainfault.org> > > I think my Reviewed-by was premature previously. Please see > few comments below. > > > --- > > v2 changes: add '[PATCH]' > > > > firmware/fw_base.S | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/firmware/fw_base.S b/firmware/fw_base.S > > index 5300ecf2..953fba95 100644 > > --- a/firmware/fw_base.S > > +++ b/firmware/fw_base.S > > @@ -107,6 +107,30 @@ _bss_zero: > > add s4, s4, __SIZEOF_POINTER__ > > blt s4, s5, _bss_zero > > > > + /* Trying to initialize the stack guard via the Zkr extension */ > > + lla t0, __stack_chk_guard_done > > + csrw CSR_MTVEC, t0 > > + li t0, 0 > > + li t3, 2 > > + li t4, 0xffff > > + li t5, __SIZEOF_POINTER__ > > +__stack_chk_guard_loop: > > + csrrw t1, CSR_SEED, x0 > > + srli t2, t1, 30 > > + andi t2, t2, 3 > > We have various hard-coded values related to SEED > CSR over here. Please introduce appropriate defines in > riscv_encoding.h and use it over here. Please review at: https://lists.infradead.org/pipermail/opensbi/2026-January/009301.html > > > + bgt t2, t3, __stack_chk_guard_done > > + blt t2, t3, __stack_chk_guard_loop > > + and t1, t1, t4 > > + slli t0, t0, 16 > > + or t0, t0, t1 > > + addi t5, t5, -2 > > Should this not be "addi t5, t5, -16" since we are > reading 16 bits of entropy in one loop iteration ? t5 is initialized with bytes size, not bits size. Regards, Xiang W > > > + bgtz t5, __stack_chk_guard_loop > > + lla t1, __stack_chk_guard > > + REG_S t0, 0(t1) > > + j __stack_chk_guard_done > > + .align 3 > > +__stack_chk_guard_done: > > + > > /* Setup temporary trap handler */ > > lla s4, _start_hang > > csrw CSR_MTVEC, s4 > > -- > > 2.47.3 > > > > Regards, > Anup
diff --git a/firmware/fw_base.S b/firmware/fw_base.S index 5300ecf2..953fba95 100644 --- a/firmware/fw_base.S +++ b/firmware/fw_base.S @@ -107,6 +107,30 @@ _bss_zero: add s4, s4, __SIZEOF_POINTER__ blt s4, s5, _bss_zero + /* Trying to initialize the stack guard via the Zkr extension */ + lla t0, __stack_chk_guard_done + csrw CSR_MTVEC, t0 + li t0, 0 + li t3, 2 + li t4, 0xffff + li t5, __SIZEOF_POINTER__ +__stack_chk_guard_loop: + csrrw t1, CSR_SEED, x0 + srli t2, t1, 30 + andi t2, t2, 3 + bgt t2, t3, __stack_chk_guard_done + blt t2, t3, __stack_chk_guard_loop + and t1, t1, t4 + slli t0, t0, 16 + or t0, t0, t1 + addi t5, t5, -2 + bgtz t5, __stack_chk_guard_loop + lla t1, __stack_chk_guard + REG_S t0, 0(t1) + j __stack_chk_guard_done + .align 3 +__stack_chk_guard_done: + /* Setup temporary trap handler */ lla s4, _start_hang csrw CSR_MTVEC, s4