On Tue, 30 Aug 2016, Kees Cook wrote:
> On Sun, Aug 28, 2016 at 9:13 AM, Julia Lawall <julia.law...@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
>
