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@xxxxxxxxxxxxxxx
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@xxxxxxxxxxxxxxxx>
> 
> 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