diff mbox series

[v5,3/7] init_on_alloc: Unpessimize default-on builds

Message ID 20210309214301.678739-4-keescook@chromium.org (mailing list archive)
State New, archived
Headers show
Series Optionally randomize kernel stack offset each syscall | expand

Commit Message

Kees Cook March 9, 2021, 9:42 p.m. UTC 
Right now, the state of CONFIG_INIT_ON_ALLOC_DEFAULT_ON (and
...ON_FREE...) did not change the assembly ordering of the static branch
tests. Use the new jump_label macro to check CONFIG settings to default
to the "expected" state, unpessimizes the resulting assembly code.

Reviewed-by: Alexander Potapenko <glider@google.com>
Link: https://lore.kernel.org/lkml/CAG_fn=X0DVwqLaHJTO6Jw7TGcMSm77GKHinrd0m_6y0SzWOrFA@mail.gmail.com/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/mm.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Andrey Konovalov March 10, 2021, 12:52 p.m. UTC | #1 
On Tue, Mar 9, 2021 at 10:43 PM Kees Cook <keescook@chromium.org> wrote:
>
> Right now, the state of CONFIG_INIT_ON_ALLOC_DEFAULT_ON (and
> ...ON_FREE...) did not change the assembly ordering of the static branch
> tests. Use the new jump_label macro to check CONFIG settings to default
> to the "expected" state, unpessimizes the resulting assembly code.
>
> Reviewed-by: Alexander Potapenko <glider@google.com>
> Link: https://lore.kernel.org/lkml/CAG_fn=X0DVwqLaHJTO6Jw7TGcMSm77GKHinrd0m_6y0SzWOrFA@mail.gmail.com/
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/mm.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index bf341a9bfe46..2ccd856ac0d1 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2874,7 +2874,8 @@ static inline void kernel_unpoison_pages(struct page *page, int numpages) { }
>  DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_ALLOC_DEFAULT_ON, init_on_alloc);
>  static inline bool want_init_on_alloc(gfp_t flags)
>  {
> -       if (static_branch_unlikely(&init_on_alloc))
> +       if (static_branch_maybe(CONFIG_INIT_ON_ALLOC_DEFAULT_ON,
> +                               &init_on_alloc))
>                 return true;
>         return flags & __GFP_ZERO;
>  }
> @@ -2882,7 +2883,8 @@ static inline bool want_init_on_alloc(gfp_t flags)
>  DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_FREE_DEFAULT_ON, init_on_free);
>  static inline bool want_init_on_free(void)
>  {
> -       return static_branch_unlikely(&init_on_free);
> +       return static_branch_maybe(CONFIG_INIT_ON_FREE_DEFAULT_ON,
> +                                  &init_on_free);
>  }
>
>  extern bool _debug_pagealloc_enabled_early;

Should we also update slab_want_init_on_alloc() and slab_want_init_on_free()?
Kees Cook March 10, 2021, 9:03 p.m. UTC | #2 
On Wed, Mar 10, 2021 at 01:52:04PM +0100, Andrey Konovalov wrote:
> On Tue, Mar 9, 2021 at 10:43 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Right now, the state of CONFIG_INIT_ON_ALLOC_DEFAULT_ON (and
> > ...ON_FREE...) did not change the assembly ordering of the static branch
> > tests. Use the new jump_label macro to check CONFIG settings to default
> > to the "expected" state, unpessimizes the resulting assembly code.
> >
> > Reviewed-by: Alexander Potapenko <glider@google.com>
> > Link: https://lore.kernel.org/lkml/CAG_fn=X0DVwqLaHJTO6Jw7TGcMSm77GKHinrd0m_6y0SzWOrFA@mail.gmail.com/
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  include/linux/mm.h | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index bf341a9bfe46..2ccd856ac0d1 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -2874,7 +2874,8 @@ static inline void kernel_unpoison_pages(struct page *page, int numpages) { }
> >  DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_ALLOC_DEFAULT_ON, init_on_alloc);
> >  static inline bool want_init_on_alloc(gfp_t flags)
> >  {
> > -       if (static_branch_unlikely(&init_on_alloc))
> > +       if (static_branch_maybe(CONFIG_INIT_ON_ALLOC_DEFAULT_ON,
> > +                               &init_on_alloc))
> >                 return true;
> >         return flags & __GFP_ZERO;
> >  }
> > @@ -2882,7 +2883,8 @@ static inline bool want_init_on_alloc(gfp_t flags)
> >  DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_FREE_DEFAULT_ON, init_on_free);
> >  static inline bool want_init_on_free(void)
> >  {
> > -       return static_branch_unlikely(&init_on_free);
> > +       return static_branch_maybe(CONFIG_INIT_ON_FREE_DEFAULT_ON,
> > +                                  &init_on_free);
> >  }
> >
> >  extern bool _debug_pagealloc_enabled_early;
> 
> Should we also update slab_want_init_on_alloc() and slab_want_init_on_free()?

Whoops! Thank you; I will update and resend. :)
diff mbox series

Patch

diff --git a/include/linux/mm.h b/include/linux/mm.h
index bf341a9bfe46..2ccd856ac0d1 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2874,7 +2874,8 @@  static inline void kernel_unpoison_pages(struct page *page, int numpages) { }
 DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_ALLOC_DEFAULT_ON, init_on_alloc);
 static inline bool want_init_on_alloc(gfp_t flags)
 {
-	if (static_branch_unlikely(&init_on_alloc))
+	if (static_branch_maybe(CONFIG_INIT_ON_ALLOC_DEFAULT_ON,
+				&init_on_alloc))
 		return true;
 	return flags & __GFP_ZERO;
 }
@@ -2882,7 +2883,8 @@  static inline bool want_init_on_alloc(gfp_t flags)
 DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_FREE_DEFAULT_ON, init_on_free);
 static inline bool want_init_on_free(void)
 {
-	return static_branch_unlikely(&init_on_free);
+	return static_branch_maybe(CONFIG_INIT_ON_FREE_DEFAULT_ON,
+				   &init_on_free);
 }
 
 extern bool _debug_pagealloc_enabled_early;