[PATCH 0/3] lib/crypto: x86/sha: Follow the normal %rbp convention

[PATCH 0/3] lib/crypto: x86/sha: Follow the normal %rbp convention

[Eric Biggers]

This series updates the x86 SHA-1, SHA-256, and SHA-512 assembly code to
set up %rbp before pushing GPRs rather than after, so that the frame
pointer unwinder will generate call traces reliably.

This doesn't matter much, given that the ORC unwinder is the default,
and the fallback to scanning the stack made call traces sort of "work"
anyway.  But it annoyed me that these assembly functions use the "wrong"
%rbp convention, differing from the C code.

This series is targeting libcrypto-next.

Eric Biggers (3):
  lib/crypto: x86/sha1: Follow the normal %rbp convention
  lib/crypto: x86/sha256: Follow the normal %rbp convention
  lib/crypto: x86/sha512: Follow the normal %rbp convention

 lib/crypto/x86/sha1-avx2-asm.S      | 10 +++++-----
 lib/crypto/x86/sha1-ssse3-and-avx.S |  9 +++++----
 lib/crypto/x86/sha256-avx-asm.S     | 10 +++++-----
 lib/crypto/x86/sha256-avx2-asm.S    | 10 ++++------
 lib/crypto/x86/sha256-ni-asm.S      |  9 +++++----
 lib/crypto/x86/sha256-ssse3-asm.S   |  8 ++++----
 lib/crypto/x86/sha512-avx-asm.S     | 11 ++++++-----
 lib/crypto/x86/sha512-avx2-asm.S    | 11 ++++++-----
 lib/crypto/x86/sha512-ssse3-asm.S   | 11 ++++++-----
 9 files changed, 46 insertions(+), 43 deletions(-)


base-commit: 6a50ce0af5c91fd665d65d357af8ae55db1df7d0
-- 
2.56.0

[PATCH 1/3] lib/crypto: x86/sha1: Follow the normal %rbp convention

[Eric Biggers]

Some of the SHA-1 assembly functions push callee-save GPRs to the stack
*before* they set up the frame pointer (%rbp).  (They are leaf
functions, but they need to set up a frame pointer anyway because they
allocate a 16- or 32-byte-aligned buffer on the stack.)

As a result, during these functions, 8(%rbp) doesn't contain a return
address like it is supposed to.

This differs from the convention used by C code, and it interferes with
the frame pointer unwinder (CONFIG_UNWINDER_FRAME_POINTER=y).
Specifically, if a call trace is dumped in these functions, then all
entries are populated purely from the fallback scan of the entire stack:

    RIP: 0010:sha1_transform_avx2+0x1921/0x1ea0
    [...]
    Call Trace:
     <TASK>
     ? sha1_transform_avx+0x2a9/0xc90
     ? sha1_blocks_avx2+0x78/0x140
     ? kernel_fpu_end+0x2d/0x40
     ? sha1_blocks_avx2+0x52/0x140
     ? sha1_update+0x77/0x220
     [...]
     ? do_syscall_64+0x6f/0x3e0

With this fix to follow the normal %rbp convention, the call trace is
generated reliably:

    RIP: 0010:sha1_transform_avx2+0x66e/0x1ea0
    [...]
    Call Trace:
     <TASK>
     sha1_blocks_avx2+0x78/0x140
     sha1_update+0x77/0x220
     [...]
     do_syscall_64+0x6f/0x3e0

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 lib/crypto/x86/sha1-avx2-asm.S      | 10 +++++-----
 lib/crypto/x86/sha1-ssse3-and-avx.S |  9 +++++----
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/lib/crypto/x86/sha1-avx2-asm.S b/lib/crypto/x86/sha1-avx2-asm.S
index 91b2dc6db179..ca64d7104a47 100644
--- a/lib/crypto/x86/sha1-avx2-asm.S
+++ b/lib/crypto/x86/sha1-avx2-asm.S
@@ -622,6 +622,9 @@
 .macro SHA1_VECTOR_ASM  name
 	SYM_FUNC_START(\name)
 
+	push	%rbp
+	mov	%rsp, %rbp
+
 	push	%rbx
 	push	%r12
 	push	%r13
