[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 14 Feb 2017 16:19:03 +0000
From: Mark Rutland <mark.rutland@....com>
To: Kaiwan N Billimoria <kaiwan@...wantech.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
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.
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.
