Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 15 Feb 2017 12:30:52 -0800
From: Kees Cook <keescook@...omium.org>
To: Kaiwan N Billimoria <kaiwan@...wantech.com>, Christoph Lameter <cl@...ux.com>
Cc: 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 Tue, Feb 14, 2017 at 11:27 PM, Kaiwan N Billimoria
<kaiwan@...wantech.com> wrote:
> Okay, I've incorporated your suggestions.
> Of course, the printk's are temporary.
>
> Pl see:
> - the updated patch, and
> - a 'truth table' enumerating some basic testing with these configs,
> below:
>
> ---
> diff --git a/init/main.c b/init/main.c
> index ef47035..ba44574 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1028,6 +1028,11 @@ static noinline void __init kernel_init_freeable(void)
>
>         do_basic_setup();
>
> +#ifdef CONFIG_MEMORY_SANITIZE
> +       pr_debug("[CONFIG_MEMORY_SANITIZE]: page_poisoning_enabled? %s\n",
> +               page_poisoning_enabled() ? "yes" : "no");
> +#endif
> +
>         /* Open the /dev/console on the rootfs, this should never fail */
>         if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
>                 pr_err("Warning: unable to open an initial console.\n");
> diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
> index 3e5eada..fbb4290 100644
> --- a/mm/Kconfig.debug
> +++ b/mm/Kconfig.debug
> @@ -97,3 +97,24 @@ config DEBUG_RODATA_TEST
>      ---help---
>        This option enables a testcase for the setting rodata read-only.
>
> +config MEMORY_SANITIZE
> +    bool "Enable memory sanitization features"
> +    select SLUB_DEBUG
> +    select PAGE_POISONING
> +    select PAGE_POISONING_NO_SANITY
> +    ---help---
> +       This option enables memory sanitization features. Particularly,
> +       when you turn on this option, it auto-enables:
> +       - SLUB debug
> +       - page poisoning
> +       - page poisoning no sanity.
> +
> +       Implication: turning this option on _will_ implicitly enable:
> +       - the SLUB_DEBUG switch to the equivalent of the kernel command-line
> +         'slub_debug=p' ; (where p=PAGE_POISON),
> +       - page poisoning, equivalent to passing the kernel command-line option
> +         'page_poison=on',
> +       irrespective of whether the options are explicitly passed or not.

Hm, we don't want to force this on against other command line
settings, so how about what I suggest below...

> +
> +       If unsure, say N.
> +
> diff --git a/mm/page_poison.c b/mm/page_poison.c
> index 2e647c6..8d1e883 100644
> --- a/mm/page_poison.c
> +++ b/mm/page_poison.c
> @@ -49,6 +49,17 @@ struct page_ext_operations page_poisoning_ops = {
>         .init = init_page_poisoning,
>  };
>
> +static int __init memory_sanitize_pagepoison_init(void)
> +{
> +       /* With 'memory sanitize' On, page poisoning Must be turned on */
> +       if (IS_ENABLED(CONFIG_MEMORY_SANITIZE)) {
> +               want_page_poisoning = true;
> +               __page_poisoning_enabled = true;
> +       }
> +       return 0;
> +}
> +early_initcall(memory_sanitize_pagepoison_init);

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.)

-Kees

-- 
Kees Cook
Pixel Security

Powered by blists - more mailing lists

Your e-mail address:

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