From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x242.google.com (mail-lj1-x242.google.com [IPv6:2a00:1450:4864:20::242]) by sourceware.org (Postfix) with ESMTPS id A216B3857C7A for ; Fri, 20 Nov 2020 23:05:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A216B3857C7A Received: by mail-lj1-x242.google.com with SMTP id x9so11729737ljc.7 for ; Fri, 20 Nov 2020 15:05:06 -0800 (PST) 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:cc; bh=WsItiRlY2ZsY1kLHaN9bW+mg6xUUzDuK1HmeXy3qoOM=; b=LOb8JwIxyxpou7T69R4iLoEBJYcsOFpYvQoYWVLPtqpoioUV6bCF+Yac8f5epRR5j4 6dOFnygHKwZakrm8yennLadtnJqCt7MUo+JhHd5E3dbMbeXy83NN9oCL2nH6ZE1QVu4j 0VO/tnyd95BzdbX+jDr2fLiwMLN9hZ9PKTB11Lr84cjp8Nd807knacP6c+0NttepZTJo PDbKxB3ipWTsv8oSrKHEvCsXGuvdaw94XG5BywzbvUzMYrVIsHjlCLqiaRf3GfVfMQmp HRuPpos0QX5j1ENvaKGYAVuQvGirRvWldfiECZxExhzerUPMMs7CQzHLhmTLWxG+Kz8D ZA4A== X-Gm-Message-State: AOAM530/1T9uiY+eRyLKUgOPUKXgzBcD9DTmrzpmNSD0EA/nyE3CFJKQ BgiVjcOdSwKbIfSjNk49YVLKu3n9ZXxVBPFk3hDDdg== X-Google-Smtp-Source: ABdhPJymlpDCBYMRse6yxmhBac3FkYvw4MIKvsrIx7Zv7EbYRWZkOmL5ixX8POB9rfwViL1vOrupv+a/zLsoSj3iJio= X-Received: by 2002:a2e:8891:: with SMTP id k17mr8457473lji.326.1605913504291; Fri, 20 Nov 2020 15:05:04 -0800 (PST) MIME-Version: 1.0 References: <20201119190237.626-1-chang.seok.bae@intel.com> <20201119190237.626-4-chang.seok.bae@intel.com> In-Reply-To: <20201119190237.626-4-chang.seok.bae@intel.com> From: Jann Horn Date: Sat, 21 Nov 2020 00:04:38 +0100 Message-ID: Subject: Re: [PATCH v2 3/4] x86/signal: Prevent an alternate stack overflow before a signal delivery To: "Chang S. Bae" Cc: Thomas Gleixner , Ingo Molnar , Borislav Petkov , Andy Lutomirski , "the arch/x86 maintainers" , Len Brown , Dave Hansen , "H.J. Lu" , Dave Martin , Michael Ellerman , Tony Luck , "Ravi V. Shankar" , libc-alpha@sourceware.org, linux-arch , Linux API , kernel list , Hiroshi Shimamoto , Roland McGrath Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-24.7 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: , X-List-Received-Date: Fri, 20 Nov 2020 23:05:08 -0000 On Thu, Nov 19, 2020 at 8:40 PM Chang S. Bae wrote: > The kernel pushes data on the userspace stack when entering a signal. If > using a sigaltstack(), the kernel precisely knows the user stack size. > > When the kernel knows that the user stack is too small, avoid the overflow > and do an immediate SIGSEGV instead. > > This overflow is known to occur on systems with large XSAVE state. The > effort to increase the size typically used for altstacks reduces the > frequency of these overflows, but this approach is still useful for legacy > binaries. > > Here the kernel expects a bit conservative stack size (for 64-bit apps). > Legacy binaries used a too-small sigaltstack would be already overflowed > before this change, if they run on modern hardware. [...] > diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c > index ee6f1ceaa7a2..cee41d684dc2 100644 > --- a/arch/x86/kernel/signal.c > +++ b/arch/x86/kernel/signal.c > @@ -251,8 +251,13 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size, > > /* This is the X/Open sanctioned signal stack switching. */ > if (ka->sa.sa_flags & SA_ONSTACK) { > - if (sas_ss_flags(sp) == 0) > + if (sas_ss_flags(sp) == 0) { > + /* If the altstack might overflow, die with SIGSEGV: */ > + if (!altstack_size_ok(current)) > + return (void __user *)-1L; > + > sp = current->sas_ss_sp + current->sas_ss_size; > + } A couple lines further down, we have this (since commit 14fc9fbc700d): /* * If we are on the alternate signal stack and would overflow it, don't. * Return an always-bogus address instead so we will die with SIGSEGV. */ if (onsigstack && !likely(on_sig_stack(sp))) return (void __user *)-1L; Is that not working? (It won't handle the case where the kernel fills up almost all of the alternate stack, and the userspace signal handler then overflows out of the alternate signal stack. But there isn't much the kernel can do about that...)