On Thu, 2019-06-27 at 09:29 -0700, Kees Cook wrote:
> On Thu, Jun 27, 2019 at 09:25:11AM -0400, Qian Cai wrote:
> > On Thu, 2019-06-27 at 15:03 +0200, Alexander Potapenko wrote:
> > > +static int __init early_init_on_alloc(char *buf)
> > > +{
> > > + int ret;
> > > + bool bool_result;
> > > +
> > > + if (!buf)
> > > + return -EINVAL;
> > > + ret = kstrtobool(buf, &bool_result);
> > > + if (bool_result && IS_ENABLED(CONFIG_PAGE_POISONING))
> > > + pr_warn("mem auto-init: CONFIG_PAGE_POISONING is on, will
> > > take precedence over init_on_alloc\n");
> >
> > I don't like the warning here. It makes people think it is bug that need to
> > be
> > fixed, but actually it is just information. People could enable both in a
> > debug
> > kernel.
>
> How would you suggest it be adjusted? Should it be silent, or be
> switched to pr_info()?
pr_info() sounds more reasonable to me, so people don't need to guess the
correct behavior. Ideally, CONFIG_PAGE_POISONING should be renamed to something
like CONFIG_INIT_ON_FREE_CHECK, and it only does the checking part if enabled,
and init_on_free will gain an ability to poison a pattern other than 0.
Also, there might be some rooms to consolidate with SLAB_POSION as well.
>
> Also, doesn't this need to check "want_page_poisoning", not just
> CONFIG_PAGE_POISONING? Perhaps just leave the warning out entirely?
>
Yes, only checking CONFIG_PAGE_POISONING is not enough, and need to check
page_poisoning_enabled().
