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

[Jie Zhan <zhanjie9@hisilicon.com>]

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.