From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: ARC-Seal: i=1; a=rsa-sha256; t=1520370083; cv=none; d=google.com; s=arc-20160816; b=boq2KZwIcuaxZ9+QuZ/sI2OAlgTy1agtJC3YYTk3f+Wkb4SaixUzb7qUGvJbKlt9NW xdUqr4xrVoge6IhGsxOpyVvBSmkDdmpOQPZC+994kcMBp7Vb3Tn6xMyq2V+tBv4DtUg2 rbhScmZ2NZmOMrE9SCfy8IJPbGlUeUHugvP+vswV5dySfMKJsfqtls1qC2PUqR/eGZy7 uxQ+iGuiarSdot4jwBFwj0JKONd+AawkedIBPQX2vIRwB5PR9hESbKr+8cNq0cqqOjZg IqbFjKGEA94CtwaKnFSHjZhdW+4if1GY98XMiULNf/X0K8Gq71dcZfxm+ibrCxSP8xd4 Kfhg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:subject:message-id:date:from:references:in-reply-to:sender :mime-version:dkim-signature:dkim-signature :arc-authentication-results; bh=y1n7Ytid9eafeUkQW/E/1MPiY9rzl2jjaa+dlH6fXqc=; b=sg48UdJu6Hu1PRpMT6kQ8bCl3EQOvFq1D/mfClBIT9I6iVdEBkjAvhjHzdmEDUOZXu /uO1eCCkQd42rsL7hNJ1tX/4gTIB9B7WJ1+kYWFMXw0g3kZTwOEmkjScnhvANIBYq/+a fA+/kW4egezwy3kavfn9WhIrPkaewLc8/nJEuGntNmVrXo0tx9XfAnfv4SalXzK8ZoL4 qa9whz4ZGVtWDYZIU1nUXDFV0nAP59lkSWaG99Fh2QNC4zyuFn+Pw/G/nQU+B481yjx7 lrjQkQB4pkeU32ZJ9fncn4c0mPZPoFkX5/Rw+mqjCKfOh+58POTSsFb0UoHltjQcoZvM M6bg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=JJkQXL4g; dkim=pass header.i=@linux-foundation.org header.s=google header.b=XDO5wyQd; spf=pass (google.com: domain of linus971@gmail.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=linus971@gmail.com Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=JJkQXL4g; dkim=pass header.i=@linux-foundation.org header.s=google header.b=XDO5wyQd; spf=pass (google.com: domain of linus971@gmail.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=linus971@gmail.com X-Google-Smtp-Source: AG47ELso97mzzMnHN6JgNBT1Vo7WFtXSWk7on5J7XgJRtn3NF0BRMWWvzRzY1QINBgl4zA2XxdLjCPmcXd49ervx1D8= MIME-Version: 1.0 Sender: linus971@gmail.com In-Reply-To: References: <1520107232-14111-1-git-send-email-alex.popov@linux.com> <1520107232-14111-5-git-send-email-alex.popov@linux.com> <20180306080855.phtgl2bzqm5hnthu@gmail.com> From: Linus Torvalds Date: Tue, 6 Mar 2018 13:01:20 -0800 X-Google-Sender-Auth: -lr1vGQYkO2FubYRwrSqGng5F_s Message-ID: Subject: Re: [PATCH RFC v9 4/7] x86/entry: Erase kernel stack in syscall_trace_enter() To: Arnd Bergmann Cc: Ard Biesheuvel , Daniel Micay , Ingo Molnar , Kees Cook , Dave Hansen , Alexander Popov , Kernel Hardening , PaX Team , Brad Spengler , Andy Lutomirski , Tycho Andersen , Laura Abbott , Mark Rutland , Borislav Petkov , Richard Sandiford , Thomas Gleixner , "H . Peter Anvin" , Peter Zijlstra , "Dmitry V . Levin" , Emese Revfy , Jonathan Corbet , Andrey Ryabinin , "Kirill A . Shutemov" , Thomas Garnier , Andrew Morton , Alexei Starovoitov , Josef Bacik , Masami Hiramatsu , Nicholas Piggin , Al Viro , "David S . Miller" , Ding Tianhong , David Woodhouse , Josh Poimboeuf , Steven Rostedt , Dominik Brodowski , Juergen Gross , Greg Kroah-Hartman , Dan Williams , Mathias Krause , Vikas Shivappa , Kyle Huey , Dmitry Safonov , Will Deacon , X86 ML , LKML Content-Type: text/plain; charset="UTF-8" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1593947986518331727?= X-GMAIL-MSGID: =?utf-8?q?1594223581053403035?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Tue, Mar 6, 2018 at 12:42 PM, Arnd Bergmann wrote: > > Forcing gcc to allocate a stack slot and zero-initialize it should > find many bugs by adding valid warnings, but also add lots of > false positives as well as prevent important optimizations in other > places that are actually well-defined. Oh, no, the "force gcc to allocate a stack slot" would be absolutely insane. You should never do that. Anybody who does that should be shot. But you don't have to force any stack allocation. You should just initialize it to zero (*without* the stack allocation). Then the optimization passes will just remove the initialization in 99.9% of all cases. Only very occasionally - when gcc cannot see it being overwritten - would it remain. And those are exactly the cases where you *want* it to remain. Of course, it is possible that you can't just do that from a plugin. But I can almost guarantee that it would be trivial to do as a gcc switch if any gcc people wanted to do it. Just look at each scalar auto variable, and if it doesn't have an initializer, just add one to zero completely mindlessly. As the crusaders said: "Kill them all and let the optimizer sort them out". (Note: the "trivial to do" is about scalar values only. Non-scalar values have more complexities, like struct padding issues etc). >> If somebody has big arrays on the stack, that's often a problem >> anyway. It may be common in non-kernel code, but kernel code is very >> special. > > I can think of a few cases that are important, this one is well-known: > > int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp, > fd_set __user *exp, struct timespec64 *end_time) > { > .... > /* Allocate small arguments on the stack to save memory and be faster */ > long stack_fds[SELECT_STACK_ALLOC/sizeof(long)]; Oh, I can well imagine things like this. But they will be a handful, they will be seen in profiles, and they'd be easy to mark. And then you can validate the ones you mark, instead of worrying about all the ones you don't even know about. > There is also the really scary code like: > > #define SKCIPHER_REQUEST_ON_STACK(name, tfm) \ > char __##name##_desc[sizeof(struct skcipher_request) + \ > crypto_skcipher_reqsize(tfm)] CRYPTO_MINALIGN_ATTR; \ > struct skcipher_request *name = (void *)__##name##_desc The crypto stuff does disgusting things. It used to do VLA's in structures too, that has morphed into using these crazy XYZ_ON_STACK() defines. They eventually need to fix their own crap at some point. It shouldn't be something we care about from a design standpoint. Linus