[PATCH] devfreq: stop monitor before calling governor GOV_STOP

[PATCH] devfreq: stop monitor before calling governor GOV_STOP

[Mukesh Ojha]

The following NULL pointer dereference is observed when devfreq_monitor
fires after userspace_exit() has freed governor_data:

  pc : devfreq_userspace_func+0x10/0x2c
  Code: 39402109 --> ldrb w9, [x0, #8]   (x0 = NULL)
  Call trace:
   devfreq_userspace_func+0x10/0x2c
   devfreq_monitor+0x34/0x134
   process_scheduled_works+0x1d8/0x804
   worker_thread+0x1c0/0x458

The fault address 0x8 is userspace_data.valid — an 8-byte unsigned long
(user_frequency) precedes it, so governor_data == NULL is the cause.

The race: devfreq_resume_device() reads df->governor without holding
devfreq_list_lock, while governor_store() updates it under that lock.
The stale pointer dispatches GOV_RESUME to the old governor
(simple_ondemand) after the switch has already completed:

  CPU 0 (devfreq_resume_device)        CPU 1 (governor_store)

  read df->governor → simple_ondemand
                                       lock(devfreq_list_lock)
                                       GOV_STOP → monitor stopped
                                       df->governor = userspace
                                       GOV_START → governor_data alloc'd
                                       unlock(devfreq_list_lock)
  simple_ondemand->event_handler
    (DEVFREQ_GOV_RESUME)
    → devfreq_monitor_resume()
        stop_polling == true → re-queue work  ← stray monitor!
        stop_polling = false

devfreq_monitor is now queued with df->governor == userspace. When
devfreq_remove_device() or governor_store() next calls userspace GOV_STOP,
userspace_exit() frees governor_data. The stray monitor fires,
dereferences NULL governor_data, and crashes.

Call devfreq_monitor_stop() before every GOV_STOP dispatch in the core,
ensuring the polling work is fully cancelled before any governor tears down
its private data.  devfreq_monitor_stop() is safe unconditionally:
IRQ_DRIVEN governors return immediately, polling governors that already
call it in their own GOV_STOP handler make the second call a no-op
(stop_polling already true), and governors that never polled get a harmless
cancel_delayed_work_sync() on an empty queue.

Four sites are fixed: devfreq_remove_device(), governor_store(),
devfreq_remove_governor() (governor module unload), and timer_store()
(timer-type change performs a GOV_STOP + GOV_START cycle).

Fixes: 7e6fdd4bad03 ("PM / devfreq: Core updates to support devices which can idle")
Cc: stable@vger.kernel.org
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/devfreq/devfreq.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index f08fc6966eae..7538f20657b7 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -993,9 +993,12 @@ int devfreq_remove_device(struct devfreq *devfreq)
 
 	devfreq_cooling_unregister(devfreq->cdev);
 
-	if (devfreq->governor)
+	if (devfreq->governor) {
+		devfreq_monitor_stop(devfreq);
 		devfreq->governor->event_handler(devfreq,
 						 DEVFREQ_GOV_STOP, NULL);
+	}
+
 	device_unregister(&devfreq->dev);
 
 	return 0;
@@ -1344,6 +1347,7 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
 
 		if (!strncmp(devfreq->governor->name, governor->name,
 			     DEVFREQ_NAME_LEN)) {
+			devfreq_monitor_stop(devfreq);
 			ret = devfreq->governor->event_handler(devfreq,
 						DEVFREQ_GOV_STOP, NULL);
 			if (ret) {
@@ -1416,6 +1420,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	 * Stop the current governor and remove the specific sysfs files
 	 * which depend on current governor.
 	 */
+	devfreq_monitor_stop(df);
 	ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
 	if (ret) {
 		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
@@ -1863,6 +1868,7 @@ static ssize_t timer_store(struct device *dev, struct device_attribute *attr,
 	df->profile->timer = timer;
 	mutex_unlock(&df->lock);
 
+	devfreq_monitor_stop(df);
 	ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
 	if (ret) {
 		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
-- 
2.53.0

Re: [PATCH] devfreq: stop monitor before calling governor GOV_STOP

[Mukesh Ojha]

On Tue, Jul 21, 2026 at 10:41:41PM +0530, Mukesh Ojha wrote:
> The following NULL pointer dereference is observed when devfreq_monitor
> fires after userspace_exit() has freed governor_data:
> 
>   pc : devfreq_userspace_func+0x10/0x2c
>   Code: 39402109 --> ldrb w9, [x0, #8]   (x0 = NULL)
>   Call trace:
>    devfreq_userspace_func+0x10/0x2c
>    devfreq_monitor+0x34/0x134
>    process_scheduled_works+0x1d8/0x804
>    worker_thread+0x1c0/0x458
> 
> The fault address 0x8 is userspace_data.valid — an 8-byte unsigned long
> (user_frequency) precedes it, so governor_data == NULL is the cause.
> 
> The race: devfreq_resume_device() reads df->governor without holding
> devfreq_list_lock, while governor_store() updates it under that lock.
> The stale pointer dispatches GOV_RESUME to the old governor
> (simple_ondemand) after the switch has already completed:
> 
>   CPU 0 (devfreq_resume_device)        CPU 1 (governor_store)
> 
>   read df->governor → simple_ondemand
>                                        lock(devfreq_list_lock)
>                                        GOV_STOP → monitor stopped
>                                        df->governor = userspace
>                                        GOV_START → governor_data alloc'd
>                                        unlock(devfreq_list_lock)
>   simple_ondemand->event_handler
>     (DEVFREQ_GOV_RESUME)
>     → devfreq_monitor_resume()
>         stop_polling == true → re-queue work  ← stray monitor!
>         stop_polling = false
> 
> devfreq_monitor is now queued with df->governor == userspace. When
> devfreq_remove_device() or governor_store() next calls userspace GOV_STOP,
> userspace_exit() frees governor_data. The stray monitor fires,
> dereferences NULL governor_data, and crashes.
> 
> Call devfreq_monitor_stop() before every GOV_STOP dispatch in the core,
> ensuring the polling work is fully cancelled before any governor tears down
> its private data.  devfreq_monitor_stop() is safe unconditionally:
> IRQ_DRIVEN governors return immediately, polling governors that already
> call it in their own GOV_STOP handler make the second call a no-op
> (stop_polling already true), and governors that never polled get a harmless
> cancel_delayed_work_sync() on an empty queue.
> 
> Four sites are fixed: devfreq_remove_device(), governor_store(),
> devfreq_remove_governor() (governor module unload), and timer_store()
> (timer-type change performs a GOV_STOP + GOV_START cycle).
> 
> Fixes: 7e6fdd4bad03 ("PM / devfreq: Core updates to support devices which can idle")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

Can we consider reviewing fix for the mentioned issue ?

> ---
>  drivers/devfreq/devfreq.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index f08fc6966eae..7538f20657b7 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -993,9 +993,12 @@ int devfreq_remove_device(struct devfreq *devfreq)
>  
>  	devfreq_cooling_unregister(devfreq->cdev);
>  
> -	if (devfreq->governor)
> +	if (devfreq->governor) {
> +		devfreq_monitor_stop(devfreq);
>  		devfreq->governor->event_handler(devfreq,
>  						 DEVFREQ_GOV_STOP, NULL);
> +	}
> +
>  	device_unregister(&devfreq->dev);
>  
>  	return 0;
> @@ -1344,6 +1347,7 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
>  
>  		if (!strncmp(devfreq->governor->name, governor->name,
>  			     DEVFREQ_NAME_LEN)) {
> +			devfreq_monitor_stop(devfreq);
>  			ret = devfreq->governor->event_handler(devfreq,
>  						DEVFREQ_GOV_STOP, NULL);
>  			if (ret) {
> @@ -1416,6 +1420,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>  	 * Stop the current governor and remove the specific sysfs files
>  	 * which depend on current governor.
>  	 */
> +	devfreq_monitor_stop(df);
>  	ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
>  	if (ret) {
>  		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
> @@ -1863,6 +1868,7 @@ static ssize_t timer_store(struct device *dev, struct device_attribute *attr,
>  	df->profile->timer = timer;
>  	mutex_unlock(&df->lock);
>  
> +	devfreq_monitor_stop(df);
>  	ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
>  	if (ret) {
>  		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
> -- 
> 2.53.0
> 

-- 
-Mukesh Ojha

Re: [PATCH] devfreq: stop monitor before calling governor GOV_STOP

[Mukesh Ojha]

On Mon, Aug 03, 2026 at 11:46:14PM +0530, Mukesh Ojha wrote:
> On Tue, Jul 21, 2026 at 10:41:41PM +0530, Mukesh Ojha wrote:
> > The following NULL pointer dereference is observed when devfreq_monitor
> > fires after userspace_exit() has freed governor_data:
> > 
> >   pc : devfreq_userspace_func+0x10/0x2c
> >   Code: 39402109 --> ldrb w9, [x0, #8]   (x0 = NULL)
> >   Call trace:
> >    devfreq_userspace_func+0x10/0x2c
> >    devfreq_monitor+0x34/0x134
> >    process_scheduled_works+0x1d8/0x804
> >    worker_thread+0x1c0/0x458
> > 
> > The fault address 0x8 is userspace_data.valid — an 8-byte unsigned long
> > (user_frequency) precedes it, so governor_data == NULL is the cause.
> > 
> > The race: devfreq_resume_device() reads df->governor without holding
> > devfreq_list_lock, while governor_store() updates it under that lock.
> > The stale pointer dispatches GOV_RESUME to the old governor
> > (simple_ondemand) after the switch has already completed:
> > 
> >   CPU 0 (devfreq_resume_device)        CPU 1 (governor_store)
> > 
> >   read df->governor → simple_ondemand
> >                                        lock(devfreq_list_lock)
> >                                        GOV_STOP → monitor stopped
> >                                        df->governor = userspace
> >                                        GOV_START → governor_data alloc'd
> >                                        unlock(devfreq_list_lock)
> >   simple_ondemand->event_handler
> >     (DEVFREQ_GOV_RESUME)
> >     → devfreq_monitor_resume()
> >         stop_polling == true → re-queue work  ← stray monitor!
> >         stop_polling = false
> > 
> > devfreq_monitor is now queued with df->governor == userspace. When
> > devfreq_remove_device() or governor_store() next calls userspace GOV_STOP,
> > userspace_exit() frees governor_data. The stray monitor fires,
> > dereferences NULL governor_data, and crashes.
> > 
> > Call devfreq_monitor_stop() before every GOV_STOP dispatch in the core,
> > ensuring the polling work is fully cancelled before any governor tears down
> > its private data.  devfreq_monitor_stop() is safe unconditionally:
> > IRQ_DRIVEN governors return immediately, polling governors that already
> > call it in their own GOV_STOP handler make the second call a no-op
> > (stop_polling already true), and governors that never polled get a harmless
> > cancel_delayed_work_sync() on an empty queue.
> > 
> > Four sites are fixed: devfreq_remove_device(), governor_store(),
> > devfreq_remove_governor() (governor module unload), and timer_store()
> > (timer-type change performs a GOV_STOP + GOV_START cycle).
> > 
> > Fixes: 7e6fdd4bad03 ("PM / devfreq: Core updates to support devices which can idle")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> 
> Can we consider reviewing fix for the mentioned issue ?

Change still applies on linux-next tip, So no need of a rebase.

Can I get review on this ?

-- 
-Mukesh Ojha

Re: [PATCH] devfreq: stop monitor before calling governor GOV_STOP

[Jie Zhan]

On 9/24/2026 8:32 PM, Mukesh Ojha wrote:
> On Mon, Aug 03, 2026 at 11:46:14PM +0530, Mukesh Ojha wrote:
>> On Tue, Jul 21, 2026 at 10:41:41PM +0530, Mukesh Ojha wrote:
>>> The following NULL pointer dereference is observed when devfreq_monitor
>>> fires after userspace_exit() has freed governor_data:
>>>
>>>   pc : devfreq_userspace_func+0x10/0x2c
>>>   Code: 39402109 --> ldrb w9, [x0, #8]   (x0 = NULL)
>>>   Call trace:
>>>    devfreq_userspace_func+0x10/0x2c
>>>    devfreq_monitor+0x34/0x134
>>>    process_scheduled_works+0x1d8/0x804
>>>    worker_thread+0x1c0/0x458
>>>
>>> The fault address 0x8 is userspace_data.valid — an 8-byte unsigned long
>>> (user_frequency) precedes it, so governor_data == NULL is the cause.
>>>
>>> The race: devfreq_resume_device() reads df->governor without holding
>>> devfreq_list_lock, while governor_store() updates it under that lock.
>>> The stale pointer dispatches GOV_RESUME to the old governor
>>> (simple_ondemand) after the switch has already completed:
>>>
>>>   CPU 0 (devfreq_resume_device)        CPU 1 (governor_store)
>>>
>>>   read df->governor → simple_ondemand
>>>                                        lock(devfreq_list_lock)
>>>                                        GOV_STOP → monitor stopped
>>>                                        df->governor = userspace
>>>                                        GOV_START → governor_data alloc'd
>>>                                        unlock(devfreq_list_lock)
>>>   simple_ondemand->event_handler
>>>     (DEVFREQ_GOV_RESUME)
>>>     → devfreq_monitor_resume()
>>>         stop_polling == true → re-queue work  ← stray monitor!
>>>         stop_polling = false
>>>
>>> devfreq_monitor is now queued with df->governor == userspace. When
>>> devfreq_remove_device() or governor_store() next calls userspace GOV_STOP,
>>> userspace_exit() frees governor_data. The stray monitor fires,
>>> dereferences NULL governor_data, and crashes.
>>>
>>> Call devfreq_monitor_stop() before every GOV_STOP dispatch in the core,
>>> ensuring the polling work is fully cancelled before any governor tears down
>>> its private data.  devfreq_monitor_stop() is safe unconditionally:
>>> IRQ_DRIVEN governors return immediately, polling governors that already
>>> call it in their own GOV_STOP handler make the second call a no-op
>>> (stop_polling already true), and governors that never polled get a harmless
>>> cancel_delayed_work_sync() on an empty queue.
>>>
>>> Four sites are fixed: devfreq_remove_device(), governor_store(),
>>> devfreq_remove_governor() (governor module unload), and timer_store()
>>> (timer-type change performs a GOV_STOP + GOV_START cycle).
>>>
>>> Fixes: 7e6fdd4bad03 ("PM / devfreq: Core updates to support devices which can idle")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
>>
>> Can we consider reviewing fix for the mentioned issue ?
> 
> Change still applies on linux-next tip, So no need of a rebase.
> 
> Can I get review on this ?
> 
Unfortunately devfreq hasn't been taken patches for a few months.  AFAIK, a
few patchsets are pending to be queued up before any review can carry on.

Re: [PATCH] devfreq: stop monitor before calling governor GOV_STOP

[Choi Chanwoo]

On 7/22/2026 2:11 AM, Mukesh Ojha wrote:
> The following NULL pointer dereference is observed when devfreq_monitor
> fires after userspace_exit() has freed governor_data:
> 
>   pc : devfreq_userspace_func+0x10/0x2c
>   Code: 39402109 --> ldrb w9, [x0, #8]   (x0 = NULL)
>   Call trace:
>    devfreq_userspace_func+0x10/0x2c
>    devfreq_monitor+0x34/0x134
>    process_scheduled_works+0x1d8/0x804
>    worker_thread+0x1c0/0x458
> 
> The fault address 0x8 is userspace_data.valid — an 8-byte unsigned long
> (user_frequency) precedes it, so governor_data == NULL is the cause.
> 
> The race: devfreq_resume_device() reads df->governor without holding
> devfreq_list_lock, while governor_store() updates it under that lock.
> The stale pointer dispatches GOV_RESUME to the old governor
> (simple_ondemand) after the switch has already completed:
> 
>   CPU 0 (devfreq_resume_device)        CPU 1 (governor_store)
> 
>   read df->governor → simple_ondemand
>                                        lock(devfreq_list_lock)
>                                        GOV_STOP → monitor stopped
>                                        df->governor = userspace
>                                        GOV_START → governor_data alloc'd
>                                        unlock(devfreq_list_lock)
>   simple_ondemand->event_handler
>     (DEVFREQ_GOV_RESUME)
>     → devfreq_monitor_resume()
>         stop_polling == true → re-queue work  ← stray monitor!
>         stop_polling = false
> 
> devfreq_monitor is now queued with df->governor == userspace. When
> devfreq_remove_device() or governor_store() next calls userspace GOV_STOP,
> userspace_exit() frees governor_data. The stray monitor fires,
> dereferences NULL governor_data, and crashes.
> 
> Call devfreq_monitor_stop() before every GOV_STOP dispatch in the core,
> ensuring the polling work is fully cancelled before any governor tears down
> its private data.  devfreq_monitor_stop() is safe unconditionally:
> IRQ_DRIVEN governors return immediately, polling governors that already
> call it in their own GOV_STOP handler make the second call a no-op
> (stop_polling already true), and governors that never polled get a harmless
> cancel_delayed_work_sync() on an empty queue.
> 
> Four sites are fixed: devfreq_remove_device(), governor_store(),
> devfreq_remove_governor() (governor module unload), and timer_store()
> (timer-type change performs a GOV_STOP + GOV_START cycle).
> 
> Fixes: 7e6fdd4bad03 ("PM / devfreq: Core updates to support devices which can idle")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/devfreq/devfreq.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index f08fc6966eae..7538f20657b7 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -993,9 +993,12 @@ int devfreq_remove_device(struct devfreq *devfreq)
>  
>  	devfreq_cooling_unregister(devfreq->cdev);
>  
> -	if (devfreq->governor)
> +	if (devfreq->governor) {
> +		devfreq_monitor_stop(devfreq);
>  		devfreq->governor->event_handler(devfreq,
>  						 DEVFREQ_GOV_STOP, NULL);
> +	}
> +
>  	device_unregister(&devfreq->dev);
>  
>  	return 0;
> @@ -1344,6 +1347,7 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
>  
>  		if (!strncmp(devfreq->governor->name, governor->name,
>  			     DEVFREQ_NAME_LEN)) {
> +			devfreq_monitor_stop(devfreq);
>  			ret = devfreq->governor->event_handler(devfreq,
>  						DEVFREQ_GOV_STOP, NULL);
>  			if (ret) {
> @@ -1416,6 +1420,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>  	 * Stop the current governor and remove the specific sysfs files
>  	 * which depend on current governor.
>  	 */
> +	devfreq_monitor_stop(df);
>  	ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
>  	if (ret) {
>  		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
> @@ -1863,6 +1868,7 @@ static ssize_t timer_store(struct device *dev, struct device_attribute *attr,
>  	df->profile->timer = timer;
>  	mutex_unlock(&df->lock);
>  
> +	devfreq_monitor_stop(df);
>  	ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
>  	if (ret) {
>  		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",

Hi,

I think that need the governor_lock to fix the issue as following.
and initialize devfreq->governor = NULL in devfreq_remove_device()
But I'm not testing it fully. just suggestion.

How about that? and could you test it to fix issue?

 drivers/devfreq/devfreq.c | 46 +++++++++++++++++++++++++++++----------
 include/linux/devfreq.h   |  6 +++++
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index f25304bca37f..814722858c47 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -779,6 +779,7 @@ static void devfreq_dev_release(struct device *dev)
 	if (devfreq->opp_table)
 		dev_pm_opp_put_opp_table(devfreq->opp_table);
 
+	mutex_destroy(&devfreq->governor_lock);
 	mutex_destroy(&devfreq->lock);
 	srcu_cleanup_notifier_head(&devfreq->transition_notifier_list);
 	kfree(devfreq);
@@ -822,6 +823,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	}
 
 	mutex_init(&devfreq->lock);
+	mutex_init(&devfreq->governor_lock);
 	mutex_lock(&devfreq->lock);
 	devfreq->dev.parent = dev;
 	devfreq->dev.class = devfreq_class;
@@ -940,9 +942,11 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		goto err_init;
 	}
 
+	mutex_lock(&devfreq->governor_lock);
 	devfreq->governor = governor;
 	err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
 						NULL);
+	mutex_unlock(&devfreq->governor_lock);
 	if (err) {
 		dev_err_probe(dev, err,
 			"%s: Unable to start governor for the device\n",
@@ -991,9 +995,14 @@ int devfreq_remove_device(struct devfreq *devfreq)
 
 	devfreq_cooling_unregister(devfreq->cdev);
 
-	if (devfreq->governor)
+	mutex_lock(&devfreq->governor_lock);
+	if (devfreq->governor) {
 		devfreq->governor->event_handler(devfreq,
 						 DEVFREQ_GOV_STOP, NULL);
+		/* Prevent a later suspend/resume from reaching the governor */
+		devfreq->governor = NULL;
+	}
+	mutex_unlock(&devfreq->governor_lock);
 	device_unregister(&devfreq->dev);
 
 	return 0;
@@ -1152,12 +1161,16 @@ int devfreq_suspend_device(struct devfreq *devfreq)
 	if (atomic_inc_return(&devfreq->suspend_count) > 1)
 		return 0;
 
+	mutex_lock(&devfreq->governor_lock);
 	if (devfreq->governor) {
 		ret = devfreq->governor->event_handler(devfreq,
 					DEVFREQ_GOV_SUSPEND, NULL);
-		if (ret)
+		if (ret) {
+			mutex_unlock(&devfreq->governor_lock);
 			return ret;
+		}
 	}
+	mutex_unlock(&devfreq->governor_lock);
 
 	if (devfreq->suspend_freq) {
 		mutex_lock(&devfreq->lock);
@@ -1197,12 +1210,16 @@ int devfreq_resume_device(struct devfreq *devfreq)
 			return ret;
 	}
 
+	mutex_lock(&devfreq->governor_lock);
 	if (devfreq->governor) {
 		ret = devfreq->governor->event_handler(devfreq,
 					DEVFREQ_GOV_RESUME, NULL);
-		if (ret)
+		if (ret) {
+			mutex_unlock(&devfreq->governor_lock);
 			return ret;
+		}
 	}
+	mutex_unlock(&devfreq->governor_lock);
 
 	return 0;
 }
@@ -1337,10 +1354,9 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
 		int ret;
 		struct device *dev = devfreq->dev.parent;
 
-		if (!devfreq->governor)
-			continue;
-
-		if (!strncmp(devfreq->governor->name, governor->name,
+		mutex_lock(&devfreq->governor_lock);
+		if (devfreq->governor &&
+		    !strncmp(devfreq->governor->name, governor->name,
 			     DEVFREQ_NAME_LEN)) {
 			ret = devfreq->governor->event_handler(devfreq,
 						DEVFREQ_GOV_STOP, NULL);
@@ -1351,6 +1367,7 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
 			}
 			devfreq->governor = NULL;
 		}
+		mutex_unlock(&devfreq->governor_lock);
 	}
 
 	list_del(&governor->node);
@@ -1398,16 +1415,18 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 		ret = PTR_ERR(governor);
 		goto out;
 	}
+
+	mutex_lock(&df->governor_lock);
 	if (!df->governor)
 		goto start_new_governor;
 
 	if (df->governor == governor) {
 		ret = 0;
-		goto out;
+		goto out_unlock;
 	} else if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)
 		|| IS_SUPPORTED_FLAG(governor->flags, IMMUTABLE)) {
 		ret = -EINVAL;
-		goto out;
+		goto out_unlock;
 	}
 
 	/*
@@ -1418,7 +1437,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	if (ret) {
 		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
 			 __func__, df->governor->name, ret);
-		goto out;
+		goto out_unlock;
 	}
 
 start_new_governor:
@@ -1436,7 +1455,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 		/* Restore previous governor */
 		df->governor = prev_governor;
 		if (!df->governor)
-			goto out;
+			goto out_unlock;
 
 		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
 		if (ret) {
@@ -1444,7 +1463,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 				"%s: reverting to Governor %s failed (%d)\n",
 				__func__, prev_governor->name, ret);
 			df->governor = NULL;
-			goto out;
+			goto out_unlock;
 		}
 	}
 
@@ -1454,6 +1473,8 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	 */
 	ret = sysfs_update_group(&df->dev.kobj, &gov_attr_group);
 
+out_unlock:
+	mutex_unlock(&df->governor_lock);
 out:
 	mutex_unlock(&devfreq_list_lock);
 
@@ -1831,6 +1852,7 @@ static ssize_t timer_store(struct device *dev, struct device_attribute *attr,
 	int ret = 0, i;
 
 	guard(mutex)(&devfreq_list_lock);
+	guard(mutex)(&df->governor_lock);
 
 	if (!df->governor || !df->profile ||
 	    !IS_SUPPORTED_ATTR(df->governor->attrs, TIMER))
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index dc1075dc3446..f5a795f1ad77 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -144,6 +144,11 @@ struct devfreq_stats {
  * @node:	list node - contains the devices with devfreq that have been
  *		registered.
  * @lock:	a mutex to protect accessing devfreq.
+ * @governor_lock:	a mutex to serialize governor switching (STOP,
+ *		@governor update and START) against DEVFREQ_GOV_SUSPEND and
+ *		DEVFREQ_GOV_RESUME dispatch. Lock order: devfreq_list_lock ->
+ *		@governor_lock -> @lock. It must not be taken while holding
+ *		@lock.
  * @dev:	device registered by devfreq class. dev.parent is the device
  *		using devfreq.
  * @profile:	device-specific devfreq profile
@@ -185,6 +190,7 @@ struct devfreq {
 	struct list_head node;
 
 	struct mutex lock;
+	struct mutex governor_lock;	/* see kernel-doc above */
 	struct device dev;
 	struct devfreq_dev_profile *profile;
 	const struct devfreq_governor *governor;
-- 
2.43.0


-- 
Best Regards,
Chanwoo Choi
Principal Engineer, Samsung Electronics