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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <8a464fef-0530-aebf-5844-c8aaf2bf6f37@suse.cz>
Date:   Wed, 2 Jun 2021 12:48:49 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Stephen Boyd <swboyd@...omium.org>,
        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>,
        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

On 6/2/21 3:03 AM, Stephen Boyd wrote:
> 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())

A minimal fix would be to put this under #ifdef CONFIG_SLUB_DEBUG
and use static_key_enabled() as we don't need the jump label optimization for
init code. But the current fix works.

> 
> 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;
>