[PATCH] lib/crypto: riscv/chacha: Maintain a frame pointer

[PATCH] lib/crypto: riscv/chacha: Maintain a frame pointer

[Vivian Wang]

crypto_zvkb doesn't maintain a frame pointer and also uses s0, which
means that if it crashes we don't get a stack trace. Modify prologue and
epilogue to maintain a frame pointer as -fno-omit-frame-pointer would.
Also reallocate registers to match.

Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
---
Found while diagnosing a crypto_zvkb "load address misaligned" crash [1]

[1]: https://lore.kernel.org/r/b3cfcdac-0337-4db0-a611-258f2868855f@iscas.ac.cn/
---
 lib/crypto/riscv/chacha-riscv64-zvkb.S | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/lib/crypto/riscv/chacha-riscv64-zvkb.S b/lib/crypto/riscv/chacha-riscv64-zvkb.S
index b777d0b4e379..dc4e45759d14 100644
--- a/lib/crypto/riscv/chacha-riscv64-zvkb.S
+++ b/lib/crypto/riscv/chacha-riscv64-zvkb.S
@@ -60,7 +60,7 @@
 #define VL		t2
 #define STRIDE		t3
 #define ROUND_CTR	t4
-#define KEY0		s0
+#define KEY0		t5
 #define KEY1		s1
 #define KEY2		s2
 #define KEY3		s3
@@ -142,8 +142,7 @@
 // the original Salsa20 paper which uses a 64-bit counter in state->x[12..13].
 // The updated 32-bit counter is written back to state->x[12] before returning.
 SYM_FUNC_START(chacha_zvkb)
-	addi		sp, sp, -96
-	sd		s0, 0(sp)
+	addi		sp, sp, -112
 	sd		s1, 8(sp)
 	sd		s2, 16(sp)
 	sd		s3, 24(sp)
@@ -155,6 +154,10 @@ SYM_FUNC_START(chacha_zvkb)
 	sd		s9, 72(sp)
 	sd		s10, 80(sp)
 	sd		s11, 88(sp)
+	sd		fp, 96(sp)
+	sd		ra, 104(sp)
+
+	addi		fp, sp, 112
 
 	li		STRIDE, 64
 
@@ -280,7 +283,6 @@ SYM_FUNC_START(chacha_zvkb)
 	bnez		NBLOCKS, .Lblock_loop
 
 	sw		COUNTER, 48(STATEP)
-	ld		s0, 0(sp)
 	ld		s1, 8(sp)
 	ld		s2, 16(sp)
 	ld		s3, 24(sp)
@@ -292,6 +294,7 @@ SYM_FUNC_START(chacha_zvkb)
 	ld		s9, 72(sp)
 	ld		s10, 80(sp)
 	ld		s11, 88(sp)
-	addi		sp, sp, 96
+	ld		fp, 96(sp)
+	addi		sp, sp, 112
 	ret
 SYM_FUNC_END(chacha_zvkb)

---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20251130-riscv-chacha_zvkb-fp-5644ed88b1a2

Best regards,
-- 
Vivian "dramforever" Wang

[PATCH] lib/crypto: riscv/chacha: Maintain a frame pointer

[Vivian Wang]

crypto_zvkb doesn't maintain a frame pointer and also uses s0, which
means that if it crashes we don't get a stack trace. Modify prologue and
epilogue to maintain a frame pointer as -fno-omit-frame-pointer would.
Also reallocate registers to match.

Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
---
Found while diagnosing a crypto_zvkb "load address misaligned" crash [1]

[1]: https://lore.kernel.org/r/b3cfcdac-0337-4db0-a611-258f2868855f@iscas.ac.cn/
---
 lib/crypto/riscv/chacha-riscv64-zvkb.S | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/lib/crypto/riscv/chacha-riscv64-zvkb.S b/lib/crypto/riscv/chacha-riscv64-zvkb.S
index b777d0b4e379..dc4e45759d14 100644
--- a/lib/crypto/riscv/chacha-riscv64-zvkb.S
+++ b/lib/crypto/riscv/chacha-riscv64-zvkb.S
@@ -60,7 +60,7 @@
 #define VL		t2
 #define STRIDE		t3
 #define ROUND_CTR	t4
