[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXu5jLXukRLg=0i00fYjZpaVj1hzcyf81aKa6mN4LNPnsfWNw@mail.gmail.com>
Date: Wed, 25 Nov 2015 09:26:22 -0800
From: Kees Cook <keescook@...omium.org>
To: Mathias Krause <minipli@...glemail.com>
Cc: "kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Andy Lutomirski <luto@...capital.net>,
Ingo Molnar <mingo@...hat.com>, Thomas Gleixner <tglx@...utronix.de>, "H. Peter Anvin" <hpa@...or.com>,
x86-ml <x86@...nel.org>, Arnd Bergmann <arnd@...db.de>, Michael Ellerman <mpe@...erman.id.au>,
linux-arch <linux-arch@...r.kernel.org>, PaX Team <pageexec@...email.hu>,
Emese Revfy <re.emese@...il.com>
Subject: Re: [PATCH 0/2] introduce post-init read-only memory
On Wed, Nov 25, 2015 at 1:13 AM, Mathias Krause <minipli@...glemail.com> wrote:
> On 24 November 2015 at 22:38, Kees Cook <keescook@...omium.org> 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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
