linux-kernel - Re: [PATCH v3 4/4] slub: Force on no_hash_pointers when slub

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAE-0n50F0E706Hzu9KyR7+7NRG39f0Rn05XT9mi8ci8csenxtQ@mail.gmail.com>
Date:   Tue, 1 Jun 2021 18:03:09 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Andrew Morton <akpm@...ux-foundation.org>,
        kernel test robot <lkp@...el.com>
Cc:     kbuild-all@...ts.01.org, clang-built-linux@...glegroups.com,
        Linux Memory Management List <linux-mm@...ck.org>,
        linux-kernel@...r.kernel.org,
        Christoph Lameter <cl@...ux-foundation.org>,
        Pekka Enberg <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Petr Mladek <pmladek@...e.com>, Joe Perches <joe@...ches.com>
Subject: Re: [PATCH v3 4/4] slub: Force on no_hash_pointers when slub_debug is enabled

Quoting Andrew Morton (2021-06-01 17:26:59)
> On Wed, 2 Jun 2021 06:45:55 +0800 kernel test robot <lkp@...el.com> wrote:
>
> > >> mm/slub.c:4464:30: error: use of undeclared identifier 'slub_debug_enabled'
> >            if (static_branch_unlikely(&slub_debug_enabled))
> >                                        ^
> > >> mm/slub.c:4464:30: error: use of undeclared identifier 'slub_debug_enabled'
> > >> mm/slub.c:4464:30: error: use of undeclared identifier 'slub_debug_enabled'
> > >> mm/slub.c:4464:30: error: use of undeclared identifier 'slub_debug_enabled'
> > >> mm/slub.c:4464:6: error: invalid argument type 'void' to unary expression
> >            if (static_branch_unlikely(&slub_debug_enabled))
> >                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Thanks.  Stephen, how about this?

Looks good to me. Thanks for the quick fix!

>
> --- a/mm/slub.c~slub-force-on-no_hash_pointers-when-slub_debug-is-enabled-fix
> +++ a/mm/slub.c
> @@ -117,12 +117,26 @@
>   */
>
>  #ifdef CONFIG_SLUB_DEBUG
> +
>  #ifdef CONFIG_SLUB_DEBUG_ON
>  DEFINE_STATIC_KEY_TRUE(slub_debug_enabled);
>  #else
>  DEFINE_STATIC_KEY_FALSE(slub_debug_enabled);
>  #endif
> -#endif
> +
> +static inline bool __slub_debug_enabled(void)
> +{
> +       return static_branch_unlikely(&slub_debug_enabled);

To make this even better it could be

	return static_branch_maybe(CONFIG_SLUB_DEBUG_ON, &slub_debug_enabled);

> +}
> +
> +#else          /* CONFIG_SLUB_DEBUG */
> +
> +static inline bool __slub_debug_enabled(void)
> +{
> +       return false;
> +}
> +
> +#endif         /* CONFIG_SLUB_DEBUG */
>
>  static inline bool kmem_cache_debug(struct kmem_cache *s)
>  {
> @@ -4493,7 +4507,7 @@ void __init kmem_cache_init(void)
>                 slub_max_order = 0;
>
>         /* Print slub debugging pointers without hashing */
> -       if (static_branch_unlikely(&slub_debug_enabled))
> +       if (__slub_debug_enabled())

It would be super cool if static branches could be optimized out when
they're never changed by any code, nor exported to code, just tested in
conditions. I've no idea if that is the case though.

>                 no_hash_pointers_enable(NULL);
>
>         kmem_cache_node = &boot_kmem_cache_node;