Re: [PATCH RFC 06/10] PM / devfreq: Add a governor for tracking remote device frequencies

[sashiko-bot@kernel.org]

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

New issues:
- [High] The remote tracking governor corrupts observed frequencies and causes continuous log spam due to core PM QoS clamping and exact OPP matching.
- [Medium] Missing validation of `get_cur_freq` during `DEVFREQ_GOV_START` causes infinite error log flooding.

Pre-existing issues:
- [High] Bypassing the IMMUTABLE governor restriction allows a local attacker (root) to trigger a NULL pointer dereference by switching a TRACK_REMOTE device to a mutable governor.
--

--- Patch [6]: [PATCH RFC 06/10] PM / devfreq: Add a governor for tracking remote device frequencies ---
commit dc71eb5d4b37ce705f4a10bb03bda903dcd75d0b
Author: Sibi Sankar <sibi.sankar@oss.qualcomm.com>

PM / devfreq: Add a governor for tracking remote device frequencies

This commit adds a new "remote" devfreq governor that uses the track_remote
flag to expose a remote device's frequency to userspace via trans_stat. The
governor avoids advertising DEVFREQ_GOV_ATTR_TARGET_FREQ and acts strictly as
an observer.

> diff --git a/drivers/devfreq/governor_remote.c b/drivers/devfreq/governor_remote.c
> new file mode 100644
> index 0000000000000..df3819757e56f
> --- /dev/null
> +++ b/drivers/devfreq/governor_remote.c
> @@ -0,0 +1,73 @@

[ ... ]

> +static int devfreq_remote_track_handler(struct devfreq *devfreq, unsigned int event, void *data)
> +{
> +	switch (event) {
> +	case DEVFREQ_GOV_START:
> +		devfreq_monitor_start(devfreq);

[Severity: Medium]
Should the governor validate the presence of get_cur_freq before starting
the monitor?

If a device profile is missing the get_cur_freq callback,
devfreq_remote_track_func() returns -ENXIO. This error is intercepted by the
core devfreq_monitor() function, which logs 'dvfs failed with (-6) error' and
reschedules the delayed work, leading to an infinite flood in the system
logs.

Could we check for get_cur_freq during governor initialization or before
calling devfreq_monitor_start() to fail gracefully?

> +		break;

[ ... ]

> +static struct devfreq_governor devfreq_remote_track = {
> +	.name = DEVFREQ_GOV_REMOTE,
> +	.attrs = DEVFREQ_GOV_ATTR_POLLING_INTERVAL
> +		| DEVFREQ_GOV_ATTR_TIMER,
> +	.flags = DEVFREQ_GOV_FLAG_IMMUTABLE

[Severity: High]
This is a pre-existing issue, but I wanted to point out a potential
vulnerability when using DEVFREQ_GOV_FLAG_IMMUTABLE.

If the governor_remote module is unloaded, devfreq_remove_governor() stops
it and sets df->governor to NULL. A privileged user could then write a new
mutable governor name (like simpleondemand) to the device's governor sysfs
node.

In the core governor_store() function, the IMMUTABLE security check is
entirely bypassed if !df->governor is true:

drivers/devfreq/devfreq.c:governor_store() {
    if (!df->governor)
        goto start_new_governor;

    if (df->governor == governor) {
        ...
    } else if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)
        ...
}

When the new mutable governor runs devfreq_set_target() without the
TRACK_REMOTE flag, it will blindly call devfreq->profile->target(). Since
TRACK_REMOTE devices intentionally omit the .target callback, this leads to a
NULL pointer dereference.

> +		| DEVFREQ_GOV_FLAG_TRACK_REMOTE,

[Severity: High]
Does the use of DEVFREQ_GOV_FLAG_TRACK_REMOTE interact correctly with PM QoS
clamping?

The devfreq_remote_track_func() provides the raw observed frequency. However,
the core devfreq_update_target() applies local PM QoS constraints to this
tracked frequency.

Because DEVFREQ_GOV_FLAG_TRACK_REMOTE bypasses the driver's .target callback,
the frequency is never rounded to a valid OPP:

drivers/devfreq/devfreq.c:devfreq_set_target() {
    if (devfreq->governor &&
        IS_SUPPORTED_FLAG(devfreq->governor->flags, TRACK_REMOTE)) {
        freqs.old = devfreq->previous_freq;
        freqs.new = new_freq;
        ...
        goto update_status;
    }
}

When this clamped, unrounded frequency is passed directly to
devfreq_update_status(), it will fail to find an exact match in the
freq_table and return -EINVAL. This causes a continuous log spam of
'Couldn't update frequency transition information' on every polling interval
and breaks trans_stat metrics.

> +	.get_target_freq = devfreq_remote_track_func,
> +	.event_handler = devfreq_remote_track_handler,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-rfc_v8_scmi_memlat-v1-0-cb732bcff1f4@oss.qualcomm.com?part=6