@@ -631,8 +634,6 @@
 	RESERVE_STACK  = (W_SIZE*4 + 8+24)
 
 	/* Align stack */
-	push	%rbp
-	mov	%rsp, %rbp
 	and	$~(0x20-1), %rsp
 	sub	$RESERVE_STACK, %rsp
 
@@ -651,14 +652,13 @@
 
 	avx2_zeroupper
 
-	mov	%rbp, %rsp
-	pop	%rbp
-
+	lea	-40(%rbp), %rsp
 	pop	%r15
 	pop	%r14
 	pop	%r13
 	pop	%r12
 	pop	%rbx
+	pop	%rbp
 
 	RET
 
diff --git a/lib/crypto/x86/sha1-ssse3-and-avx.S b/lib/crypto/x86/sha1-ssse3-and-avx.S
index 78b48cb09c5b..fe0faaf494c7 100644
--- a/lib/crypto/x86/sha1-ssse3-and-avx.S
+++ b/lib/crypto/x86/sha1-ssse3-and-avx.S
@@ -69,11 +69,12 @@
 .macro SHA1_VECTOR_ASM  name
 	SYM_FUNC_START(\name)
 
-	push	%rbx
-	push	%r12
 	push	%rbp
 	mov	%rsp, %rbp
 
+	push	%rbx
+	push	%r12
+
 	sub	$64, %rsp		# allocate workspace
 	and	$~15, %rsp		# align stack
 
@@ -95,10 +96,10 @@
 	xor	%eax, %eax
 	rep stosq
 
-	mov	%rbp, %rsp		# deallocate workspace
-	pop	%rbp
+	lea	-16(%rbp), %rsp		# deallocate workspace
 	pop	%r12
 	pop	%rbx
+	pop	%rbp
 	RET
 
 	SYM_FUNC_END(\name)
-- 
2.56.0

[PATCH 2/3] lib/crypto: x86/sha256: Follow the normal %rbp convention

[Eric Biggers]

Some of the SHA-256 assembly functions push callee-save GPRs to the
stack *before* they set up the frame pointer (%rbp).  (They are leaf
functions, but they need to set up a frame pointer anyway because they
allocate a 16- or 32-byte-aligned buffer on the stack.)

As a result, during these functions, 8(%rbp) doesn't contain a return
address like it is supposed to.

This differs from the convention used by C code, and it interferes with
the frame pointer unwinder (CONFIG_UNWINDER_FRAME_POINTER=y).
Specifically, if a call trace is dumped in these functions, then all
entries are populated purely from the fallback scan of the entire stack:

    RIP: 0010:sha256_transform_rorx+0xd78/0x1120
    [...]
    Call Trace:
     <TASK>
     ? sha256_blocks_avx2+0x37/0x70
     ? __sha256_update+0x77/0x220
     ? sha256+0x9d/0x110
     [...]
     ? do_syscall_64+0x6f/0x3e0

With this fix to follow the normal %rbp convention, the call trace is
generated reliably:

    RIP: 0010:sha256_transform_rorx+0xdae/0x1120
    [...]
    Call Trace:
     <TASK>
     sha256_blocks_avx2+0x37/0x70
     __sha256_update+0x77/0x220
     sha256+0x9d/0x110
     [...]
     do_syscall_64+0x6f/0x3e0

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 lib/crypto/x86/sha256-avx-asm.S   | 10 +++++-----
 lib/crypto/x86/sha256-avx2-asm.S  | 10 ++++------
 lib/crypto/x86/sha256-ni-asm.S    |  9 +++++----
 lib/crypto/x86/sha256-ssse3-asm.S |  8 ++++----
 4 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/lib/crypto/x86/sha256-avx-asm.S b/lib/crypto/x86/sha256-avx-asm.S
index c1aceb3ba3a3..efb8dff5e8a9 100644
--- a/lib/crypto/x86/sha256-avx-asm.S
+++ b/lib/crypto/x86/sha256-avx-asm.S
@@ -345,13 +345,13 @@ a = TMP_
 ########################################################################
 .text
 SYM_FUNC_START(sha256_transform_avx)
