linux-kernel - Re: [PATCH] x86/resctrl: Remove lockdep annotation that triggers false positive

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]

Date: Wed, 21 Feb 2024 08:48:24 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: James Morse <james.morse@....com>, <x86@...nel.org>,
	<linux-kernel@...r.kernel.org>
CC: Fenghua Yu <fenghua.yu@...el.com>, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, H Peter Anvin
	<hpa@...or.com>, Tony Luck <tony.luck@...el.com>
Subject: Re: [PATCH] x86/resctrl: Remove lockdep annotation that triggers
 false positive

Hi James,

On 2/21/2024 4:23 AM, James Morse wrote:
> get_domain_from_cpu() walks a list of domains to find the one that
> contains the specified CPU. This needs to be protected against races
> with CPU hotplug when the list is modified. It has recently gained a
> lockdep annotation to check this.
> 
> The lockdep annotation causes false positives when called via IPI
> as the lock is held, but by another process. Remove it.
> 
> Reported-by: Tony Luck <tony.luck@...el.com>
> Link: https://lore.kernel.org/all/ZdUSwOM9UUNpw84Y@agluck-desk3/
> Fixes: fb700810d30b ("x86/resctrl: Separate arch and fs resctrl locks")
> Signed-off-by: James Morse <james.morse@....com>
> ---
>  arch/x86/kernel/cpu/resctrl/core.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 9f1aa555a8ea..83e40341583e 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -362,15 +362,6 @@ struct rdt_domain *get_domain_from_cpu(int cpu, struct rdt_resource *r)
>  {
>  	struct rdt_domain *d;
>  
> -	/*
> -	 * Walking r->domains, ensure it can't race with cpuhp.
> -	 * Because this is called via IPI by rdt_ctrl_update(), assertions
> -	 * about locks this thread holds will lead to false positives. Check
> -	 * someone is holding the CPUs lock.
> -	 */
> -	if (IS_ENABLED(CONFIG_LOCKDEP))
> -		lockdep_is_cpus_held();
> -
>  	list_for_each_entry(d, &r->domains, list) {
>  		/* Find the domain that contains this CPU */
>  		if (cpumask_test_cpu(cpu, &d->cpu_mask))

I agree with this change. Could you please base it on x86/cache
branch of tip? 

Thank you

Reinette