Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 21 Feb 2017 15:26:03 -0800
From: Kees Cook <keescook@...omium.org>
To: Kaiwan N Billimoria <kaiwan@...wantech.com>
Cc: Christoph Lameter <cl@...ux.com>, Laura Abbott <labbott@...hat.com>, 
	"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: Merge in PAX_MEMORY_SANITIZE work from grsec
 to linux-next

On Thu, Feb 16, 2017 at 7:18 PM, Kaiwan N Billimoria
<kaiwan@...wantech.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

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.