-#define KEY0		s0
+#define KEY0		t5
 #define KEY1		s1
 #define KEY2		s2
 #define KEY3		s3
@@ -142,8 +142,7 @@
 // the original Salsa20 paper which uses a 64-bit counter in state->x[12..13].
 // The updated 32-bit counter is written back to state->x[12] before returning.
 SYM_FUNC_START(chacha_zvkb)
-	addi		sp, sp, -96
-	sd		s0, 0(sp)
+	addi		sp, sp, -112
 	sd		s1, 8(sp)
 	sd		s2, 16(sp)
 	sd		s3, 24(sp)
@@ -155,6 +154,10 @@ SYM_FUNC_START(chacha_zvkb)
 	sd		s9, 72(sp)
 	sd		s10, 80(sp)
 	sd		s11, 88(sp)
+	sd		fp, 96(sp)
+	sd		ra, 104(sp)
+
+	addi		fp, sp, 112
 
 	li		STRIDE, 64
 
@@ -280,7 +283,6 @@ SYM_FUNC_START(chacha_zvkb)
 	bnez		NBLOCKS, .Lblock_loop
 
 	sw		COUNTER, 48(STATEP)
-	ld		s0, 0(sp)
 	ld		s1, 8(sp)
 	ld		s2, 16(sp)
 	ld		s3, 24(sp)
@@ -292,6 +294,7 @@ SYM_FUNC_START(chacha_zvkb)
 	ld		s9, 72(sp)
 	ld		s10, 80(sp)
 	ld		s11, 88(sp)
-	addi		sp, sp, 96
+	ld		fp, 96(sp)
+	addi		sp, sp, 112
 	ret
 SYM_FUNC_END(chacha_zvkb)

---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20251130-riscv-chacha_zvkb-fp-5644ed88b1a2

Best regards,
-- 
Vivian "dramforever" Wang


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] lib/crypto: riscv/chacha: Maintain a frame pointer

[Eric Biggers]

On Sun, Nov 30, 2025 at 06:23:50PM +0800, Vivian Wang wrote:
> crypto_zvkb doesn't maintain a frame pointer and also uses s0, which
> means that if it crashes we don't get a stack trace. Modify prologue and
> epilogue to maintain a frame pointer as -fno-omit-frame-pointer would.
> Also reallocate registers to match.
> 
> Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
> ---
> Found while diagnosing a crypto_zvkb "load address misaligned" crash [1]
> 
> [1]: https://lore.kernel.org/r/b3cfcdac-0337-4db0-a611-258f2868855f@iscas.ac.cn/
> ---
>  lib/crypto/riscv/chacha-riscv64-zvkb.S | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)

Do I understand correctly that the problem isn't so much that
crypto_zvkb() doesn't set up its own frame pointer, but rather it reuses
the frame pointer register (s0 i.e. fp) for other data?

That's what we've seen on other architectures, like x86_64 with %rbp.
Assembly functions need to set their own frame pointer only if they call
other functions.  Otherwise, they can just run with their parent's frame
pointer.  However, in either case, they must not store other data in the
frame pointer register.

Is that the case on RISC-V too?  If so, the appropriate fix is to just
stop using s0 for other data; we don't actually need to set up a frame
pointer.  (Note that none of the RISC-V crypto assembly code sets up
frame pointers.  So if that was an issue, it would affect every file.)

- Eric

Re: [PATCH] lib/crypto: riscv/chacha: Maintain a frame pointer

[Eric Biggers]

On Sun, Nov 30, 2025 at 06:23:50PM +0800, Vivian Wang wrote:
> crypto_zvkb doesn't maintain a frame pointer and also uses s0, which
> means that if it crashes we don't get a stack trace. Modify prologue and
> epilogue to maintain a frame pointer as -fno-omit-frame-pointer would.
> Also reallocate registers to match.
> 
> Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
> ---
> Found while diagnosing a crypto_zvkb "load address misaligned" crash [1]
> 
> [1]: https://lore.kernel.org/r/b3cfcdac-0337-4db0-a611-258f2868855f@iscas.ac.cn/
> ---
>  lib/crypto/riscv/chacha-riscv64-zvkb.S | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)

