[PATCH v2] lib/crypto: riscv/chacha: Avoid s0/fp register

[PATCH v2] lib/crypto: riscv/chacha: Avoid s0/fp register

[Vivian Wang]

In chacha_zvkb, avoid using the s0 register, which is the frame pointer,
by reallocating KEY0 to t5. This makes stack traces available if e.g. a
crash happens in chacha_zvkb.

No frame pointer maintenence is otherwise required since this is a leaf
function.

Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
---
Changes in v2:
- Remove frame pointer maintenance, and simply avoid touching s0. Since
  this is a leaf function, this also allows unwinding to work.
- Link to v1: https://lore.kernel.org/r/20251130-riscv-chacha_zvkb-fp-v1-1-68ef7a6d477a@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 | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/crypto/riscv/chacha-riscv64-zvkb.S b/lib/crypto/riscv/chacha-riscv64-zvkb.S
index b777d0b4e379..3d183ec818f5 100644
--- a/lib/crypto/riscv/chacha-riscv64-zvkb.S
+++ b/lib/crypto/riscv/chacha-riscv64-zvkb.S
@@ -60,7 +60,8 @@
 #define VL		t2
 #define STRIDE		t3
 #define ROUND_CTR	t4
-#define KEY0		s0
+#define KEY0		t5
+// Avoid s0/fp to allow for unwinding
 #define KEY1		s1
 #define KEY2		s2
 #define KEY3		s3
@@ -143,7 +144,6 @@
 // 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)
 	sd		s1, 8(sp)
 	sd		s2, 16(sp)
 	sd		s3, 24(sp)
@@ -280,7 +280,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)

---
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 v2] lib/crypto: riscv/chacha: Avoid s0/fp register

[Eric Biggers]

On Tue, Dec 02, 2025 at 01:25:07PM +0800, Vivian Wang wrote:
> In chacha_zvkb, avoid using the s0 register, which is the frame pointer,
> by reallocating KEY0 to t5. This makes stack traces available if e.g. a
> crash happens in chacha_zvkb.
> 
> No frame pointer maintenence is otherwise required since this is a leaf
> function.

maintenence => maintenance

>  SYM_FUNC_START(chacha_zvkb)
>  	addi		sp, sp, -96
> -	sd		s0, 0(sp)

I know it's annoying, but would you mind also changing the 96 to 88, and
decreasing all the offsets by 8, so that we don't leave a hole in the
stack where s0 used to be?  Likewise at the end of the function.

- Eric

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

Re: [PATCH v2] lib/crypto: riscv/chacha: Avoid s0/fp register

[Vivian Wang]

On 12/2/25 13:31, Eric Biggers wrote:
> On Tue, Dec 02, 2025 at 01:25:07PM +0800, Vivian Wang wrote:
>> In chacha_zvkb, avoid using the s0 register, which is the frame pointer,
>> by reallocating KEY0 to t5. This makes stack traces available if e.g. a
>> crash happens in chacha_zvkb.
>>
>> No frame pointer maintenence is otherwise required since this is a leaf
>> function.
> maintenence => maintenance
>
Ouch... I swear I specifically checked this before sending, but
apparently didn't see this. Thanks for the catch.

>>  SYM_FUNC_START(chacha_zvkb)
>>  	addi		sp, sp, -96
>> -	sd		s0, 0(sp)
> I know it's annoying, but would you mind also changing the 96 to 88, and
> decreasing all the offsets by 8, so that we don't leave a hole in the
> stack where s0 used to be?  Likewise at the end of the function.

No can do. Stack alignment on RISC-V is 16 bytes, and 80 won't fit.


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

Re: [PATCH v2] lib/crypto: riscv/chacha: Avoid s0/fp register

[Eric Biggers]

On Tue, Dec 02, 2025 at 02:24:46PM +0800, Vivian Wang wrote:
> On 12/2/25 13:31, Eric Biggers wrote:
> > On Tue, Dec 02, 2025 at 01:25:07PM +0800, Vivian Wang wrote:
> >> In chacha_zvkb, avoid using the s0 register, which is the frame pointer,
> >> by reallocating KEY0 to t5. This makes stack traces available if e.g. a
> >> crash happens in chacha_zvkb.
> >>
> >> No frame pointer maintenence is otherwise required since this is a leaf
> >> function.
> > maintenence => maintenance
> >
> Ouch... I swear I specifically checked this before sending, but
> apparently didn't see this. Thanks for the catch.
> 
> >>  SYM_FUNC_START(chacha_zvkb)
> >>  	addi		sp, sp, -96
> >> -	sd		s0, 0(sp)
> > I know it's annoying, but would you mind also changing the 96 to 88, and
> > decreasing all the offsets by 8, so that we don't leave a hole in the
> > stack where s0 used to be?  Likewise at the end of the function.
> 
> No can do. Stack alignment on RISC-V is 16 bytes, and 80 won't fit.
> 

Hmm, interesting.  It shouldn't actually matter, since this doesn't call
any other function, but we might as well leave it at 96 then.  I don't
think this was considered when any of the RISC-V crypto code was
written, but fortunately this is the only one that uses the stack.

Anyway, I guess I'll apply this as-is then.

- Eric

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

Re: [PATCH v2] lib/crypto: riscv/chacha: Avoid s0/fp register

[Eric Biggers]

