[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPDLWs-m4ESK-5p4bKW7g9xo=D+9ZSkW-YcsgQd5QGdxwit-FQ@mail.gmail.com>
Date: Tue, 14 Feb 2017 16:33:31 +0000
From: Kaiwan N Billimoria <kaiwan@...wantech.com>
To: Mark Rutland <mark.rutland@....com>
Cc: Kees Cook <keescook@...omium.org>, Laura Abbott <labbott@...hat.com>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, nd@....com
Subject: Re: Merge in PAX_MEMORY_SANITIZE work from grsec
to linux-next
Thanks for your succinct explanation Mark! Makes sense.
Will incorporate it, as Kees suggests..
On Tue, 14 Feb 2017, 9:49 pm Mark Rutland, <mark.rutland@....com> wrote:
> On Tue, Feb 14, 2017 at 01:34:38PM +0530, Kaiwan N Billimoria wrote:
> > >> diff --git a/mm/page_poison.c b/mm/page_poison.c
> > >> index 2e647c6..b45bc0a 100644
> > >> --- a/mm/page_poison.c
> > >> +++ b/mm/page_poison.c
> > >> @@ -49,6 +49,19 @@ struct page_ext_operations page_poisoning_ops = {
> > >> .init = init_page_poisoning,
> > >> };
> > >>
> > >> +#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.
> >
> > An academic question perhaps- am not clear as to why the IS_ENABLED()
> > is preferable to an #ifdef. I thought eliminating a runtime 'if'
> > condition (by using an ifdef) is more optimal than the "if
> > IS_ENABLED(foo)"? Or (probably) does the compiler optimize it out
> > regardless (assuming that CONFIG_MEMORY_SANITIZE is not selected of
> > course)?
>
> We expect the compiler to optimize the code out, since IS_ENABLED(x) is
> a compile-time constant.
>
> As for why it's preferable, it has several benefits.
>
> For one thing, using if (IS_ENABLED(x)) means that we always get build
> coverage of the code predicated on the config option, so it's harder to
> accidentally break some build configurations.
>
> For another, it composes more nicely with other conditionals, which is
> useful when you have a condition that depends on several config options,
> or config options and runtime expressions.
>
> e.g. something like:
>
> if (IS_ENABLED(FOO) && IS_ENABLED(BAR) && runtime_option)
> do_combination_thing();
> else if (IS_ENABLED(BAR))
> do_bar_only_thing();
> else
> do_other_thing();
>
> ... is much more painful to write using ifdefs.
>
> Generally, it's nicer for people to deal with than ifdeffery. It's
> easier for people to match braces in well-formatted code than it is to
> match #ifdef #endif pairs.
>
> Thanks,
> Mark.
>
>
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.
