Re: constification and cocci / kernel build test robot ?

[Julia Lawall <julia.lawall@lip6.fr>]

On Tue, 30 Aug 2016, Kees Cook wrote:

> On Sun, Aug 28, 2016 at 9:13 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > [Adding Kees, in case it's of interest]
> >
> > Below is the list of types of top-level initialized structures and the
> > number that are const.  For quicker reading, here are some that are
> > sometimes const (numerator), but not always (denominator):
> >
> > file_operations: 2221/2233
> > attribute_group: 447/919
> > irq_chip: 1/518
> > net_device_ops: 488/498
> > regmap_config: 407/447
> > dev_pm_ops: 398/415
> > clk_ops: 314/386
> > resource: 6/385
> > seq_operations: 327/328
> > snd_pcm_ops: 9/288
> >
> > and here are the most used ones that are never const at all:
> >
> > platform_driver: 2943
> > platform_device: 2226
> > clk_branch: 1131
> > i2c_driver: 786
> > pci_driver: 781
> > omap_hwmod_ocp_if: 670
> > omap_hwmod: 582
> > notifier_block: 556
> > clk: 473
> > clk_rcg2: 384
> >
> > [...]
>
> 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();

Thanks for the clarification.  The above has to be added to the code
manually, or the plugin does it?

julia

>         }
>
> (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...)
>
> 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
> - 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).
>
> -Kees
>
> --
> Kees Cook
> Nexus Security
>