On Tue, Dec 02, 2025 at 01:25:07PM +0800, Vivian Wang wrote:
> In chacha_zvkb, avoid using the s0 register, which is the frame pointer,
> by reallocating KEY0 to t5. This makes stack traces available if e.g. a
> crash happens in chacha_zvkb.
> 
> No frame pointer maintenence is otherwise required since this is a leaf
> function.
> 
> Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
> ---
> Changes in v2:
> - Remove frame pointer maintenance, and simply avoid touching s0. Since
>   this is a leaf function, this also allows unwinding to work.
> - Link to v1: https://lore.kernel.org/r/20251130-riscv-chacha_zvkb-fp-v1-1-68ef7a6d477a@iscas.ac.cn
> ---
> 

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-fixes

- Eric

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

Re: [PATCH v2] lib/crypto: riscv/chacha: Avoid s0/fp register

[Jerry Shih]

On Tue, Dec 2, 2025 at 2:32 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Dec 02, 2025 at 02:24:46PM +0800, Vivian Wang wrote:
> > On 12/2/25 13:31, Eric Biggers wrote:
> > > On Tue, Dec 02, 2025 at 01:25:07PM +0800, Vivian Wang wrote:
> > >> In chacha_zvkb, avoid using the s0 register, which is the frame pointer,
> > >> by reallocating KEY0 to t5. This makes stack traces available if e.g. a
> > >> crash happens in chacha_zvkb.
> > >>
> > >> No frame pointer maintenence is otherwise required since this is a leaf
> > >> function.
> > > maintenence => maintenance
> > >
> > Ouch... I swear I specifically checked this before sending, but
> > apparently didn't see this. Thanks for the catch.
> >
> > >>  SYM_FUNC_START(chacha_zvkb)
> > >>    addi            sp, sp, -96
> > >> -  sd              s0, 0(sp)
> > > I know it's annoying, but would you mind also changing the 96 to 88, and
> > > decreasing all the offsets by 8, so that we don't leave a hole in the
> > > stack where s0 used to be?  Likewise at the end of the function.
> >
> > No can do. Stack alignment on RISC-V is 16 bytes, and 80 won't fit.
> >
>
> Hmm, interesting.  It shouldn't actually matter, since this doesn't call
> any other function, but we might as well leave it at 96 then.  I don't
> think this was considered when any of the RISC-V crypto code was
> written, but fortunately this is the only one that uses the stack.
>
> Anyway, I guess I'll apply this as-is then.
>
> - Eric

The 16-byte stack alignment is in RISC-V calling convention:
https://riscv.org/wp-content/uploads/2024/12/riscv-calling.pdf
It says:
In the standard RISC-V calling convention, the stack grows downward
and the stack pointer is always kept 16-byte aligned.

-Jerry

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

Re: [PATCH v2] lib/crypto: riscv/chacha: Avoid s0/fp register

[Samuel Holland]

On 2025-12-09 12:58 PM, Jerry Shih wrote:
> On Tue, Dec 2, 2025 at 2:32 PM Eric Biggers <ebiggers@kernel.org> wrote:
>>
>> On Tue, Dec 02, 2025 at 02:24:46PM +0800, Vivian Wang wrote:
>>> On 12/2/25 13:31, Eric Biggers wrote:
>>>> On Tue, Dec 02, 2025 at 01:25:07PM +0800, Vivian Wang wrote:
>>>>> In chacha_zvkb, avoid using the s0 register, which is the frame pointer,
>>>>> by reallocating KEY0 to t5. This makes stack traces available if e.g. a
>>>>> crash happens in chacha_zvkb.
>>>>>
>>>>> No frame pointer maintenence is otherwise required since this is a leaf
>>>>> function.
>>>> maintenence => maintenance
>>>>
>>> Ouch... I swear I specifically checked this before sending, but
>>> apparently didn't see this. Thanks for the catch.
>>>
>>>>>  SYM_FUNC_START(chacha_zvkb)
>>>>>    addi            sp, sp, -96
>>>>> -  sd              s0, 0(sp)
>>>> I know it's annoying, but would you mind also changing the 96 to 88, and
>>>> decreasing all the offsets by 8, so that we don't leave a hole in the
>>>> stack where s0 used to be?  Likewise at the end of the function.
>>>
>>> No can do. Stack alignment on RISC-V is 16 bytes, and 80 won't fit.
>>>
>>
>> Hmm, interesting.  It shouldn't actually matter, since this doesn't call
>> any other function, but we might as well leave it at 96 then.  I don't
>> think this was considered when any of the RISC-V crypto code was
>> written, but fortunately this is the only one that uses the stack.
>>
>> Anyway, I guess I'll apply this as-is then.
>>
>> - Eric
> 
> The 16-byte stack alignment is in RISC-V calling convention:
> https://riscv.org/wp-content/uploads/2024/12/riscv-calling.pdf
> It says:
> In the standard RISC-V calling convention, the stack grows downward
> and the stack pointer is always kept 16-byte aligned.

Indeed, and this does matter if the code runs with IRQs enabled, as the RISC-V
entry assembly assumes the kernel stack is already properly aligned.

Regards,
Samuel


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

Re: [PATCH v2] lib/crypto: riscv/chacha: Avoid s0/fp register

[patchwork-bot+linux-riscv]

Hello:

This patch was applied to riscv/linux.git (fixes)
by Eric Biggers <ebiggers@kernel.org>:

On Tue, 02 Dec 2025 13:25:07 +0800 you wrote:
> In chacha_zvkb, avoid using the s0 register, which is the frame pointer,
> by reallocating KEY0 to t5. This makes stack traces available if e.g. a
> crash happens in chacha_zvkb.
> 
> No frame pointer maintenence is otherwise required since this is a leaf
> function.
> 
> [...]

Here is the summary with links:
  - [v2] lib/crypto: riscv/chacha: Avoid s0/fp register
    https://git.kernel.org/riscv/c/43169328c7b4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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