+	pushq   %rbp
+	movq    %rsp, %rbp
 	pushq   %rbx
 	pushq   %r12
 	pushq   %r13
 	pushq   %r14
 	pushq   %r15
-	pushq	%rbp
-	movq	%rsp, %rbp
 
 	subq    $STACK_SIZE, %rsp	# allocate stack space
 	and	$~15, %rsp		# align stack pointer
@@ -445,13 +445,13 @@ SYM_FUNC_START(sha256_transform_avx)
 	cmp     _INP_END(%rsp), INP
 	jne     .Lloop0
 
-	mov	%rbp, %rsp
-	popq	%rbp
+	lea     -40(%rbp), %rsp
 	popq    %r15
 	popq    %r14
 	popq    %r13
-	popq	%r12
+	popq    %r12
 	popq    %rbx
+	popq    %rbp
 	RET
 SYM_FUNC_END(sha256_transform_avx)
 
diff --git a/lib/crypto/x86/sha256-avx2-asm.S b/lib/crypto/x86/sha256-avx2-asm.S
index eb8836fb9695..a719c2173b61 100644
--- a/lib/crypto/x86/sha256-avx2-asm.S
+++ b/lib/crypto/x86/sha256-avx2-asm.S
@@ -522,15 +522,14 @@ STACK_SIZE	= _CTX      + _CTX_SIZE
 ########################################################################
 .text
 SYM_FUNC_START(sha256_transform_rorx)
+	pushq	%rbp
+	mov	%rsp, %rbp
 	pushq	%rbx
 	pushq	%r12
 	pushq	%r13
 	pushq	%r14
 	pushq	%r15
 
-	push	%rbp
-	mov	%rsp, %rbp
-
 	subq	$STACK_SIZE, %rsp
 	and	$-32, %rsp	# align rsp to 32 byte boundary
 
@@ -704,14 +703,13 @@ SYM_FUNC_START(sha256_transform_rorx)
 
 .Ldone_hash:
 
-	mov	%rbp, %rsp
-	pop	%rbp
-
+	lea	-40(%rbp), %rsp
 	popq	%r15
 	popq	%r14
 	popq	%r13
 	popq	%r12
 	popq	%rbx
+	popq	%rbp
 	vzeroupper
 	RET
 SYM_FUNC_END(sha256_transform_rorx)
diff --git a/lib/crypto/x86/sha256-ni-asm.S b/lib/crypto/x86/sha256-ni-asm.S
index de5f707e7ef7..e47263273f43 100644
--- a/lib/crypto/x86/sha256-ni-asm.S
+++ b/lib/crypto/x86/sha256-ni-asm.S
@@ -276,10 +276,11 @@ SYM_FUNC_END(sha256_ni_transform)
 // to taking better advantage of the CPU's SHA-256 and SIMD throughput.
 //
 SYM_FUNC_START(sha256_ni_finup2x)
-	// Allocate 128 bytes of stack space, 16-byte aligned.
-	push		%rbx
 	push		%rbp
 	mov		%rsp, %rbp
+	push		%rbx
+
+	// Allocate 128 bytes of stack space, 16-byte aligned.
 	sub		$128, %rsp
 	and		$~15, %rsp
 
@@ -527,9 +528,9 @@ SYM_FUNC_START(sha256_ni_finup2x)
 	movdqu		STATE1_A, 0*16(OUT1)
 	movdqu		STATE1_B, 0*16(OUT2)
 
-	mov		%rbp, %rsp
-	pop		%rbp
+	lea		-8(%rbp), %rsp
 	pop		%rbx
+	pop		%rbp
 	RET
 SYM_FUNC_END(sha256_ni_finup2x)
 
diff --git a/lib/crypto/x86/sha256-ssse3-asm.S b/lib/crypto/x86/sha256-ssse3-asm.S
index 383b8eec7ebe..cc91b3e15a83 100644
--- a/lib/crypto/x86/sha256-ssse3-asm.S
+++ b/lib/crypto/x86/sha256-ssse3-asm.S
@@ -352,13 +352,13 @@ a = TMP_
 ########################################################################
 .text
 SYM_FUNC_START(sha256_transform_ssse3)
+	pushq   %rbp
+	mov     %rsp, %rbp
 	pushq   %rbx
 	pushq   %r12
 	pushq   %r13
 	pushq   %r14
 	pushq   %r15
