Messages in this thread | ![/](/images/icornerl.gif) | | From | Sami Tolvanen <> | Date | Thu, 11 Apr 2024 17:47:10 +0000 | Subject | Re: [RFC PATCH 08/12] riscv: dynamic (zicfiss) shadow call stack support |
| |
On Thu, Apr 11, 2024 at 5:30 PM Deepak Gupta <debug@rivosinc.com> wrote: > > On Thu, Apr 11, 2024 at 05:05:38PM +0000, Sami Tolvanen wrote: > >Hi Deepak, > > > >Thanks for the patches! > > > >On Tue, Apr 9, 2024 at 6:12 AM Deepak Gupta <debug@rivosinc.com> wrote: > >> > >> Adding support for dynamic shadow call stack on riscv. zicfiss ISA extn. > >> enables protection for shadow stack against stray writes. This patch > >> enables scs_* macros to use zicfiss shadow stack pointer (CSR_SSP) instead > >> of relying on `gp`. > > > >CONFIG_DYNAMIC_SCS implies that runtime patching is used to select > >between software SCS and an alternative hardware implementation (in > >arm64's case, PAC instead of hardware shadow stacks). I understand > >this series is still an RFC, but I didn't see runtime patching > >support. Are you planning on implementing this later? > > Since I didn't see any example on selecting PAC when `CONFIG_DYNAMIC_SCS` > is selected. So I had that confusion but wasn't sure. I thought of doing it > but I don't know how to binary rewrite all the functions. It might be too much. > So I went ahead with using `CONFIG_DYNAMIC_SCS` in this RFC series. > > Question: > If arm64 were to use PAC with CONFIG_DYNAMIC_SCS, how would it fixup the code > sequences already setup by compiler for shadow stack push and pop in runtime? > You expect this to be some offline process using some object editing tool or > a runtime decision?
We use unwind tables for locating instructions to patch, look for UNWIND_PATCH_PAC_INTO_SCS. The actual patching code is in arch/arm64/kernel/pi/patch-scs.c. I suspect this is going to be a bit trickier when patching between two shadow stack options though.
> >If there's no plan to actually patch between Zicfiss and SCS at > >runtime, CONFIG_DYNAMIC_SCS doesn't seem like the appropriate choice > >and we might need a separate config option that still allows you to > >reuse most of the software SCS code. > > I wanted to avoid "#ifdef RISCV_SPECIFIC_HW_SHSTK" in arch agnostic scs code. > And that's why went with CONFIG_DYNAMIC_SCS which sets dynamic static key once. > And then I use `is_dynamic` everywhere else in arch agnostic scs code.
We could define arch_ functions for any architecture-specific code (with a weak default implementation), and maybe add a config option for specifying which way the shadow stack grows?
Sami
| ![\](/images/icornerr.gif) |