Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751923AbbKYR0u (ORCPT ); Wed, 25 Nov 2015 12:26:50 -0500 Received: from mail-ig0-f176.google.com ([209.85.213.176]:36558 "EHLO mail-ig0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751700AbbKYR0X (ORCPT ); Wed, 25 Nov 2015 12:26:23 -0500 MIME-Version: 1.0 In-Reply-To: References: <1448401114-24650-1-git-send-email-keescook@chromium.org> Date: Wed, 25 Nov 2015 09:26:22 -0800 X-Google-Sender-Auth: XUuASypgJiNetpIAQu5axVHKFhw Message-ID: Subject: Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory From: Kees Cook To: Mathias Krause Cc: "kernel-hardening@lists.openwall.com" , "linux-kernel@vger.kernel.org" , Andy Lutomirski , Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , x86-ml , Arnd Bergmann , Michael Ellerman , linux-arch , PaX Team , Emese Revfy Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2951 Lines: 60 On Wed, Nov 25, 2015 at 1:13 AM, Mathias Krause wrote: > On 24 November 2015 at 22:38, Kees Cook wrote: >> Many things are written to only during __init, and never changed >> again. These cannot be made "const" since the compiler will do the wrong >> thing (we do actually need to write to them). Instead, move these items >> into a memory region that will be made read-only during mark_rodata_ro() >> which happens after all kernel __init code has finished. >> >> This introduces __read_only as a way to mark such memory, and uses it on >> the x86 vDSO to kill an extant kernel exploitation method. > > ...just some random notes on the experience with kernels implementing > such a feature for quite a lot of locations, not just the vDSO. > > While having that annotation makes perfect sense, not only from a > security perspective but also from a micro-optimization point of view > (much like the already existing __read_mostly annotation), it has its > drawbacks. Violating the "r/o after init" rule by writing to such > annotated variables from non-init code goes unnoticed as far as it > concerns the toolchain. Neither the compiler nor the linker will flag > that incorrect use. It'll just trap at runtime and that's bad. Well, or, it's good: that's the point of it. Either from the perspective of robustness or from that of security. > I myself had some educating experience seeing my machine triple fault > when resuming from a S3 sleep. The root cause was a variable that was > annotated __read_only but that was (unnecessarily) modified during CPU > bring-up phase. Debugging that kind of problems is sort of a PITA, you > could imagine. As PaX Team mentions, it should be easy to catch these traps and report them. That could certainly be a nice addition. > So, prior extending the usage of the __read_only annotation some > toolchain support is needed. Maybe a gcc plugin that'll warn/error on > code that writes to such a variable but is not __init itself. The > initify and checker plugins from the PaX patch might be worth to look > at for that purpose, as they're doing similar things already. Adding > such a check to sparse might be worth it, too. > A modpost check probably won't work as it's unable to tell if it's a > legitimate access (r/o) or a violation (/w access). So the gcc plugin > is the way to go, IMHO. There are many more pieces to add to the annotation use, but I don't want to risk getting us into a Catch-22. Emese's work on the plugin side will see this usage and utility grow, and I think getting these basic building blocks in place is the right place to start. -Kees -- Kees Cook Chrome OS & Brillo Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/