Do I understand correctly that the problem isn't so much that
crypto_zvkb() doesn't set up its own frame pointer, but rather it reuses
the frame pointer register (s0 i.e. fp) for other data?

That's what we've seen on other architectures, like x86_64 with %rbp.
Assembly functions need to set their own frame pointer only if they call
other functions.  Otherwise, they can just run with their parent's frame
pointer.  However, in either case, they must not store other data in the
frame pointer register.

Is that the case on RISC-V too?  If so, the appropriate fix is to just
stop using s0 for other data; we don't actually need to set up a frame
pointer.  (Note that none of the RISC-V crypto assembly code sets up
frame pointers.  So if that was an issue, it would affect every file.)

- Eric

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] lib/crypto: riscv/chacha: Maintain a frame pointer

[Vivian Wang]

On 12/1/25 02:29, Eric Biggers wrote:
> On Sun, Nov 30, 2025 at 06:23:50PM +0800, Vivian Wang wrote:
>> crypto_zvkb doesn't maintain a frame pointer and also uses s0, which
>> means that if it crashes we don't get a stack trace. Modify prologue and
>> epilogue to maintain a frame pointer as -fno-omit-frame-pointer would.
>> Also reallocate registers to match.
>>
>> Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
>> ---
>> Found while diagnosing a crypto_zvkb "load address misaligned" crash [1]
>>
>> [1]: https://lore.kernel.org/r/b3cfcdac-0337-4db0-a611-258f2868855f@iscas.ac.cn/
>> ---
>>  lib/crypto/riscv/chacha-riscv64-zvkb.S | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
> Do I understand correctly that the problem isn't so much that
> crypto_zvkb() doesn't set up its own frame pointer, but rather it reuses
> the frame pointer register (s0 i.e. fp) for other data?
>
> That's what we've seen on other architectures, like x86_64 with %rbp.
> Assembly functions need to set their own frame pointer only if they call
> other functions.  Otherwise, they can just run with their parent's frame
> pointer.  However, in either case, they must not store other data in the
> frame pointer register.
>
> Is that the case on RISC-V too?  If so, the appropriate fix is to just
> stop using s0 for other data; we don't actually need to set up a frame
> pointer.  (Note that none of the RISC-V crypto assembly code sets up
> frame pointers.  So if that was an issue, it would affect every file.)

Thanks for the hint, I can confirm that indeed simply avoiding s0 also
fixes the stack trace problem.

I'll drop the rest of the patch for the next version.

Vivian "dramforever" Wang

Re: [PATCH] lib/crypto: riscv/chacha: Maintain a frame pointer

[Vivian Wang]

On 12/1/25 02:29, Eric Biggers wrote:
> On Sun, Nov 30, 2025 at 06:23:50PM +0800, Vivian Wang wrote:
>> crypto_zvkb doesn't maintain a frame pointer and also uses s0, which
>> means that if it crashes we don't get a stack trace. Modify prologue and
>> epilogue to maintain a frame pointer as -fno-omit-frame-pointer would.
>> Also reallocate registers to match.
>>
>> Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
>> ---
>> Found while diagnosing a crypto_zvkb "load address misaligned" crash [1]
>>
>> [1]: https://lore.kernel.org/r/b3cfcdac-0337-4db0-a611-258f2868855f@iscas.ac.cn/
>> ---
>>  lib/crypto/riscv/chacha-riscv64-zvkb.S | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
> Do I understand correctly that the problem isn't so much that
> crypto_zvkb() doesn't set up its own frame pointer, but rather it reuses
> the frame pointer register (s0 i.e. fp) for other data?
>
> That's what we've seen on other architectures, like x86_64 with %rbp.
> Assembly functions need to set their own frame pointer only if they call
> other functions.  Otherwise, they can just run with their parent's frame
> pointer.  However, in either case, they must not store other data in the
> frame pointer register.
>
> Is that the case on RISC-V too?  If so, the appropriate fix is to just
> stop using s0 for other data; we don't actually need to set up a frame
> pointer.  (Note that none of the RISC-V crypto assembly code sets up
> frame pointers.  So if that was an issue, it would affect every file.)

Thanks for the hint, I can confirm that indeed simply avoiding s0 also
fixes the stack trace problem.

I'll drop the rest of the patch for the next version.

Vivian "dramforever" Wang


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv