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=-3.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,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 85B8F1F4B4 for ; Thu, 17 Sep 2020 00:46:47 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6962E386EC72; Thu, 17 Sep 2020 00:46:46 +0000 (GMT) Received: from albireo.enyo.de (albireo.enyo.de [37.24.231.21]) by sourceware.org (Postfix) with ESMTPS id 02AE83857039 for ; Thu, 17 Sep 2020 00:46:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 02AE83857039 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=deneb.enyo.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=fw@deneb.enyo.de Received: from [172.17.203.2] (helo=deneb.enyo.de) by albireo.enyo.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) id 1kIi4P-0000cq-Bm; Thu, 17 Sep 2020 00:46:41 +0000 Received: from fw by deneb.enyo.de with local (Exim 4.92) (envelope-from ) id 1kIi4P-0001eT-6I; Thu, 17 Sep 2020 02:46:41 +0200 From: Florian Weimer To: "H.J. Lu via Libc-alpha" Subject: Re: [PATCH] x86/CET: Update vfork to prevent child return References: <20200916234503.3553822-1-hjl.tools@gmail.com> Date: Thu, 17 Sep 2020 02:46:41 +0200 In-Reply-To: <20200916234503.3553822-1-hjl.tools@gmail.com> (H. J. Lu via Libc-alpha's message of "Wed, 16 Sep 2020 16:45:03 -0700") Message-ID: <874knx1b7y.fsf@mid.deneb.enyo.de> MIME-Version: 1.0 Content-Type: text/plain 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: , Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" * H. J. Lu via Libc-alpha: > 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 > returning to caller of vfork's caller. With shadow stack enabled, we > can introduce mismatched shadow stack in child of vfork. When the child > returns from the function in which vfork was called, mismatched shadow > stack will trigger SIGSEGV. > --- > sysdeps/unix/sysv/linux/i386/vfork.S | 6 +++ > sysdeps/unix/sysv/linux/x86/Makefile | 5 ++ > sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c | 54 +++++++++++++++++++ > sysdeps/unix/sysv/linux/x86_64/vfork.S | 6 +++ > 4 files changed, 71 insertions(+) > 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..e54fdb7e4c 100644 > --- a/sysdeps/unix/sysv/linux/i386/vfork.S > +++ b/sysdeps/unix/sysv/linux/i386/vfork.S > @@ -91,6 +91,12 @@ ENTRY (__vfork) > /* Normal return if shadow stack isn't in use. */ > je L(no_shstk) > > + testl %eax, %eax > + jnz 2f > + /* NB: Jump back to caller directly with mismatched shadow stack > + to prevent child return. */ > + jmp *%ecx > +2: Doesn't the jmp need a notrack prefix? Or does GCC generate special code for returns_twice functions? The comment should say that the *function* calling vfork cannot return in the subprocess. > diff --git a/sysdeps/unix/sysv/linux/x86/Makefile b/sysdeps/unix/sysv/linux/x86/Makefile > index 920edd8948..f3fae85c1e 100644 > --- a/sysdeps/unix/sysv/linux/x86/Makefile > +++ b/sysdeps/unix/sysv/linux/x86/Makefile > @@ -47,6 +47,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 Does -mshstk alter the ISA? Then I think you can't test for the presence of support if you build the whole translation unit with -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..9ca148e857 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/x86/tst-cet-vfork-1.c > @@ -0,0 +1,54 @@ > +/* Verify that child of vfork can't return with shadow stack. Likewise: It's the vfork-calling function that must not return. > +__attribute__ ((noclone, noinline)) > +static pid_t > +do_test_1 (void) > +{ > + pid_t pid; > + > + pid = vfork (); > + if (pid == 0) > + { > + /* Child return should trigger SIGSEGV. */ > + return 0; > + } > + _exit (EXIT_SUCCESS); > + > + return pid; The return statement immediately above is unreachable. > +static int > +do_test (void) > +{ > + /* NB: This test should trigger SIGSEGV with shadow stack enabled. */ > + if (_get_ssp () == 0) > + return EXIT_UNSUPPORTED; > + return do_test_1 () ? EXIT_SUCCESS : EXIT_FAILURE; > +} > + > +#define EXPECTED_SIGNAL (_get_ssp () == 0 ? 0 : SIGSEGV) > +#include I'm surprised EXPECTED_SIGNAL works here. I would expect that the original test process would have to wait using xwaitpid and check for the signal in the subprocess. I think it would also be good to add a check that the subprocess actually returned from vfork without crashing, say using a pipe and a write before the return statement. > diff --git a/sysdeps/unix/sysv/linux/x86_64/vfork.S b/sysdeps/unix/sysv/linux/x86_64/vfork.S > index 776d2fc610..5dd5cb714c 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/vfork.S > +++ b/sysdeps/unix/sysv/linux/x86_64/vfork.S > @@ -71,6 +71,12 @@ ENTRY (__vfork) > /* Normal return if shadow stack isn't in use. */ > je L(no_shstk) > > + testl %eax, %eax > + jnz 2f > + /* NB: Jump back to caller directly with mismatched shadow stack > + to prevent child return. */ > + jmp *%rdi > +2: > /* Pop return address from shadow stack and jump back to caller > directly. */ > movl $1, %esi See above.