Messages in this thread | | | From | Sami Tolvanen <> | Date | Mon, 14 Aug 2023 11:57:30 -0700 | Subject | Re: [PATCH 0/5] riscv: SCS support |
| |
On Mon, Aug 14, 2023 at 11:33 AM Kees Cook <keescook@chromium.org> wrote: > > On Mon, Aug 14, 2023 at 10:59:28AM -0700, Nathan Chancellor wrote: > > Hi Sami, > > > > On Fri, Aug 11, 2023 at 11:35:57PM +0000, Sami Tolvanen wrote: > > > Hi folks, > > > > > > This series adds Shadow Call Stack (SCS) support for RISC-V. SCS > > > uses compiler instrumentation to store return addresses in a > > > separate shadow stack to protect them against accidental or > > > malicious overwrites. More information about SCS can be found > > > here: > > > > > > https://clang.llvm.org/docs/ShadowCallStack.html > > > > > > Patch 1 is from Deepak, and it simplifies VMAP_STACK overflow > > > handling by adding support for accessing per-CPU variables > > > directly in assembly. The patch is included in this series to > > > make IRQ stack switching cleaner with SCS, and I've simply > > > rebased it. Patch 2 uses this functionality to clean up the stack > > > switching by moving duplicate code into a single function. On > > > RISC-V, the compiler uses the gp register for storing the current > > > shadow call stack pointer, which is incompatible with global > > > pointer relaxation. Patch 3 moves global pointer loading into a > > > macro that can be easily disabled with SCS. Patch 4 implements > > > SCS register loading and switching, and allows the feature to be > > > enabled, and patch 5 adds separate per-CPU IRQ shadow call stacks > > > when CONFIG_IRQ_STACKS is enabled. > > > > > > Note that this series requires Clang 17. Earlier Clang versions > > > support SCS on RISC-V, but use the x18 register instead of gp, > > > which isn't ideal. gcc has SCS support for arm64, but I'm not > > > aware of plans to support RISC-V. Once the Zicfiss extension is > > > ratified, it's probably preferable to use hardware-backed shadow > > > stacks instead of SCS on hardware that supports the extension, > > > and we may want to consider implementing CONFIG_DYNAMIC_SCS to > > > patch between the implementation at runtime (similarly to the > > > arm64 implementation, which switches to SCS when hardware PAC > > > support isn't available). > > > > I took this series for a spin on top of 6.5-rc6 with both LLVM 18 (built > > within the past couple of days) and LLVM 17.0.0-rc2 but it seems that > > the CFI_BACKWARDS LKDTM test does not pass with > > CONFIG_SHADOW_CALL_STACK=y. > > > > [ 73.324652] lkdtm: Performing direct entry CFI_BACKWARD > > [ 73.324900] lkdtm: Attempting unchecked stack return address redirection ... > > [ 73.325178] lkdtm: Eek: return address mismatch! 0000000000000002 != ffffffff80614982 > > [ 73.325478] lkdtm: FAIL: stack return address manipulation failed! > > > > Does the test need to be adjusted or is there some other issue? > > Does it pass without the series? I tried to write it to be > arch-agnostic, but I never tested it on RISC-V. It's very possible that > test needs adjusting for the architecture. Besides the label horrors, > the use of __builtin_frame_address may not work there either...
Looks like __builtin_frame_address behaves differently on RISC-V. After staring at the disassembly a bit, using __builtin_frame_address(0) - 1 instead of + 1 seems to yield correct results. WIth that change, here's the test without SCS:
# echo CFI_BACKWARD > /sys/kernel/debug/provoke-crash/DIRECT [ 16.272028] lkdtm: Performing direct entry CFI_BACKWARD [ 16.272368] lkdtm: Attempting unchecked stack return address redirection ... [ 16.272671] lkdtm: ok: redirected stack return address. [ 16.272885] lkdtm: Attempting checked stack return address redirection ... [ 16.273384] lkdtm: FAIL: stack return address was redirected! [ 16.273755] lkdtm: This is probably expected, since this kernel (6.5.0-rc5-00005-g5a1201f89265-dirty riscv64) was built *without* CONFIG_ARM64_PTR_AUTH_KERNEL=y nor CONFIG_SHADOW_CALL_STACK=y
And with SCS:
# echo CFI_BACKWARD > /sys/kernel/debug/provoke-crash/DIRECT [ 17.429551] lkdtm: Performing direct entry CFI_BACKWARD [ 17.430184] lkdtm: Attempting unchecked stack return address redirection ... [ 17.431402] lkdtm: ok: redirected stack return address. [ 17.432031] lkdtm: Attempting checked stack return address redirection ... [ 17.432861] lkdtm: ok: control flow unchanged.
Sami
| |