* [PATCH v2] PM / devfreq: governor_userspace: Guard against NULL governor_data during switch
@ 2026-10-22 2:21 Finley Xiao
2026-11-14 14:16 ` Choi Chanwoo
0 siblings, 1 reply; 2+ messages in thread
From: Finley Xiao @ 2026-10-22 2:21 UTC (permalink / raw)
To: finley.xiao, heiko, linux-pm
Cc: cw00.choi, myungjoo.ham, kant, kyungmin.park, linux-rockchip,
linux-kernel, tony.xie, huangtao, stable
The userspace governor is the only one that uses devfreq->governor_data,
allocated in its DEVFREQ_GOV_START handler (userspace_init()) and freed
in its DEVFREQ_GOV_STOP handler (userspace_exit()).
When switching governors via sysfs (governor_store in devfreq.c), the
sequence is:
STOP(old) -> df->governor = userspace -> START(userspace) -> userspace_init
Between the df->governor assignment and userspace_init() completing,
governor_data is still NULL (the previous governor never touched it,
and the core does not clear or pre-allocate it). During that window,
asynchronous paths that call update_devfreq() outside of the driver's
control -- the OPP notifier (devfreq.c:686) and the pm_qos notifier
(devfreq.c:707) -- run devfreq_update_target() under df->lock and reach
devfreq_userspace_func(), which dereferences governor_data without a
NULL check, causing:
Unable to handle kernel access to user memory outside uaccess routines
pc : devfreq_userspace_func+0xc/0x28
lr : devfreq_update_target+0x58/0x158
Add a NULL check in devfreq_userspace_func() and fall back to
previous_freq, which is the same behaviour as the existing "no user
frequency specified yet" branch. This closes the race for all callers
since every path ultimately goes through get_target_freq.
Additionally, take devfreq->lock around the kfree()/NULL assignment in
userspace_exit(). Without it, a concurrent OPP/pm_qos notifier running
under devfreq->lock could read governor_data after it is freed but
before it is set to NULL, constituting a use-after-free.
Fixes: 5fdded844892 ("PM/devfreq: governor: Add a private governor_data for governor")
Cc: stable@vger.kernel.org
Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
Link: https://lore.kernel.org/linux-pm/20260723080806.1451-1-finley.xiao@rock-chips.com/
---
Changes in v2:
- Add Fixes: tag and Cc: stable@vger.kernel.org
drivers/devfreq/governor_userspace.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
index 3906ebedbae8..f7a485c43fd7 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -24,7 +24,7 @@ static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
{
struct userspace_data *data = df->governor_data;
- if (data->valid)
+ if (data && data->valid)
*freq = data->user_frequency;
else
*freq = df->previous_freq; /* No user freq specified yet */
@@ -110,8 +110,10 @@ static void userspace_exit(struct devfreq *devfreq)
if (devfreq->dev.kobj.sd)
sysfs_remove_group(&devfreq->dev.kobj, &dev_attr_group);
+ mutex_lock(&devfreq->lock);
kfree(devfreq->governor_data);
devfreq->governor_data = NULL;
+ mutex_unlock(&devfreq->lock);
}
static int devfreq_userspace_handler(struct devfreq *devfreq,
--
2.43.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v2] PM / devfreq: governor_userspace: Guard against NULL governor_data during switch
2026-10-22 2:21 [PATCH v2] PM / devfreq: governor_userspace: Guard against NULL governor_data during switch Finley Xiao
@ 2026-11-14 14:16 ` Choi Chanwoo
0 siblings, 0 replies; 2+ messages in thread
From: Choi Chanwoo @ 2026-11-14 14:16 UTC (permalink / raw)
To: Finley Xiao, heiko, linux-pm
Cc: cw00.choi, myungjoo.ham, kant, kyungmin.park, linux-rockchip,
linux-kernel, tony.xie, huangtao, stable
On 10/22/2026 11:21 AM, Finley Xiao wrote:
> The userspace governor is the only one that uses devfreq->governor_data,
> allocated in its DEVFREQ_GOV_START handler (userspace_init()) and freed
> in its DEVFREQ_GOV_STOP handler (userspace_exit()).
>
> When switching governors via sysfs (governor_store in devfreq.c), the
> sequence is:
>
> STOP(old) -> df->governor = userspace -> START(userspace) -> userspace_init
>
> Between the df->governor assignment and userspace_init() completing,
> governor_data is still NULL (the previous governor never touched it,
> and the core does not clear or pre-allocate it). During that window,
> asynchronous paths that call update_devfreq() outside of the driver's
> control -- the OPP notifier (devfreq.c:686) and the pm_qos notifier
> (devfreq.c:707) -- run devfreq_update_target() under df->lock and reach
> devfreq_userspace_func(), which dereferences governor_data without a
> NULL check, causing:
>
> Unable to handle kernel access to user memory outside uaccess routines
> pc : devfreq_userspace_func+0xc/0x28
> lr : devfreq_update_target+0x58/0x158
>
> Add a NULL check in devfreq_userspace_func() and fall back to
> previous_freq, which is the same behaviour as the existing "no user
> frequency specified yet" branch. This closes the race for all callers
> since every path ultimately goes through get_target_freq.
>
> Additionally, take devfreq->lock around the kfree()/NULL assignment in
> userspace_exit(). Without it, a concurrent OPP/pm_qos notifier running
> under devfreq->lock could read governor_data after it is freed but
> before it is set to NULL, constituting a use-after-free.
>
> Fixes: 5fdded844892 ("PM/devfreq: governor: Add a private governor_data for governor")
> Cc: stable@vger.kernel.org
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
>
> Link: https://lore.kernel.org/linux-pm/20260723080806.1451-1-finley.xiao@rock-chips.com/
> ---
> Changes in v2:
> - Add Fixes: tag and Cc: stable@vger.kernel.org
>
> drivers/devfreq/governor_userspace.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
> index 3906ebedbae8..f7a485c43fd7 100644
> --- a/drivers/devfreq/governor_userspace.c
> +++ b/drivers/devfreq/governor_userspace.c
> @@ -24,7 +24,7 @@ static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
> {
> struct userspace_data *data = df->governor_data;
>
> - if (data->valid)
> + if (data && data->valid)
> *freq = data->user_frequency;
> else
> *freq = df->previous_freq; /* No user freq specified yet */
> @@ -110,8 +110,10 @@ static void userspace_exit(struct devfreq *devfreq)
> if (devfreq->dev.kobj.sd)
> sysfs_remove_group(&devfreq->dev.kobj, &dev_attr_group);
>
> + mutex_lock(&devfreq->lock);
> kfree(devfreq->governor_data);
> devfreq->governor_data = NULL;
> + mutex_unlock(&devfreq->lock);
> }
>
> static int devfreq_userspace_handler(struct devfreq *devfreq,
Hi,
Looks good to me.
I think that it is needed to lock on usrespace_init.
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
index f7a485c43fd7..cfbd72892598 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -94,7 +94,9 @@ static int userspace_init(struct devfreq *devfreq)
goto out;
}
data->valid = false;
+ mutex_lock(&devfreq->lock);
devfreq->governor_data = data;
+ mutex_unlock(&devfreq->lock);
err = sysfs_create_group(&devfreq->dev.kobj, &dev_attr_group);
out:
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-11-14 14:16 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-22 2:21 [PATCH v2] PM / devfreq: governor_userspace: Guard against NULL governor_data during switch Finley Xiao
2026-11-14 14:16 ` Choi Chanwoo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox