From mboxrd@z Thu Jan 1 00:00:00 1970 Sender: Ingo Molnar Date: Fri, 6 Jul 2018 00:20:06 +0200 From: Ingo Molnar Subject: Re: [PATCH v13 (resend) 2/6] x86/entry: Add STACKLEAK erasing the kernel stack at the end of syscalls Message-ID: <20180705222006.GA5410@gmail.com> References: <1529686717-16017-1-git-send-email-alex.popov@linux.com> <1529686717-16017-3-git-send-email-alex.popov@linux.com> <20180705081236.GB20903@gmail.com> <45db2e83-5b87-0083-924f-f1ce7c89fe73@linux.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <45db2e83-5b87-0083-924f-f1ce7c89fe73@linux.com> To: Alexander Popov Cc: kernel-hardening@lists.openwall.com, Kees Cook , PaX Team , Brad Spengler , Andy Lutomirski , Tycho Andersen , Laura Abbott , Mark Rutland , Ard Biesheuvel , 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 , Linus Torvalds , Greg Kroah-Hartman , Dan Williams , Dave Hansen , Mathias Krause , Vikas Shivappa , Kyle Huey , Dmitry Safonov , Will Deacon , Arnd Bergmann , Florian Weimer , Boris Lukashev , Andrey Konovalov , x86@kernel.org, linux-kernel@vger.kernel.org List-ID: * Alexander Popov wrote: > Hello Ingo, > > Thanks for your review! I'll fix the style issues you pointed at. > > Please also see my answers below. > > On 05.07.2018 11:12, Ingo Molnar wrote: > >> + The tradeoff is the performance impact: on a single CPU system kernel > >> + compilation sees a 1% slowdown, other systems and workloads may vary > >> + and you are advised to test this feature on your expected workload > >> + before deploying it. > > > > Is there a way to patch this out runtime? I.e. if a distro enabled it, is there an > > easy way to disable much of the overhead without rebooting the kernel? > > Hm. We can't completely disable STACKLEAK in runtime, since STACKLEAK gcc plugin > performs compile-time instrumentation of the kernel code. So we can only chop > off a part of functionality, for example, by introducing some variable and > checking it before every stack erasing (additional performance impact), but the > kernel will stay uselessly instrumented. It doesn't look reasonable to me. Or we could use what every other performance critical instrumentation feature uses to reduce overhead (ftrace, perf): kernel patching. > > If so then please make this: > > > > if (WARN_ON(boundary - kstack_ptr >= THREAD_SIZE)) > > return; > > > > or so, to make it non-fatal and to allow users to report it, should it trigger > > against all expectations. > > I've made an experiment. The results: > 1. BUG_ON() here doesn't freeze the kernel output - I see a full 'PANIC: double > fault' report; Only in text mode - very few users are using text mode. > 2. WARN_ON() here gives absolutely same 'PANIC: double fault' here. that should only happen if the kernel is otherwise already fatally corrupted, right? Thanks, Ingo