|
Message-ID: <1561653844.5154.87.camel@lca.pw> Date: Thu, 27 Jun 2019 12:44:04 -0400 From: Qian Cai <cai@....pw> To: Kees Cook <keescook@...omium.org> Cc: Alexander Potapenko <glider@...gle.com>, Andrew Morton <akpm@...ux-foundation.org>, Christoph Lameter <cl@...ux.com>, Masahiro Yamada <yamada.masahiro@...ionext.com>, Michal Hocko <mhocko@...nel.org>, James Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>, Nick Desaulniers <ndesaulniers@...gle.com>, Kostya Serebryany <kcc@...gle.com>, Dmitry Vyukov <dvyukov@...gle.com>, Sandeep Patil <sspatil@...roid.com>, Laura Abbott <labbott@...hat.com>, Randy Dunlap <rdunlap@...radead.org>, Jann Horn <jannh@...gle.com>, Mark Rutland <mark.rutland@....com>, Marco Elver <elver@...gle.com>, linux-mm@...ck.org, linux-security-module@...r.kernel.org, kernel-hardening@...ts.openwall.com Subject: Re: [PATCH v9 1/2] mm: security: introduce init_on_alloc=1 and init_on_free=1 boot options 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().
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.