Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1473559

Re: constification and cocci / kernel build test robot ?

From Kees Cook <keescook@chromium.org>
Newsgroups linux.kernel
Subject Re: constification and cocci / kernel build test robot ?
Date 2016-08-31 16:50 +0200
Message-ID <sceZX-3Ew-9@gated-at.bofh.it> (permalink)
References <saQZH-87i-7@gated-at.bofh.it> <saRj3-8sH-1@gated-at.bofh.it> <sb8ae-2bS-5@gated-at.bofh.it> <sbWqm-b6-19@gated-at.bofh.it> <scaMF-17E-15@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Wed, Aug 31, 2016 at 6:08 AM, Mark Rutland <mark.rutland@arm.com> 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

-- 
Kees Cook
Nexus Security

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] thermal: fix of_table.cocci warnings Julia Lawall <julia.lawall@lip6.fr> - 2016-08-27 21:00 +0200
  constification and cocci / kernel build  test robot ? Joe Perches <joe@perches.com> - 2016-08-27 21:20 +0200
    Re: constification and cocci / kernel build test robot ? Julia Lawall <julia.lawall@lip6.fr> - 2016-08-28 11:50 +0200
    Re: constification and cocci / kernel build test robot ? Joe Perches <joe@perches.com> - 2016-08-28 19:40 +0200
      Re: constification and cocci / kernel build test robot ? Julia Lawall <julia.lawall@lip6.fr> - 2016-08-28 19:50 +0200
    Re: constification and cocci / kernel build test robot ? Kees Cook <keescook@chromium.org> - 2016-08-30 21:00 +0200
      Re: constification and cocci / kernel build test robot ? Julia Lawall <julia.lawall@lip6.fr> - 2016-08-30 21:30 +0200
        Re: constification and cocci / kernel build test robot ? Kees Cook <keescook@chromium.org> - 2016-08-31 00:20 +0200
          Re: constification and cocci / kernel build test robot ? Julia Lawall <julia.lawall@lip6.fr> - 2016-08-31 07:30 +0200
      Re: constification and cocci / kernel build test robot ? Mark Rutland <mark.rutland@arm.com> - 2016-08-31 12:20 +0200
        Re: constification and cocci / kernel build test robot ? Kees Cook <keescook@chromium.org> - 2016-08-31 16:50 +0200
          Re: constification and cocci / kernel build test robot ? Mark Rutland <mark.rutland@arm.com> - 2016-08-31 18:50 +0200
            Re: constification and cocci / kernel build test robot ? "PaX Team" <pageexec@freemail.hu> - 2016-09-01 17:00 +0200
  Re: [PATCH] thermal: fix of_table.cocci warnings Bin Gao <bin.gao@linux.intel.com> - 2016-08-29 19:00 +0200

csiph-web