linux-kernel - Re: [PATCH] mm: slub: Fix slub_debug disablement for list of slabs

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: <e0442add-dcf0-57d7-2298-3459136673af@codeaurora.org>
Date:   Tue, 27 Jul 2021 10:13:16 +0530
From:   Vijayanand Jitta <vjitta@...eaurora.org>
To:     Vlastimil Babka <vbabka@...e.cz>, cl@...ux.com, penberg@...nel.org,
        rientjes@...gle.com, iamjoonsoo.kim@....com,
        akpm@...ux-foundation.org
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        vinmenon@...eaurora.org
Subject: Re: [PATCH] mm: slub: Fix slub_debug disablement for list of slabs



On 7/27/2021 4:02 AM, Vlastimil Babka wrote:
> On 7/13/21 1:45 PM, vjitta@...eaurora.org wrote:
>> From: Vijayanand Jitta <vjitta@...eaurora.org>
>>
>> Consider the scenario where CONFIG_SLUB_DEBUG_ON is set
>> and we would want to disable slub_debug for few slabs.
>> Using boot parameter with slub_debug=-,slab_name syntax
>> doesn't work as expected i.e; only disabling debugging for
>> the specified list of slabs, instead it disables debugging
>> for all slabs. Fix this.
>>
>> Signed-off-by: Vijayanand Jitta <vjitta@...eaurora.org>
> 
> Would the following work too, and perhaps be easier to follow?

Right, the below change would also work and its easier to follow as you
said. We can go with this.

Reviewed-by: Vijayanand Jitta <vjitta@...eaurora.org>

Thanks,
Vijay

> ----8<----
> diff --git a/mm/slub.c b/mm/slub.c
> index 090fa14628f9..024f49706386 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1400,12 +1400,13 @@ parse_slub_debug_flags(char *str, slab_flags_t *flags, char **slabs, bool init)
>  static int __init setup_slub_debug(char *str)
>  {
>  	slab_flags_t flags;
> +	slab_flags_t global_flags;
>  	char *saved_str;
>  	char *slab_list;
>  	bool global_slub_debug_changed = false;
>  	bool slab_list_specified = false;
>  
> -	slub_debug = DEBUG_DEFAULT_FLAGS;
> +	global_flags = DEBUG_DEFAULT_FLAGS;
>  	if (*str++ != '=' || !*str)
>  		/*
>  		 * No options specified. Switch on full debugging.
> @@ -1417,7 +1418,7 @@ static int __init setup_slub_debug(char *str)
>  		str = parse_slub_debug_flags(str, &flags, &slab_list, true);
>  
>  		if (!slab_list) {
> -			slub_debug = flags;
> +			global_flags = flags;
>  			global_slub_debug_changed = true;
>  		} else {
>  			slab_list_specified = true;
> @@ -1426,16 +1427,18 @@ static int __init setup_slub_debug(char *str)
>  
>  	/*
>  	 * For backwards compatibility, a single list of flags with list of
> -	 * slabs means debugging is only enabled for those slabs, so the global
> -	 * slub_debug should be 0. We can extended that to multiple lists as
> +	 * slabs means debugging is only changed for those slabs, so the global
> +	 * slub_debug should be unchanged (0 or DEBUG_DEFAULT_FLAGS, depending
> +	 * on CONFIG_SLUB_DEBUG_ON). We can extended that to multiple lists as
>  	 * long as there is no option specifying flags without a slab list.
>  	 */
>  	if (slab_list_specified) {
>  		if (!global_slub_debug_changed)
> -			slub_debug = 0;
> +			global_flags = slub_debug;
>  		slub_debug_string = saved_str;
>  	}
>  out:
> +	slub_debug = global_flags;
>  	if (slub_debug != 0 || slub_debug_string)
>  		static_branch_enable(&slub_debug_enabled);
>  	else
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation