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-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-3.7 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, PDS_BAD_THREAD_QP_64,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (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 40DB01F5AE for ; Thu, 22 Apr 2021 22:04:16 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 479D83847804; Thu, 22 Apr 2021 22:04:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 479D83847804 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1619129054; bh=B3A9gMtm/0+JGWlDMEHXzzXP5jhwumGeCA7FwWPcIa0=; h=To:Subject:Date:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=Lc8xFqxREerIxN1ZnPFDzF/tzTdRjqhmLW9j6mZcrFN1AYjTHYCRkStoZaE73FK6Y /rud7QbxuyTUmFOUWJqWyZ3/A5h961Wcz7OsWw4/qeKTbVzeeRcKfLOdsQ1x/7qwRP Gf/pQBEbQTi3+P8EyNc38ocum8njwtBPH/crqxBo= Received: from eu-smtp-delivery-151.mimecast.com (eu-smtp-delivery-151.mimecast.com [185.58.85.151]) by sourceware.org (Postfix) with ESMTP id 4D5F53850400 for ; Thu, 22 Apr 2021 22:04:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4D5F53850400 Received: from AcuMS.aculab.com (156.67.243.121 [156.67.243.121]) (Using TLS) by relay.mimecast.com with ESMTP id uk-mta-256-gvvl59V8ObOmYuCwWGLDrg-1; Thu, 22 Apr 2021 23:04:08 +0100 X-MC-Unique: gvvl59V8ObOmYuCwWGLDrg-1 Received: from AcuMS.Aculab.com (fd9f:af1c:a25b:0:994c:f5c2:35d6:9b65) by AcuMS.aculab.com (fd9f:af1c:a25b:0:994c:f5c2:35d6:9b65) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Thu, 22 Apr 2021 23:04:07 +0100 Received: from AcuMS.Aculab.com ([fe80::994c:f5c2:35d6:9b65]) by AcuMS.aculab.com ([fe80::994c:f5c2:35d6:9b65%12]) with mapi id 15.00.1497.015; Thu, 22 Apr 2021 23:04:07 +0100 To: "'Bae, Chang Seok'" Subject: RE: [PATCH v8 5/6] x86/signal: Detect and prevent an alternate signal stack overflow Thread-Topic: [PATCH v8 5/6] x86/signal: Detect and prevent an alternate signal stack overflow Thread-Index: AQHXNzOfB++Ln2WD/U+jOvjJUzWT2qrAORRggABxogCAAGjMcA== Date: Thu, 22 Apr 2021 22:04:07 +0000 Message-ID: <1955da4c211f4d4fbbf74a6b8bdae0f6@AcuMS.aculab.com> References: <20210422044856.27250-1-chang.seok.bae@intel.com> <20210422044856.27250-6-chang.seok.bae@intel.com> <854d6aefdf604b559e37e82669b5e67f@AcuMS.aculab.com> <9C452E66-0C41-462B-9971-56825444AD65@intel.com> In-Reply-To: <9C452E66-0C41-462B-9971-56825444AD65@intel.com> Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.202.205.107] MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: aculab.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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: David Laight via Libc-alpha Reply-To: David Laight Cc: "linux-arch@vger.kernel.org" , "Brown, Len" , "Luck, Tony" , "libc-alpha@sourceware.org" , "jannh@google.com" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , "Dave.Martin@arm.com" , "Hansen, Dave" , "luto@kernel.org" , "linux-api@vger.kernel.org" , "tglx@linutronix.de" , "bp@suse.de" , "mingo@kernel.org" , "Shankar, Ravi V" Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" From: Bae, Chang Seok > Sent: 22 April 2021 17:31 >=20 > On Apr 22, 2021, at 01:46, David Laight wrote: > > From: Chang S. Bae > >> Sent: 22 April 2021 05:49 > >> > >> The kernel pushes context on to the userspace stack to prepare for the > >> user's signal handler. When the user has supplied an alternate signal > >> stack, via sigaltstack(2), it is easy for the kernel to verify that th= e > >> stack size is sufficient for the current hardware context. > >> > >> Check if writing the hardware context to the alternate stack will exce= ed > >> it's size. If yes, then instead of corrupting user-data and proceeding= with > >> the original signal handler, an immediate SIGSEGV signal is delivered. > > > > What happens if SIGSEGV is caught? >=20 > Boris pointed out the relevant notes before [1]. I think "unpredictable > results" is a somewhat vague statement but process termination is unavoid= able > in this situation. >=20 > In the thread [1], a new signal number was discussed for the signal deliv= ery > failure, but my takeaway is this SIGSEGV is still recognizable. >=20 > FWIW, Len summarized other possible approaches as well [2]. Let's see... I use an on-stack buffer for the alternate stack and then setup siglongjmp() to return back from whatever ran out of stack space back to the processing loop. So my attempts to trap over-deep recursion cause the main stack to get corrupted - this is a normal stack overwrite that might be exploitable! Alternatively I used malloc() and we have a potentially exploitable heap overrun. The only thing the kernel can do is to immediately kill the process (possibly with a core dump). Since signals can get nested the kernel needs to ensure there is a reasonably amount of space left after the signal info is written to the alternate stack. >=20 > >> Refactor the stack pointer check code from on_sig_stack() and use the = new > >> helper. > >> > >> While the kernel allows new source code to discover and use a sufficie= nt > >> alternate signal stack size, this check is still necessary to protect > >> binaries with insufficient alternate signal stack size from data > >> corruption. > > ... > >> diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal= .h > >> index 3f6a0fcaa10c..ae60f838ebb9 100644 > >> --- a/include/linux/sched/signal.h > >> +++ b/include/linux/sched/signal.h > >> @@ -537,6 +537,17 @@ static inline int kill_cad_pid(int sig, int priv) > >> #define SEND_SIG_NOINFO ((struct kernel_siginfo *) 0) > >> #define SEND_SIG_PRIV=09((struct kernel_siginfo *) 1) > >> > >> +static inline int __on_sig_stack(unsigned long sp) > >> +{ > >> +#ifdef CONFIG_STACK_GROWSUP > >> +=09return sp >=3D current->sas_ss_sp && > >> +=09=09sp - current->sas_ss_sp < current->sas_ss_size; > >> +#else > >> +=09return sp > current->sas_ss_sp && > >> +=09=09sp - current->sas_ss_sp <=3D current->sas_ss_size; > >> +#endif > >> +} > >> + > > > > Those don't look different enough. >=20 > The difference is on the SS_AUTODISARM flag check. This refactoring was > suggested as on_sig_stack() brought confusion [3]. I was just confused by the #ifdef. Whether %sp points to the last item or the next space is actually independent of the stack direction. A stack might usually use pre-decrement and post-increment but it doesn't have to. The stack pointer can't be right at one end of the alt-stack area (because that is the address you'd use when you switch to it), and if you are any where near the other end you are hosed. So a common test: =09return (unsigned long)(sp - current->sas_ss_sp) < current->sas_ss_size; will always work. It isn't as though the stack pointer should be anywhere else other than the 'real' thread stack. =09David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1= PT, UK Registration No: 1397386 (Wales)