linux-kernel - Re: [PATCH v2] cpufreq: fix locking order in store_local

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] [day] [month] [year] [list]

Message-ID: <laqfi4de6ang3ugwwpvdgpmwffk4ice6vcw34vdwgeo6fsixly@msdle5sobc7o>
Date: Tue, 29 Apr 2025 14:21:29 -0400
From: Seyediman Seyedarab <imandevel@...il.com>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: rafael@...nel.org, linux-pm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, skhan@...uxfoundation.org, linux-kernel-mentees@...ts.linux.dev
Subject: Re: [PATCH v2] cpufreq: fix locking order in store_local_boost to
 prevent deadlock

On 25/04/29 10:54AM, Viresh Kumar wrote:
> On 28-04-25, 13:19, Seyediman Seyedarab wrote:
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index acf19b004..5464c8487 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -1045,11 +1042,17 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr,
> >  	if (!fattr->store)
> >  		return -EIO;
> >  
> > +	if (fattr == &local_boost)
> > +		cpus_read_lock();
> > +
> >  	down_write(&policy->rwsem);
 
> And please rebase over PM tree's linux-next branch, your patch won't
> apply anymore due to recent changes.
 
Since the new code uses the guard() macro to lock the policy,
I see two possible ways to address this:
 1. Switch to scoped_guard() and manually wrap the local boost case
    in an if block, calling cpus_read_lock() and cpus_read_unlock()
    around the scoped_guard(cpufreq_policy_write, policy) block.

 2. Use DEFINE_GUARD() to create a cpu_hotplug_read guard, but this
    requires introducing an empty struct to satisfy the macro. Feels
    semantically awkward, since there's no actual object being guarded.

Option 1 looks more reasonable to me, because it's more direct and avoids
misusing the guard pattern for something that's not resource-based.

Let me know if you'd prefer the second approach for consistency.

Regards,
Seyediman