Re: [PATCH RFC v7 4/9] PM / devfreq: Add new target_freq attribute flag for governors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing ABBA deadlock between sysfs read/store operations on governor attributes and governor_store().
- [High] Pre-existing Use-After-Free vulnerabilities due to lockless accesses to df->governor in governor_show() and devfreq_remove_device().
--

commit 362eb55beaf9c6e36069d42a3c923196961acbd8
Author: Sibi Sankar <sibi.sankar@oss.qualcomm.com>

PM / devfreq: Add new target_freq attribute flag for governors

The target_freq sysfs attribute exposes a governor's predicted next target
frequency. This commit introduces DEVFREQ_GOV_ATTR_TARGET_FREQ to allow
governors to opt in to exposing their target_freq via sysfs, and updates
target_freq_show to gate the read on this flag.

> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index f08fc6966eaee..2f27a239e34a6 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1530,6 +1530,12 @@ static ssize_t target_freq_show(struct device *dev,

[Severity: High]
This is a pre-existing issue, but there appears to be an ABBA deadlock
between sysfs read/store operations on governor attributes and governor_store().

When userspace reads or writes attributes in gov_attr_group (like
polling_interval or timer), the sysfs core acquires an active reference
(kn->active) on the kernfs node before calling the sysfs handler
(e.g., polling_interval_show()). The handler then acquires devfreq_list_lock:

    guard(mutex)(&devfreq_list_lock);

Concurrently, if another thread writes to the governor attribute,
governor_store() acquires devfreq_list_lock and calls
sysfs_update_group(&df->dev.kobj, &gov_attr_group):

    mutex_lock(&devfreq_list_lock);
    ...
    ret = sysfs_update_group(&df->dev.kobj, &gov_attr_group);

This function synchronously removes the existing files in the group via
kernfs_remove_by_name(), which waits indefinitely for all active
references on those files to drain.

Because the reader thread holds the active reference and waits for the
lock, and the writer thread holds the lock and waits for the active
reference, a permanent ABBA deadlock occurs. 

(Note that target_freq_show modified in this patch is not affected since it
resides in devfreq_group, not gov_attr_group).

Is there a way to safely restructure the locking here to avoid this deadlock?

[Severity: High]
This isn't a bug introduced by this patch, but there are Use-After-Free
vulnerabilities due to lockless accesses to df->governor in
governor_show() and devfreq_remove_device().

In governor_show() and devfreq_remove_device(), df->governor is
accessed without holding devfreq_list_lock:

drivers/devfreq/devfreq.c:governor_show() {
    ...
    if (!df->governor)
        return -ENOENT;

    return sprintf(buf, "%s\n", df->governor->name);
}

drivers/devfreq/devfreq.c:devfreq_remove_device() {
    ...
    if (devfreq->governor)
        devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_STOP, NULL);
    ...
}

If the active governor is removed concurrently (e.g., via module
unload), devfreq_remove_governor() takes devfreq_list_lock, stops the
governor, sets df->governor = NULL, and drops the lock before the
module is unloaded.

Since the unprotected functions do not acquire devfreq_list_lock to
serialize with governor removal, they can read a valid pointer to the
governor, get preempted while the module is unloaded, and dereference
freed module memory or execute unloaded function pointers when they
resume.

Could devfreq_list_lock be used in these functions to prevent this race?

>  {
>  	struct devfreq *df = to_devfreq(dev);
>  
> +	guard(mutex)(&devfreq_list_lock);
> +
> +	if (!df->profile || !df->governor ||
> +	    !IS_SUPPORTED_ATTR(df->governor->attrs, TARGET_FREQ))
> +		return -EINVAL;
> +
>  	return sprintf(buf, "%lu\n", df->previous_freq);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610-rfc_v7_scmi_memlat-v7-0-f3f68c608f25@oss.qualcomm.com?part=4