Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 31 Aug 2016 10:44:28 -0400
From: Kees Cook <>
To: Mark Rutland <>
Cc: Julia Lawall <>, Joe Perches <>, 
	Fengguang Wu <>, LKML <>, 
	"" <>, 
	Glenn Wurster <>, PaX Team <>
Subject: Re: constification and cocci / kernel build test robot ?

On Wed, Aug 31, 2016 at 6:08 AM, Mark Rutland <> wrote:
> On Tue, Aug 30, 2016 at 02:50:36PM -0400, Kees Cook wrote:
>> The structures that should get the greatest level of attention are
>> those that contain function pointers. The "constify" gcc plugin from
>> PaX/Grsecurity does this, but it uses a big hammer: it moves all of
>> them const even if they receive assignment. To handle this, there is
>> the concept of an open/close method to gain temporary access to the
>> structure. For example:
>> drivers/cdrom/cdrom.c:
>> int register_cdrom(...) {
>>         ...
>>         if (!cdo->generic_packet) {
>>                 pax_open_kernel();
>>                 const_cast(cdo->generic_packet) = cdrom_dummy_generic_packet;
>>                 pax_close_kernel();
>>         }
>> (The "const_cast" here is just a macro to convince gcc it's only to
>> write to a const value, so really it should maybe be called
>> "unconst_cast", but whatever...)
> Just to check, are they actually marked as const, or some pseudo-const
> like __ro_after_init?

The plugin marks them actually const, so the const_cast() is needed to
"forget" that marking and treat it as a normal variable. (And PaX Team
noted that this is the name used in C++ already.)

> I can imagine a number of issues with casting a real const away (e.g. if
> the compiler decides to cache the value in a register and decides since
> it is const it need not hazard with a memory clobber).

AFAIK, this hasn't caused problems. PaX Team may be able to speak more
to this...

>> This allows all of struct cdrom_device_ops to be const, even if they
>> need to be updated once during registration.
>> (This is a stronger version of __ro_after_init, which is for things
>> that are only written during __init.)
>> AUIU, the goals of the open/close_kernel idea are:
>> - always inline
>> - make sure the CPU cannot be interrupted
> I guess s/be interrupted/take an exception/, to cover stuff like debug
> breakpoints and such.

PaX team corrected me: it's not interrupt blocking but rather than it
shouldn't be preempted.

> That might not always be possible, and might not strictly be a
> requirement, if you can guarantee that taking an exception will cause
> the mapping to become non-writeable during the handler.
> That's one approach I plan to look into for arm64, assuming that we
> create a temporarily RW alias in TTBR0.
>> - BUG if memory is already writable
>> - make the memory writable only by the current CPU
>> - update the value
>> - restore memory permissions
>> - allow CPU interruption again
>> This makes sure there aren't races with other CPUs to write things,
>> and that it's harder to use for an attack since with the "make
>> writable" code is always followed by a "make read-only" action (i.e.
>> not separate functions that could be used as a trivial ROP gadget).
> FWIW, on that front we may want to look into reworking the fixmap code.
> For at least arm, arm64, microblaze, and powerpc, __set_fixmap is a C
> function that might offer a trivial ROP gadget for creating a RW
> mapping.
> Other architectures have that as a static inline in a header, which
> ensures that it's folded into callers, making it less trivial to reuse.

Even with the inlining, it's just making things slightly harder, since
there is still a memory write happening, so a careful setup before
calling it can still be used as a ROP gadget.


Kees Cook
Nexus 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.