-	pushq   %rbp
-	mov	%rsp, %rbp
 
 	subq    $STACK_SIZE, %rsp
 	and	$~15, %rsp
@@ -456,13 +456,13 @@ SYM_FUNC_START(sha256_transform_ssse3)
 	cmp     _INP_END(%rsp), INP
 	jne     .Lloop0
 
-	mov	%rbp, %rsp
-	popq	%rbp
+	lea     -40(%rbp), %rsp
 	popq    %r15
 	popq    %r14
 	popq    %r13
 	popq    %r12
 	popq    %rbx
+	popq    %rbp
 
 	RET
 SYM_FUNC_END(sha256_transform_ssse3)
-- 
2.56.0

[PATCH 3/3] lib/crypto: x86/sha512: Follow the normal %rbp convention

[Eric Biggers]

The SHA-512 assembly functions push callee-save GPRs to the stack
*before* they set up the frame pointer (%rbp).  (They are leaf
functions, but they need to set up a frame pointer anyway because they
allocate a 32-byte-aligned buffer on the stack.)

As a result, during these functions, 8(%rbp) doesn't contain a return
address like it is supposed to.

This differs from the convention used by C code, and it interferes with
the frame pointer unwinder (CONFIG_UNWINDER_FRAME_POINTER=y).
Specifically, if a call trace is dumped in these functions, then all
entries are populated purely from the fallback scan of the entire stack:

    RIP: 0010:sha512_transform_rorx+0x673/0xdf0
    [...]
    Call Trace:
     <TASK>
     ? sha512_blocks_avx2+0x37/0x60
     ? __sha512_update+0x80/0x230
     ? sha512+0x31/0x190
     ? sha512+0x114/0x190
     [...]
     ? do_syscall_64+0x6f/0x3e0

With this fix to follow the normal %rbp convention, the call trace is
generated reliably:

    RIP: 0010:sha512_transform_rorx+0x90c/0xdf0
    [...]
    Call Trace:
     <TASK>
     sha512_blocks_avx2+0x37/0x60
     __sha512_update+0x80/0x230
     sha512+0x114/0x190
     [...]
     do_syscall_64+0x6f/0x3e0

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 lib/crypto/x86/sha512-avx-asm.S   | 11 ++++++-----
 lib/crypto/x86/sha512-avx2-asm.S  | 11 ++++++-----
 lib/crypto/x86/sha512-ssse3-asm.S | 11 ++++++-----
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/lib/crypto/x86/sha512-avx-asm.S b/lib/crypto/x86/sha512-avx-asm.S
index 7732aa8fd850..a17dfedc1c51 100644
--- a/lib/crypto/x86/sha512-avx-asm.S
+++ b/lib/crypto/x86/sha512-avx-asm.S
@@ -276,6 +276,9 @@ frame_size = frame_WK + WK_SIZE
 ########################################################################
 SYM_FUNC_START(sha512_transform_avx)
 
+	push	%rbp
+	mov	%rsp, %rbp
+
 	# Save GPRs
 	push	%rbx
 	push	%r12
@@ -284,8 +287,6 @@ SYM_FUNC_START(sha512_transform_avx)
 	push	%r15
 
 	# Allocate Stack Space
-	push	%rbp
-	mov	%rsp, %rbp
 	sub     $frame_size, %rsp
 	and	$~(0x20 - 1), %rsp
 
@@ -348,9 +349,8 @@ SYM_FUNC_START(sha512_transform_avx)
 	dec     msglen
 	jnz     .Lupdateblock
 
-	# Restore Stack Pointer
-	mov	%rbp, %rsp
-	pop	%rbp
+	# Deallocate Stack Space
+	lea	-40(%rbp), %rsp
 
 	# Restore GPRs
 	pop	%r15
@@ -358,6 +358,7 @@ SYM_FUNC_START(sha512_transform_avx)
 	pop	%r13
 	pop	%r12
 	pop	%rbx
+	pop	%rbp
 
 	RET
 SYM_FUNC_END(sha512_transform_avx)
