| Message ID | CAMe9rOqHrEzjWR3H7cYjM1X97WHSPy8m3TG97oxadhgRwiSm+w@mail.gmail.com |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 31695 invoked by alias); 15 Jul 2018 13:54:35 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 31685 invoked by uid 89); 15 Jul 2018 13:54:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=intent X-HELO: mail-oi0-f54.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=nkS1XOZ0MJHdHWBbWNyj3eGqP6fTXyxQQjfYIJtgL00=; b=oCw/MGIlW/FyY3j4eEmX+sKfBlOr1/Nr71vZ9UuaXOfq1ExSV3WqB8DMH2lDWniQGm tMfflYVxIIB+zMstXq4eDsHup57y/6KNUxlMNX30EqGa0fxj9xFTFuo4DyMpwzasGlNu p+kdsn2Ci020rAnnh171hcuUbexCsrwYOGac55N/gPPBIQqJZYL5krWuQswC3B5h9fuy xLsRdrF7W3q33tw1WmD1guhQM4fL4gmre48RGS/E6LIU5CitWW1Pa/RtNv5KdHDDU4QS n5DN0TlBDRi8enMUvMm6QnZC0D5gVLUSuv11qyxfBSAVYm3TX6it8LC00vkXavFb0/98 qAOw== MIME-Version: 1.0 In-Reply-To: <87fu0kj50z.fsf@mid.deneb.enyo.de> References: <20180613153207.57232-1-hjl.tools@gmail.com> <20180613153207.57232-3-hjl.tools@gmail.com> <abc02573-7240-e248-9bfe-bdc324b9e07d@redhat.com> <20180713132252.GC2606@gmail.com> <87fu0lk2tf.fsf@mid.deneb.enyo.de> <CAMe9rOqufMcMEB0V0y8ssTYsOi2_31ad7W9rniRLHHvsGppUvQ@mail.gmail.com> <87wotxifg7.fsf@mid.deneb.enyo.de> <CAMe9rOpCHBK1bDmC6mVEB7rGhfEdp6Ka=juBNN3pzZsKnNSocg@mail.gmail.com> <87fu0kj50z.fsf@mid.deneb.enyo.de> From: "H.J. Lu" <hjl.tools@gmail.com> Date: Sun, 15 Jul 2018 06:54:30 -0700 Message-ID: <CAMe9rOqHrEzjWR3H7cYjM1X97WHSPy8m3TG97oxadhgRwiSm+w@mail.gmail.com> Subject: Re: V2: [PATCH 02/24] x86: Support shadow stack pointer in setjmp/longjmp To: Florian Weimer <fw@deneb.enyo.de> Cc: "Carlos O'Donell" <carlos@redhat.com>, GNU C Library <libc-alpha@sourceware.org> Content-Type: text/plain; charset="UTF-8" |
Commit Message
H.J. Lu
July 15, 2018, 1:54 p.m. UTC
On Sun, Jul 15, 2018 at 1:07 AM, Florian Weimer <fw@deneb.enyo.de> wrote: > * H. J. Lu: > >>> We currently have this (as of commit >>> faaee1f07ed25b2779bfd935ffb29f431b80d6d3): >>> >>> ==> sysdeps/unix/sysv/linux/x86/jmp_buf-ssp.sym <== >>> #include <setjmpP.h> >>> #undef __saved_mask >>> >>> -- >>> SHADOW_STACK_POINTER_OFFSET offsetof(struct __jmp_buf_tag, >>> __saved_mask.__saved.__shadow_stack_pointer) >>> >>> ==> sysdeps/x86/jmp_buf-ssp.sym <== >>> -- FIXME: Define SHADOW_STACK_POINTER_OFFSET to support shadow stack. >>> >>> So SHADOW_STACK_POINTER_OFFSET is defined unconditionally. I don't >>> see how the quoted patch changes that. >>> >>> Making sure that rdssp is only assembled with --enable-cet looks like >>> the right solution, but you need something like #if ENABLE_CET, and >>> not depend on SHADOW_STACK_POINTER_OFFSET being defined. >> >> Take sysdeps/x86_64/setjmp.S as example: >> >> /* Don't save shadow stack register if shadow stack isn't enabled. */ >> #if !SHSTK_ENABLED >> # undef SHADOW_STACK_POINTER_OFFSET >> #endif >> ..... >> >> Shadow stack pointer is saved/restored only if --enable-cet is used to >> configure glibc. If you compile glibc with -fcf-protection, but without >> configuring glibc with --enable-cet, result is undefined. > > That doesn't work because <jmp_buf-ssp.h> is included after the You are right. sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S checks SHTSTK_ENABLED after <jmp_buf-ssp.h> is included. > #undef. I think you can avoid that by using #if SHTSTK_ENABLED > instead of #ifdef SHADOW_STACK_POINTER_OFFSET, which also expresses > the intent more accurately. It is done on purpose. sysdeps/x86_64/__longjmp.S has /* 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 # undef SHADOW_STACK_POINTER_OFFSET #endif >> BTW, it passed build-many-glibcs.py. > > With binutils 2.28? I am checking in this patch. Tested with build-many-glibcs.py using binutils 2.28.
Comments
* H. J. Lu: >> #undef. I think you can avoid that by using #if SHTSTK_ENABLED >> instead of #ifdef SHADOW_STACK_POINTER_OFFSET, which also expresses >> the intent more accurately. > > It is done on purpose. sysdeps/x86_64/__longjmp.S has > > /* 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 > # undef SHADOW_STACK_POINTER_OFFSET > #endif It's a bit awkward. > Subject: [PATCH] x86_64: Undef SHADOW_STACK_POINTER_OFFSET last > > Since SHADOW_STACK_POINTER_OFFSET is defined in jmp_buf-ssp.h, we must > undef SHADOW_STACK_POINTER_OFFSET after including <jmp_buf-ssp.h>. > > * sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S: Undef > SHADOW_STACK_POINTER_OFFSET after including <jmp_buf-ssp.h>. This looks okay as a fix.
On Sun, Jul 15, 2018 at 12:22 PM, Florian Weimer <fw@deneb.enyo.de> wrote: > * H. J. Lu: > >>> #undef. I think you can avoid that by using #if SHTSTK_ENABLED >>> instead of #ifdef SHADOW_STACK_POINTER_OFFSET, which also expresses >>> the intent more accurately. >> >> It is done on purpose. sysdeps/x86_64/__longjmp.S has >> >> /* 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 >> # undef SHADOW_STACK_POINTER_OFFSET >> #endif > > It's a bit awkward. Linux and Hurd share the same implementation of setjmp/longjmp. But 1. For Linux, SHADOW_STACK_POINTER_OFFSET is always defined regardless if CET is enabled. 2.For Hurd, SHADOW_STACK_POINTER_OFFSET is undefined since it is unknown how to save shadow stack pointer. 3. When CET is enabled, setjmp/longjmp is assembled twice. One preserves shadow stack pointer and the other doesn't. Both versions support IBT. 4. All assembly files are compiled with the same compiler options. SHADOW_STACK_POINTER_OFFSET is used to control if shadow stack pointer should be preserved: 1. If CET isn't enabled, undef SHADOW_STACK_POINTER_OFFSET. 2. If CET is enabled, undef SHADOW_STACK_POINTER_OFFSET when not to preserve shadow stack pointer. >> Subject: [PATCH] x86_64: Undef SHADOW_STACK_POINTER_OFFSET last >> >> Since SHADOW_STACK_POINTER_OFFSET is defined in jmp_buf-ssp.h, we must >> undef SHADOW_STACK_POINTER_OFFSET after including <jmp_buf-ssp.h>. >> >> * sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S: Undef >> SHADOW_STACK_POINTER_OFFSET after including <jmp_buf-ssp.h>. > > This looks okay as a fix. I will check it in. Thanks.
diff --git a/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S b/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S index 7eb26fafca..5d2d275721 100644 --- a/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S +++ b/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S @@ -19,15 +19,14 @@ #include <jmpbuf-offsets.h> #include <asm-syntax.h> #include <stap-probe.h> +#include <sigaltstack-offsets.h> +#include <jmp_buf-ssp.h> /* Don't restore shadow stack register if shadow stack isn't enabled. */ #if !SHSTK_ENABLED # undef SHADOW_STACK_POINTER_OFFSET #endif -#include <sigaltstack-offsets.h> -#include <jmp_buf-ssp.h> - .section .rodata.str1.1,"aMS",@progbits,1 .type longjmp_msg,@object longjmp_msg: