Re: [kernel-hardening] Merge in PAX_MEMORY_SANITIZE work from grsec to linux-next

Kees Cook
2017-02-21T23:26:03Z

On Thu, Feb 16, 2017 at 7:18 PM, Kaiwan N Billimoria
<kaiwan@kaiwantech.com> wrote:
>> Hm, we don't want to force this on against other command line
>> settings, so how about what I suggest below...
>>
>
> Ok, can see the logic in that..
>
>>
>> Can this be changed to interact better with the command line
>> arguments, in case someone selects CONFIG_MEMORY_SANITIZE, but boots
>> with page_poison=off?
>>
>> e.g. instead of your init code, do this:
>>
>> static bool want_page_poisoning __read_mostly =
>> IS_ENABLED(CONFIG_MEMORY_SANITIZE);
>>
>> That way the logic for __page_poisoning_enabled is retained, etc.
>>
>>> +
>>> static inline void set_page_poison(struct page *page)
>>> {
>>> struct page_ext *page_ext;
>>> diff --git a/mm/slub.c b/mm/slub.c
>>> index d24e1ce..62de543 100644
>>> --- a/mm/slub.c
>>> +++ b/mm/slub.c
>>> @@ -457,6 +457,17 @@ static int slub_debug;
>>> static char *slub_debug_slabs;
>>> static int disable_higher_order_debug;
>>>
>>> +static int __init memory_sanitize_slubdebug_init(void)
>>> +{
>>> +/* With 'memory sanitize' On, slub_debug Must be set to 'p' */
>>> + if (IS_ENABLED(CONFIG_SLUB_DEBUG_ON) &&
>>> + IS_ENABLED(CONFIG_MEMORY_SANITIZE)) {
>>> + slub_debug |= SLAB_POISON;
>>> + }
>>> + return 0;
>>> +}
>>> +early_initcall(memory_sanitize_slubdebug_init);
>>
>> And instead of this, use:
>>
>> #if defined(CONFIG_SLUB_DEBUG_ON)
>> # define SLUB_DEBUG_INITIAL_FLAGS DEBUG_DEFAULT_FLAGS
>> #elif defined(CONFIG_MEMORY_SANITIZE)
>> # define SLUB_DEBUG_INITIAL_FLAGS SLAB_POISON
>> #else
>> # define SLUB_DEBUG_INITIAL_FLAGS 0
>> #endif
>>
>> static int slub_debug = SLUB_DEBUG_INITIAL_FLAGS;
>>
>> That way we're just setting the boot-time defaults and it'll safely
>> interact with everything else.
>>
>> (I've added Christoph to CC to see what he thinks of this.)
>>
>
> What about a kernel command-line param - called 'mem_sanitize'?
> If set to 'on', it will enforce the forced behaviour (as per the curr
> implementation).
>
> I can think of this (but...):
> --------------------+--------------------------+---------------------------
> mem_sanitize | page_poisoning | slub_debug
> --------------------+--------------------------+---------------------------
> strict | on | |= SLAB_POISON
> normal | <current default>
> off | off | off
> --------------------+--------------------------+---------------------------
>
> But... if you think about it, I guess only the "strict" option is useful in any
> meaningful way.
> 'normal' is what we'll get when I re-implement the code
> as you suggested above.
> Do we even want an 'off' case? As again, we'd be poking into the page_poisoning
> and slub_debug values.
>
> Of course we can (and will) document all this...
> Thoughts?

I think mem_sanitize should likely follow the logic used by
pax_sanitize_slab. i.e CONFIG_MEMORIZE_SANITIZE as suggested above,
then a mem_sanitize= option for "off" and "full". (i.e.
CONFIG_MEMORIZE_SANITIZE implies "on")

-Kees

--
Kees Cook
Pixel Security