diff mbox series

arm: Fix an incorrect check in ____longjmp_chk [BZ #27709]

Message ID 20210408174724.19063-1-szabolcs.nagy@arm.com
State Committed
Commit 8d4d77f6c848538cfb9e5ad0a14825e7ae4a1657
Headers
Series arm: Fix an incorrect check in ____longjmp_chk [BZ #27709] |

Commit Message

Szabolcs Nagy April 8, 2021, 5:47 p.m. UTC 
  An incorrect check in __longjmp_chk could fail on valid code causing

FAIL: debug/tst-longjmp_chk2

The original check was

  altstack_sp + altstack_size - setjmp_sp > altstack_size

i.e. sp at setjmp was outside of the altstack range. Here we know that
longjmp is called from a signal handler on the altstack (SS_ONSTACK),
and that it jumps in the wrong direction (sp decreases), so the check
wants to ensure the jump goes to another stack.

The check is wrong when altstack_sp == setjmp_sp which can happen
when the altstack is a local buffer in the function that calls setjmp,
so the patch allows == too. This fixes bug 27709.

Note that the generic __longjmp_chk check seems to be different.
(it checks if longjmp was on the altstack but does not check setjmp,
so it would not catch incorrect longjmp use within the signal handler).
---
 sysdeps/unix/sysv/linux/arm/____longjmp_chk.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andreas Schwab April 12, 2021, 12:45 p.m. UTC | #1 
On Apr 08 2021, Szabolcs Nagy via Libc-alpha wrote:

> An incorrect check in __longjmp_chk could fail on valid code causing
>
> FAIL: debug/tst-longjmp_chk2
>
> The original check was
>
>   altstack_sp + altstack_size - setjmp_sp > altstack_size
>
> i.e. sp at setjmp was outside of the altstack range. Here we know that
> longjmp is called from a signal handler on the altstack (SS_ONSTACK),
> and that it jumps in the wrong direction (sp decreases), so the check
> wants to ensure the jump goes to another stack.
>
> The check is wrong when altstack_sp == setjmp_sp which can happen
> when the altstack is a local buffer in the function that calls setjmp,
> so the patch allows == too. This fixes bug 27709.
>
> Note that the generic __longjmp_chk check seems to be different.
> (it checks if longjmp was on the altstack but does not check setjmp,
> so it would not catch incorrect longjmp use within the signal handler).

Ok.

Andreas.
diff mbox series

Patch

diff --git a/sysdeps/unix/sysv/linux/arm/____longjmp_chk.S b/sysdeps/unix/sysv/linux/arm/____longjmp_chk.S
index 7f35b4da7e..a26315b7cf 100644
--- a/sysdeps/unix/sysv/linux/arm/____longjmp_chk.S
+++ b/sysdeps/unix/sysv/linux/arm/____longjmp_chk.S
@@ -72,8 +72,8 @@  longjmp_msg:
 	ldr	r3, [sp, #8];			\
 	add	r2, r2, r3;			\
 	sub	r2, r2, reg;			\
-	cmp	r2, r3;				\
-	bhi	.Lok2;				\
+	cmp	r3, r2;				\
+	bls	.Lok2;				\
 .Lfail:						\
 	add	sp, sp, #12;			\
 	cfi_adjust_cfa_offset (-12);		\