From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 93DDC1F4B4 for ; Wed, 14 Oct 2020 13:31:39 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 521B73857C63; Wed, 14 Oct 2020 13:31:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 521B73857C63 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1602682298; bh=+B03cgFDHCr4ARVWEyhWEPIjUBKpPX6jkPprnJYuVwY=; h=References:In-Reply-To:Date:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=QuPLQziUjrdxIc/8ZYvawrwgi1OZJHyHi5gdGuyiwTHWNfP1oN0XDwV40BY/KNY3R nSyxv167Kdntu0RJFLpaUsgLtSKOYg4J0ZBV7D7Ww8UIfgeLZdzmdqtnF2HzFsDpMS N6f+EvQ3IhLjkShdGsdVrO0LyTxEqw7sKH/b5KWc= Received: from mail-oi1-x244.google.com (mail-oi1-x244.google.com [IPv6:2607:f8b0:4864:20::244]) by sourceware.org (Postfix) with ESMTPS id 1E0553857C46 for ; Wed, 14 Oct 2020 13:31:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1E0553857C46 Received: by mail-oi1-x244.google.com with SMTP id t77so3234561oie.4 for ; Wed, 14 Oct 2020 06:31:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=+B03cgFDHCr4ARVWEyhWEPIjUBKpPX6jkPprnJYuVwY=; b=oRG0YirWZVx+/ioyWlhEDOUkdx6k6Tcl2PLcZB2TMzNHOcPHt9jOHfO+ISTm/8CGr5 97w7eGO2QFO93OCmEv7+rp7rTPol6pUjQmDvyUwzDNxaZYhAV6+VY9UV8rGNVWcRIIl1 wpPRuQOUj0NK+XMEDR/p594XH0E/dJOnh+pitA9rZzP7dQN5fiKQdQPz3BoTlrd7wVxf Zo/MX3g4XcsOgLUqptat6bMKe58m4uBgxUbO3imefnRNyubZGaN38ocm7HEqkhWKH7ya dpiadafa2INiwQTTMwb5ggPyceqCtsHAIXaRrsYLq5pI/hptr7jzL8H5P1A9ev2gqks5 On5g== X-Gm-Message-State: AOAM531U75NOZ3C1mzRIoA73iLkjII5TLmjCdqq2Xje6h6+FYT85Ub50 ceKLDipI6tIFohApxSY0R47Ca6Fue/xkqiNcwIXz7BET X-Google-Smtp-Source: ABdhPJw5LOuQX0DLLS02x3qo5mg6nXx2WiUryZRJ3quziHj17gGaD2+2K/QOxDV6aLDktNU1wlHzF8PBW/Ty6klxIPU= X-Received: by 2002:aca:5058:: with SMTP id e85mr2211381oib.79.1602682294029; Wed, 14 Oct 2020 06:31:34 -0700 (PDT) MIME-Version: 1.0 References: <20201013175146.58558-1-hjl.tools@gmail.com> In-Reply-To: <20201013175146.58558-1-hjl.tools@gmail.com> Date: Wed, 14 Oct 2020 06:30:57 -0700 Message-ID: Subject: Re: [PATCH] x86/CET: Update vfork to prevent child return To: GNU C Library Content-Type: text/plain; charset="UTF-8" X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: "H.J. Lu via Libc-alpha" Reply-To: "H.J. Lu" Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On Tue, Oct 13, 2020 at 10:51 AM H.J. Lu wrote: > > Child of vfork should either call _exit or one of the exec family of > functions. But normally there is nothing to prevent child of vfork from > return of the vfork-calling function. Simpilfy x86 vfork when shadow > stack is in use to introduce mismatched shadow stack in child of vfork > to trigger SIGSEGV when the child returns from the function in which > vfork was called. I will check it in tomorrow if there are no objections. > --- > sysdeps/unix/sysv/linux/i386/vfork.S | 55 +++--------- > sysdeps/unix/sysv/linux/x86/Makefile | 5 ++ > sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c | 88 +++++++++++++++++++ > sysdeps/unix/sysv/linux/x86_64/vfork.S | 36 +++----- > 4 files changed, 113 insertions(+), 71 deletions(-) > create mode 100644 sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c > > diff --git a/sysdeps/unix/sysv/linux/i386/vfork.S b/sysdeps/unix/sysv/linux/i386/vfork.S > index ceb41db0bd..91277a639f 100644 > --- a/sysdeps/unix/sysv/linux/i386/vfork.S > +++ b/sysdeps/unix/sysv/linux/i386/vfork.S > @@ -21,39 +21,6 @@ > #include > #include > > -#if SHSTK_ENABLED > -/* The shadow stack prevents us from pushing the saved return PC onto > - the stack and returning normally. Instead we pop the shadow stack > - and return directly. This is the safest way to return and ensures > - any stack manipulations done by the vfork'd child doesn't cause the > - parent to terminate when CET is enabled. */ > -# undef SYSCALL_ERROR_HANDLER > -# ifdef PIC > -# define SYSCALL_ERROR_HANDLER \ > -0: \ > - calll .L1; \ > -.L1: \ > - popl %edx; \ > -.L2: \ > - addl $_GLOBAL_OFFSET_TABLE_ + (.L2 - .L1), %edx; \ > - movl __libc_errno@gotntpoff(%edx), %edx; \ > - negl %eax; \ > - movl %eax, %gs:(%edx); \ > - orl $-1, %eax; \ > - jmp 1b; > -# else > -# define SYSCALL_ERROR_HANDLER \ > -0: \ > - movl __libc_errno@indntpoff, %edx; \ > - negl %eax; \ > - movl %eax, %gs:(%edx); \ > - orl $-1, %eax; \ > - jmp 1b; > -# endif > -# undef SYSCALL_ERROR_LABEL > -# define SYSCALL_ERROR_LABEL 0f > -#endif > - > /* Clone the calling process, but without copying the whole address space. > The calling process is suspended until the new process exits or is > replaced by a call to `execve'. Return -1 for errors, 0 to the new process, > @@ -70,20 +37,17 @@ ENTRY (__vfork) > movl $SYS_ify (vfork), %eax > int $0x80 > > -#if !SHSTK_ENABLED > /* Jump to the return PC. Don't jump directly since this > disturbs the branch target cache. Instead push the return > address back on the stack. */ > pushl %ecx > cfi_adjust_cfa_offset (4) > -#endif > > cmpl $-4095, %eax > /* Branch forward if it failed. */ > jae SYSCALL_ERROR_LABEL > > #if SHSTK_ENABLED > -1: > /* Check if shadow stack is in use. */ > xorl %edx, %edx > rdsspd %edx > @@ -91,18 +55,19 @@ ENTRY (__vfork) > /* Normal return if shadow stack isn't in use. */ > je L(no_shstk) > > - /* Pop return address from shadow stack and jump back to caller > - directly. */ > - movl $1, %edx > - incsspd %edx > + testl %eax, %eax > + /* In parent, normal return. */ > + jnz L(no_shstk) > + > + /* NB: In child, jump back to caller via indirect branch without > + popping shadow stack which is shared with parent. Keep shadow > + stack mismatched so that child returns in the vfork-calling > + function will trigger SIGSEGV. */ > + popl %ecx > + cfi_adjust_cfa_offset (-4) > jmp *%ecx > > L(no_shstk): > - /* Jump to the return PC. Don't jump directly since this > - disturbs the branch target cache. Instead push the return > - address back on the stack. */ > - pushl %ecx > - cfi_adjust_cfa_offset (4) > #endif > > ret > diff --git a/sysdeps/unix/sysv/linux/x86/Makefile b/sysdeps/unix/sysv/linux/x86/Makefile > index 50fd018fa3..6bfd6bec49 100644 > --- a/sysdeps/unix/sysv/linux/x86/Makefile > +++ b/sysdeps/unix/sysv/linux/x86/Makefile > @@ -40,6 +40,11 @@ $(objpfx)tst-cet-property-2.out: $(objpfx)tst-cet-property-2 \ > $(evaluate-test) > endif > > +ifeq ($(subdir),posix) > +tests += tst-cet-vfork-1 > +CFLAGS-tst-cet-vfork-1.c += -mshstk > +endif > + > ifeq ($(subdir),stdlib) > tests += tst-cet-setcontext-1 > CFLAGS-tst-cet-setcontext-1.c += -mshstk > diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c b/sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c > new file mode 100644 > index 0000000000..5b9fc8c170 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c > @@ -0,0 +1,88 @@ > +/* Verify that child of the vfork-calling function can't return when > + shadow stack is in use. > + Copyright (C) 2020 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +__attribute__ ((noclone, noinline)) > +static void > +do_test_1 (void) > +{ > + pid_t p1; > + int fd[2]; > + > + if (pipe (fd) == -1) > + { > + puts ("pipe failed"); > + _exit (EXIT_FAILURE); > + } > + > + if ((p1 = vfork ()) == 0) > + { > + pid_t p = getpid (); > + TEMP_FAILURE_RETRY (write (fd[1], &p, sizeof (p))); > + /* Child return should trigger SIGSEGV. */ > + return; > + } > + else if (p1 == -1) > + { > + puts ("vfork failed"); > + _exit (EXIT_FAILURE); > + } > + > + pid_t p2 = 0; > + if (TEMP_FAILURE_RETRY (read (fd[0], &p2, sizeof (pid_t))) > + != sizeof (pid_t)) > + puts ("pipd read failed"); > + else > + { > + int r; > + if (TEMP_FAILURE_RETRY (waitpid (p1, &r, 0)) != p1) > + puts ("waitpid failed"); > + else if (r != 0) > + puts ("pip write in child failed"); > + } > + > + /* Parent exits immediately so that parent returns without triggering > + SIGSEGV when shadow stack isn't in use. */ > + _exit (EXIT_FAILURE); > +} > + > +static int > +do_test (void) > +{ > + /* NB: This test should trigger SIGSEGV with shadow stack enabled. */ > + if (_get_ssp () == 0) > + return EXIT_UNSUPPORTED; > + do_test_1 (); > + /* Child exits immediately so that child returns without triggering > + SIGSEGV when shadow stack isn't in use. */ > + _exit (EXIT_FAILURE); > +} > + > +#define EXPECTED_SIGNAL (_get_ssp () == 0 ? 0 : SIGSEGV) > +#include > diff --git a/sysdeps/unix/sysv/linux/x86_64/vfork.S b/sysdeps/unix/sysv/linux/x86_64/vfork.S > index 776d2fc610..613ff7e846 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/vfork.S > +++ b/sysdeps/unix/sysv/linux/x86_64/vfork.S > @@ -20,22 +20,6 @@ > #include > #include > > -#if SHSTK_ENABLED > -/* The shadow stack prevents us from pushing the saved return PC onto > - the stack and returning normally. Instead we pop the shadow stack > - and return directly. This is the safest way to return and ensures > - any stack manipulations done by the vfork'd child doesn't cause the > - parent to terminate when CET is enabled. */ > -# undef SYSCALL_ERROR_HANDLER > -# define SYSCALL_ERROR_HANDLER \ > -0: \ > - SYSCALL_SET_ERRNO; \ > - or $-1, %RAX_LP; \ > - jmp 1b; > -# undef SYSCALL_ERROR_LABEL > -# define SYSCALL_ERROR_LABEL 0f > -#endif > - > /* Clone the calling process, but without copying the whole address space. > The calling process is suspended until the new process exits or is > replaced by a call to `execve'. Return -1 for errors, 0 to the new process, > @@ -53,17 +37,14 @@ ENTRY (__vfork) > movl $SYS_ify (vfork), %eax > syscall > > -#if !SHSTK_ENABLED > /* Push back the return PC. */ > pushq %rdi > cfi_adjust_cfa_offset(8) > -#endif > > cmpl $-4095, %eax > jae SYSCALL_ERROR_LABEL /* Branch forward if it failed. */ > > #if SHSTK_ENABLED > -1: > /* Check if shadow stack is in use. */ > xorl %esi, %esi > rdsspq %rsi > @@ -71,16 +52,19 @@ ENTRY (__vfork) > /* Normal return if shadow stack isn't in use. */ > je L(no_shstk) > > - /* Pop return address from shadow stack and jump back to caller > - directly. */ > - movl $1, %esi > - incsspq %rsi > + testl %eax, %eax > + /* In parent, normal return. */ > + jnz L(no_shstk) > + > + /* NB: In child, jump back to caller via indirect branch without > + popping shadow stack which is shared with parent. Keep shadow > + stack mismatched so that child returns in the vfork-calling > + function will trigger SIGSEGV. */ > + popq %rdi > + cfi_adjust_cfa_offset(-8) > jmp *%rdi > > L(no_shstk): > - /* Push back the return PC. */ > - pushq %rdi > - cfi_adjust_cfa_offset(8) > #endif > > /* Normal return. */ > -- > 2.26.2 > -- H.J.