[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 14 Feb 2017 08:31:08 +0530
From: Kaiwan N Billimoria <kaiwan@...wantech.com>
To: Kees Cook <keescook@...omium.org>
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
Thanks for your response...
>
>
> +config MEMORY_SANITIZE
> > + bool "Enable memory sanitization features"
> > + select SLUB_DEBUG
> > + select PAGE_POISONING
> > + select PAGE_POISONING_NO_SANITY if HIBERNATION
> > + ---help---
> > + This option enables ...
>
> Good start! Why the "if HIBERNATION" bit? It seems like sanity checks
> are very expensive, so we'd not want it as part of this config?
>
> Okay, I wasn't sure. So would it be (more) correct to retain the first
two configs plus
PAGE_POISONING_NO_SANITY (without the if)?
>
> > +#ifdef CONFIG_MEMORY_SANITIZE
> > +static int __init memory_sanitize_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_init);
> > +#endif
>
> The ifdef and the IS_ENABLED seem redundant to me. I'd drop the ifdef.
>
> Agree on the redundancy, will do.
> > #if defined(CONFIG_SLUB_DEBUG_ON)
> > +#if defined(CONFIG_MEMORY_SANITIZE)
> > +/* With 'memory sanitize' On, slub_debug should be 'P' */
> > +static int slub_debug = SLAB_POISON;
> > +#else
> > static int slub_debug = DEBUG_DEFAULT_FLAGS;
> > +#endif /* CONFIG_MEMORY_SANITIZE */
> > #else
> > static int slub_debug;
> > -#endif
> > +#endif /* CONFIG_SLUB_DEBUG_ON */
>
> Could the definition of DEBUG_DEFAULT_FLAGS be adjusted instead of
> doing the ifdefs here in the .c file? Or, perhaps do a slub_debug |=
> SLAB_POISON in memory_sanitize_init()?
>
> Yes, the latter sounds good but the init function is in mm/page_poison.c
and the slub_debug var is a static in mm/slub.c . Any suggestions?
>
> > +#ifdef CONFIG_MEMORY_SANITIZE
> > + pr_info("[CONFIG_MEMORY_SANITIZE]: slub_debug = P? %s [0x%x]\n",
> > + slub_debug & SLAB_POISON ? "yes" : "no", slub_debug);
> > +#endif
> > +
> > mutex_lock(&slab_mutex);
> >
> > slab_kset = kset_create_and_add("slab", &slab_uevent_ops,
> kernel_kobj);
>
> Good first step to getting the CONFIG to do something meaningful, thanks!
>
> -Kees
>
> Thanks, happy to help!
- Kaiwan.
> --
> Kees Cook
> Pixel Security
>
>
Content of type "text/html" skipped
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.