diff --git a/lib/crypto/x86/sha512-avx2-asm.S b/lib/crypto/x86/sha512-avx2-asm.S
index 22bdbfd899d0..2256d51297e1 100644
--- a/lib/crypto/x86/sha512-avx2-asm.S
+++ b/lib/crypto/x86/sha512-avx2-asm.S
@@ -568,6 +568,9 @@ frame_size = frame_CTX + CTX_SIZE
 ########################################################################
 SYM_FUNC_START(sha512_transform_rorx)
 
+	push	%rbp
+	mov	%rsp, %rbp
+
 	# Save GPRs
 	push	%rbx
 	push	%r12
@@ -576,8 +579,6 @@ SYM_FUNC_START(sha512_transform_rorx)
 	push	%r15
 
 	# Allocate Stack Space
-	push	%rbp
-	mov	%rsp, %rbp
 	sub	$frame_size, %rsp
 	and	$~(0x20 - 1), %rsp
 
@@ -667,9 +668,8 @@ SYM_FUNC_START(sha512_transform_rorx)
 	cmp	frame_INPEND(%rsp), INP
 	jne	.Lloop0
 
-	# Restore Stack Pointer
-	mov	%rbp, %rsp
-	pop	%rbp
+	# Deallocate Stack Space
+	lea	-40(%rbp), %rsp
 
 	# Restore GPRs
 	pop	%r15
@@ -677,6 +677,7 @@ SYM_FUNC_START(sha512_transform_rorx)
 	pop	%r13
 	pop	%r12
 	pop	%rbx
+	pop	%rbp
 
 	vzeroupper
 	RET
diff --git a/lib/crypto/x86/sha512-ssse3-asm.S b/lib/crypto/x86/sha512-ssse3-asm.S
index 4cae7445b2a8..1a37b7a735cf 100644
--- a/lib/crypto/x86/sha512-ssse3-asm.S
+++ b/lib/crypto/x86/sha512-ssse3-asm.S
@@ -275,6 +275,9 @@ frame_size = frame_WK + WK_SIZE
 ########################################################################
 SYM_FUNC_START(sha512_transform_ssse3)
 
+	push	%rbp
+	mov	%rsp, %rbp
+
 	# Save GPRs
 	push	%rbx
 	push	%r12
@@ -283,8 +286,6 @@ SYM_FUNC_START(sha512_transform_ssse3)
 	push	%r15
 
 	# Allocate Stack Space
-	push	%rbp
-	mov	%rsp, %rbp
 	sub	$frame_size, %rsp
 	and	$~(0x20 - 1), %rsp
 
@@ -347,9 +348,8 @@ SYM_FUNC_START(sha512_transform_ssse3)
 	dec	msglen
 	jnz	.Lupdateblock
 
-	# Restore Stack Pointer
-	mov	%rbp, %rsp
-	pop	%rbp
+	# Deallocate Stack Space
+	lea	-40(%rbp), %rsp
 
 	# Restore GPRs
 	pop	%r15
@@ -357,6 +357,7 @@ SYM_FUNC_START(sha512_transform_ssse3)
 	pop	%r13
 	pop	%r12
 	pop	%rbx
+	pop	%rbp
 
 	RET
 SYM_FUNC_END(sha512_transform_ssse3)
-- 
2.56.0

Re: [PATCH 0/3] lib/crypto: x86/sha: Follow the normal %rbp convention

[Ard Biesheuvel]

On Sun, 11 Oct 2026, at 15:48, Eric Biggers wrote:
> This series updates the x86 SHA-1, SHA-256, and SHA-512 assembly code to
> set up %rbp before pushing GPRs rather than after, so that the frame
> pointer unwinder will generate call traces reliably.
>
> This doesn't matter much, given that the ORC unwinder is the default,
> and the fallback to scanning the stack made call traces sort of "work"
> anyway.  But it annoyed me that these assembly functions use the "wrong"
> %rbp convention, differing from the C code.
>
> This series is targeting libcrypto-next.
>
> Eric Biggers (3):
>   lib/crypto: x86/sha1: Follow the normal %rbp convention
>   lib/crypto: x86/sha256: Follow the normal %rbp convention
>   lib/crypto: x86/sha512: Follow the normal %rbp convention
>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>