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?
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).
> 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.
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.
Thanks,
Mark.
