lib/crypto: x86/sha: Follow the normal %rbp convention
Subsystems (2)
Eric Biggers
maintainercover lib/crypto: x86/sha: Follow the normal %rbp convention
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
Signature · 1 line
2.56.0
Eric Biggers
maintainerPATCH 1/3 lib/crypto: x86/sha1: Follow the normal %rbp convention
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/0x3e0With 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--- lib/crypto/x86/sha1-avx2-asm.S | 10 +++++----- lib/crypto/x86/sha1-ssse3-and-avx.S | 9 +++++---- 2 files changed, 10 insertions(+), 9 deletions(-)
lib/crypto/x86/sha1-avx2-asm.S +5 -5
@@ -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
lib/crypto/x86/sha1-ssse3-and-avx.S +5 -4
@@ -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)
Signature · 1 line
2.56.0
Eric Biggers
maintainerPATCH 2/3 lib/crypto: x86/sha256: Follow the normal %rbp convention
In reply to Eric BiggersSome 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/0x3e0With 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--- 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(-)
lib/crypto/x86/sha256-avx-asm.S +5 -5
@@ -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)
lib/crypto/x86/sha256-avx2-asm.S +4 -6
@@ -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)
lib/crypto/x86/sha256-ni-asm.S +5 -4
@@ -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)
lib/crypto/x86/sha256-ssse3-asm.S +4 -4
@@ -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)
Signature · 1 line
2.56.0
Eric Biggers
maintainerPATCH 3/3 lib/crypto: x86/sha512: Follow the normal %rbp convention
In reply to Eric BiggersThe 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/0x3e0With 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--- 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(-)
lib/crypto/x86/sha512-avx-asm.S +6 -5
@@ -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)
lib/crypto/x86/sha512-avx2-asm.S +6 -5
@@ -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
lib/crypto/x86/sha512-ssse3-asm.S +6 -5
@@ -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)
Signature · 1 line
2.56.0
Quoted 9 lines · 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.
Quoted 7 lines
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
Quoted 16 lines · 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
Quoted 10 lines
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(-)
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-next
- Eric