From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x31.google.com (mail-oa1-x31.google.com [IPv6:2001:4860:4864:20::31]) by sourceware.org (Postfix) with ESMTPS id A4DE03857C49 for ; Thu, 4 Jan 2024 19:51:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A4DE03857C49 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A4DE03857C49 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:4860:4864:20::31 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704397876; cv=none; b=LJahDjXTB1krTB62HptxBirjIHPRwDOzelftNmcD0LUt8mTW5YU4caHFakXcYpXrQnMW5mpD31ZifmF0Yr6nCSxaDD2EOzcWJujNZ+qzhTheUfFHjsfoZJJM89rtA7y9mRNt7TefFcLGQcymCc27Tj1cWQzSfCnelGXcE10g5Yo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704397876; c=relaxed/simple; bh=WaZOP5Wed7s0wxoknGtC48APEfeZu3hDUXXTg6VZkC8=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=i28jn/FVfH1HYfIbhLBHb2q2+xB0yxbWe6qZ6STSdqYIdIAoeTe0GUFGarMtrtznut36tx4dnHXn2fzstMVkvvvdnj/Q/5Rs2HCJke61eGSPKJ8RwlDDsbSSLenR8I+Vr66k3S1CSJsfgvQI0yRhamAJ/t1h/rmijZKzuE9GKXA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-oa1-x31.google.com with SMTP id 586e51a60fabf-204fdd685fdso569914fac.2 for ; Thu, 04 Jan 2024 11:51:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1704397873; x=1705002673; darn=sourceware.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=t8QrByBZa/w8j/XJ2X0T7xWrT1wpRLIx0/z6T69E/CI=; b=Zy1ocaH6sb7ojjyMhdlnWuUz/dTd8HHhCGuBjghOwpyBl6IW66FpIGAKC8Hn+LPNKa 29Lx1UbiyqhUCw6vfazxenQnX5ichKaLttcujD8OdEASzC5/bBrNWFi/CZZDfEWDk0L/ QN1YqE76pqtccs4pd3IJwcbW0W9AqrcjTL3oLtKwnMdbzEZtRJQ2m6NVX2B+laFEQ8WS eIMhwagpjqQqBjykZ0363FgulhP0iPA5uZI+RUkw6HqTr/M/AuBfX1xk7g3EwMH/nxaS dC5CrJvh475Y/2QMb0I026yuHwGKYuV27leAzlGFPVp6dESqREwF5Q6ufw8UXnndhxuO Q2tw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704397873; x=1705002673; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=t8QrByBZa/w8j/XJ2X0T7xWrT1wpRLIx0/z6T69E/CI=; b=LM/cCFg7Q7tJY7J3+OVyFjDsEztIrWPnr10ZWSfHFqkk3xexXZG05wWwP9Rd0O8xxM gNwKGn1rPxTwkszXMAf8J3rS0joGRdp9LWoGoF2brvNfHFSNo/SwlrGhbx9l7n0o2Khm 9hdTTI1tIPb7cE15KeASS41QdNCrKN+9GIQBs/r5XEB+03cL8EXkD7UhICSvzf9le4zS PKzom60PLaeJ8iBfsIUV9aKDQ9mLQvv0zz7Qx2gC5Xj+3UUCfyPvS6HBQXPJarGVBPXO OIXZoG8WpPedaAeLR7AiB854NulG9deXxBmSyd+8hcWUuGk50teGOsGs3ykj3f2lZ7hU uF6w== X-Gm-Message-State: AOJu0YyPAA76lp/hMAjea3SiYBqRyNr4cQMzeo1vmEXVlEl5AhFasAY/ 9RvjgulSdidSWhlRwcpss2YvhWu1uGMhRmW3ibiMnHNf X-Google-Smtp-Source: AGHT+IGMrVUn2k6QAfv9FOab9py2cEr+ZYklbtXKiKKtn6A+1hG8Of8GHUhmLHtjqHdaVQEb7f5lQROGdU+DHNPB7s8= X-Received: by 2002:a05:6870:a550:b0:205:c642:f4df with SMTP id p16-20020a056870a55000b00205c642f4dfmr988730oal.3.1704397872911; Thu, 04 Jan 2024 11:51:12 -0800 (PST) MIME-Version: 1.0 References: <20240102150329.3152784-1-hjl.tools@gmail.com> In-Reply-To: <20240102150329.3152784-1-hjl.tools@gmail.com> From: Noah Goldstein Date: Thu, 4 Jan 2024 11:51:00 -0800 Message-ID: Subject: Re: [PATCH v2] x86-64/cet: Check the restore token in longjmp To: "H.J. Lu" Cc: libc-alpha@sourceware.org, rick.p.edgecombe@intel.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Tue, Jan 2, 2024 at 7:03=E2=80=AFAM H.J. Lu wrote: > > Changes in v2: > > 1. Merge __longjmp.S and __longjmp_chk.S by adding CHECK_INVALID_LONGJMP. > 2. Remove a branch in the restore token searching loop. > 3. Cache the target shadow stack pointer. > > setcontext and swapcontext put a restore token on the old shadow stack > which is used to restore the target shadow stack when switching user > contexts. When longjmp from a user context, the target shadow stack > can be different from the current shadow stack and INCSSP can't be > used to restore the shadow stack pointer to the target shadow stack. > > Update longjmp to search for a restore token. If found, use the token > to restore the shadow stack pointer before using INCSSP to pop the > shadow stack. Stop the token search and use INCSSP if the shadow stack > entry value is the same as the current shadow stack pointer. > > It is a user error if there is a shadow stack switch without leaving a > restore token on the old shadow stack. > > The only difference between __longjmp.S and __longjmp_chk.S is that > __longjmp_chk.S has a check for invalid longjmp usages. Merge > __longjmp.S and __longjmp_chk.S by adding the CHECK_INVALID_LONGJMP > macro. > --- > .../unix/sysv/linux/x86_64/____longjmp_chk.S | 179 ++++-------------- > sysdeps/x86/__longjmp_cancel.S | 3 + > sysdeps/x86_64/__longjmp.S | 47 ++++- > 3 files changed, 84 insertions(+), 145 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S b/sysdeps/u= nix/sysv/linux/x86_64/____longjmp_chk.S > index deb6398f43..9aa24620b9 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S > +++ b/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S > @@ -15,18 +15,7 @@ > License along with the GNU C Library; if not, see > . */ > > -#include > -#include > -#include > -#include > -#include > #include > -#include > - > -/* Don't restore shadow stack register if shadow stack isn't enabled. *= / > -#if !SHSTK_ENABLED > -# undef SHADOW_STACK_POINTER_OFFSET > -#endif > > .section .rodata.str1.1,"aMS",@progbits,1 > .type longjmp_msg,@object > @@ -34,136 +23,48 @@ longjmp_msg: > .string "longjmp causes uninitialized stack frame" > .size longjmp_msg, .-longjmp_msg > > - > -//#define __longjmp ____longjmp_chk > - > #ifdef PIC > -# define CALL_FAIL sub $8, %RSP_LP; = \ > - cfi_remember_state; = \ > - cfi_def_cfa_offset(16); = \ > - lea longjmp_msg(%rip), %RDI_LP; = \ > - call HIDDEN_JUMPTARGET(__fortify_fail); = \ > - nop; = \ > - cfi_restore_state > +# define LOAD_MSG lea longjmp_msg(%rip), %RDI_LP > #else > -# define CALL_FAIL sub $8, %RSP_LP; = \ > - cfi_remember_state; = \ > - cfi_def_cfa_offset(16); = \ > - mov $longjmp_msg, %RDI_LP; = \ > - call HIDDEN_JUMPTARGET(__fortify_fail); = \ > - nop; = \ > - cfi_restore_state > +# define LOAD_MSG mov $longjmp_msg, %RDI_LP > #endif > > -/* Jump to the position specified by ENV, causing the > - setjmp call there to return VAL, or 1 if VAL is 0. > - void __longjmp (__jmp_buf env, int val). */ > - .text > -ENTRY(____longjmp_chk) > - /* Restore registers. */ > - mov (JB_RSP*8)(%rdi), %R8_LP > - mov (JB_RBP*8)(%rdi),%R9_LP > - mov (JB_PC*8)(%rdi), %RDX_LP > -#ifdef PTR_DEMANGLE > - PTR_DEMANGLE (%R8_LP) > - PTR_DEMANGLE (%R9_LP) > - PTR_DEMANGLE (%RDX_LP) > -# ifdef __ILP32__ > - /* We ignored the high bits of the %rbp value because only the lo= w > - bits are mangled. But we cannot presume that %rbp is being us= ed > - as a pointer and truncate it, so recover the high bits. */ > - movl (JB_RBP*8 + 4)(%rdi), %eax > - shlq $32, %rax > - orq %rax, %r9 > -# endif > -#endif > - > - cmp %R8_LP, %RSP_LP > - jbe .Lok > - > - /* Save function parameters. */ > - movq %rdi, %r10 > - cfi_register (%rdi, %r10) > - movl %esi, %ebx > - cfi_register (%rsi, %rbx) > - > - xorl %edi, %edi > - lea -sizeSS(%rsp), %RSI_LP > - movl $__NR_sigaltstack, %eax > - syscall > - /* Without working sigaltstack we cannot perform the test. */ > - testl %eax, %eax > - jne .Lok2 > - testl $1, (-sizeSS + oSS_FLAGS)(%rsp) > - jz .Lfail > - > - mov (-sizeSS + oSS_SP)(%rsp), %RAX_LP > - add (-sizeSS + oSS_SIZE)(%rsp), %RAX_LP > - sub %R8_LP, %RAX_LP > - cmp (-sizeSS + oSS_SIZE)(%rsp), %RAX_LP > - jae .Lok2 > - > -.Lfail: CALL_FAIL > - > -.Lok2: movq %r10, %rdi > - cfi_restore (%rdi) > - movl %ebx, %esi > - cfi_restore (%rsi) > - > +#define CHECK_INVALID_LONGJMP \ > + cmp %R8_LP, %RSP_LP; \ > + jbe .Lok; \ > + /* Save function parameters. */ \ > + movq %rdi, %r10; \ > + cfi_register (%rdi, %r10); \ > + movl %esi, %ebx; \ > + cfi_register (%rsi, %rbx); \ > + xorl %edi, %edi; \ > + lea -sizeSS(%rsp), %RSI_LP; \ > + movl $__NR_sigaltstack, %eax; \ > + syscall; \ > + /* Without working sigaltstack we cannot perform the test. */ \ > + testl %eax, %eax; \ > + jne .Lok2; \ > + testl $1, (-sizeSS + oSS_FLAGS)(%rsp); \ > + jz .Lfail; \ > + mov (-sizeSS + oSS_SP)(%rsp), %RAX_LP; \ > + add (-sizeSS + oSS_SIZE)(%rsp), %RAX_LP; \ > + sub %R8_LP, %RAX_LP; \ > + cmp (-sizeSS + oSS_SIZE)(%rsp), %RAX_LP; \ > + jae .Lok2; \ > +.Lfail: = \ > + sub $8, %RSP_LP; \ > + cfi_remember_state; \ > + cfi_def_cfa_offset(16); \ > + LOAD_MSG; \ > + call HIDDEN_JUMPTARGET(__fortify_fail); \ > + nop; \ what is this nop for? If you want `.Lok2` aligned just use `.p2align`? > + cfi_restore_state; \ > +.Lok2: \ > + movq %r10, %rdi; \ > + cfi_restore (%rdi); \ > + movl %ebx, %esi; \ > + cfi_restore (%rsi); \ > .Lok: > -#ifdef SHADOW_STACK_POINTER_OFFSET > -# if IS_IN (libc) && defined SHARED && defined FEATURE_1_OFFSET > - /* Check if Shadow Stack is enabled. */ > - testl $X86_FEATURE_1_SHSTK, %fs:FEATURE_1_OFFSET > - jz L(skip_ssp) > -# else > - xorl %eax, %eax > -# endif > - /* Check and adjust the Shadow-Stack-Pointer. */ > - rdsspq %rax > - /* And compare it with the saved ssp value. */ > - subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax > - je L(skip_ssp) > - /* Count the number of frames to adjust and adjust it > - with incssp instruction. The instruction can adjust > - the ssp by [0..255] value only thus use a loop if > - the number of frames is bigger than 255. */ > - negq %rax > - shrq $3, %rax > - /* NB: We saved Shadow-Stack-Pointer of setjmp. Since we are > - restoring Shadow-Stack-Pointer of setjmp's caller, we > - need to unwind shadow stack by one more frame. */ > - addq $1, %rax > - movl $255, %ebx > -L(loop): > - cmpq %rbx, %rax > - cmovb %rax, %rbx > - incsspq %rbx > - subq %rbx, %rax > - ja L(loop) > -L(skip_ssp): > -#endif > - LIBC_PROBE (longjmp, 3, LP_SIZE@%RDI_LP, -4@%esi, LP_SIZE@%RDX_LP= ) > - /* We add unwind information for the target here. */ > - cfi_def_cfa(%rdi, 0) > - cfi_register(%rsp,%r8) > - cfi_register(%rbp,%r9) > - cfi_register(%rip,%rdx) > - cfi_offset(%rbx,JB_RBX*8) > - cfi_offset(%r12,JB_R12*8) > - cfi_offset(%r13,JB_R13*8) > - cfi_offset(%r14,JB_R14*8) > - cfi_offset(%r15,JB_R15*8) > - movq (JB_RBX*8)(%rdi), %rbx > - movq (JB_R12*8)(%rdi), %r12 > - movq (JB_R13*8)(%rdi), %r13 > - movq (JB_R14*8)(%rdi), %r14 > - movq (JB_R15*8)(%rdi), %r15 > - /* Set return value for setjmp. */ > - movl %esi, %eax > - mov %R8_LP, %RSP_LP > - movq %r9,%rbp > - LIBC_PROBE (longjmp_target, 3, > - LP_SIZE@%RDI_LP, -4@%eax, LP_SIZE@%RDX_LP) > - jmpq *%rdx > -END (____longjmp_chk) > + > +#define __longjmp ____longjmp_chk > +#include <__longjmp.S> > diff --git a/sysdeps/x86/__longjmp_cancel.S b/sysdeps/x86/__longjmp_cance= l.S > index e71b304257..b03f52b308 100644 > --- a/sysdeps/x86/__longjmp_cancel.S > +++ b/sysdeps/x86/__longjmp_cancel.S > @@ -16,5 +16,8 @@ > License along with the GNU C Library; if not, see > . */ > > +/* Don't restore shadow stack register for __longjmp_cancel. */ > +#define DO_NOT_RESTORE_SHADOW_STACK > + > #define __longjmp __longjmp_cancel > #include <__longjmp.S> > diff --git a/sysdeps/x86_64/__longjmp.S b/sysdeps/x86_64/__longjmp.S > index c9f70f8e2a..22fedc4997 100644 > --- a/sysdeps/x86_64/__longjmp.S > +++ b/sysdeps/x86_64/__longjmp.S > @@ -22,14 +22,15 @@ > #include > #include > > -/* Don't restore shadow stack register if > - 1. Shadow stack isn't enabled. Or > - 2. __longjmp is defined for __longjmp_cancel. > - */ > -#if !SHSTK_ENABLED || defined __longjmp > +/* Don't restore shadow stack register if shadow stack isn't enabled. *= / > +#if !SHSTK_ENABLED || defined DO_NOT_RESTORE_SHADOW_STACK > # undef SHADOW_STACK_POINTER_OFFSET > #endif > > +#ifndef CHECK_INVALID_LONGJMP > +# define CHECK_INVALID_LONGJMP > +#endif > + > /* Jump to the position specified by ENV, causing the > setjmp call there to return VAL, or 1 if VAL is 0. > void __longjmp (__jmp_buf env, int val). */ > @@ -52,6 +53,9 @@ ENTRY(__longjmp) > orq %rax, %r9 > # endif > #endif > + > + CHECK_INVALID_LONGJMP This only runs if `__longjmp` is defined `____longjmp_chk` correct? As far as I can tell its only used once. Maybe have the code here i.e: #ifdef SHADOW_STACK_POINT_OFFSET //cur code #else //CHECK_INVALID_LONGJMP code #endif or am I misunderstanding? > + > #ifdef SHADOW_STACK_POINTER_OFFSET > # if IS_IN (libc) && defined SHARED && defined FEATURE_1_OFFSET > /* Check if Shadow Stack is enabled. */ > @@ -63,9 +67,40 @@ ENTRY(__longjmp) > /* Check and adjust the Shadow-Stack-Pointer. */ > /* Get the current ssp. */ > rdsspq %rax > + /* Save the current ssp. */ > + movq %rax, %r10 > /* And compare it with the saved ssp value. */ > - subq SHADOW_STACK_POINTER_OFFSET(%rdi), %rax > + movq SHADOW_STACK_POINTER_OFFSET(%rdi), %rcx > + subq %rcx, %rax > je L(skip_ssp) > + > + /* Save the target ssp. */ > + movq %rcx, %r11 > + > +L(find_restore_token_loop): > + /* Look for a restore token. */ > + movq -8(%rcx), %rbx > + andq $-8, %rbx > + cmpq %rcx, %rbx > + /* Find the restore token. */ > + je L(restore_shadow_stack) > + > + /* Try the next slot. */ > + subq $8, %rcx > + /* Stop if the current ssp is found. */ > + cmpq %rcx, %r10 > + jne L(find_restore_token_loop) > + jmp L(no_shadow_stack_token) > + > +L(restore_shadow_stack): > + /* Restore the target shadow stack. */ > + rstorssp -8(%rcx) > + /* Save the restore token on the old shadow stack. */ > + saveprevssp > + rdsspq %rax > + subq %r11, %rax > + > +L(no_shadow_stack_token): > /* Count the number of frames to adjust and adjust it > with incssp instruction. The instruction can adjust > the ssp by [0..255] value only thus use a loop if > -- > 2.43.0 >