This is an automated email from the ASF dual-hosted git repository. acassis pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 414694b7803c5e12794ec4a6872eaea662806a59 Author: guanyi3 <[email protected]> AuthorDate: Tue Mar 10 11:52:54 2026 +0800 devfreq/ondemand: fix use-after-free in ondemand worker When devfreq_gov_ondemand_stop() is called from idle task context, work_cancel() is used instead of work_cancel_sync(), which does not wait for the currently running worker to complete. If devfreq_gov_ondemand_exit() then frees governor_data, the worker may still be accessing it, causing a use-after-free crash. Fix this by: - Nullifying dev->governor_data under dev->lock in exit before freeing. - Moving the governor_data read inside dev->lock in the worker and adding a NULL check to bail out early if data has been freed. Signed-off-by: guanyi3 <[email protected]> --- drivers/devfreq/devfreq.c | 31 +++++++++++++++++++++---------- drivers/devfreq/devfreq_ondemand.c | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 52 insertions(+), 13 deletions(-) diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 5fad5626549..bf152c73ec8 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -173,7 +173,7 @@ static int devfreq_start_governor(FAR struct devfreq_s *devfreq) static void devfreq_stop_governor(FAR struct devfreq_s *devfreq) { - if (devfreq->suspended || !devfreq->governor) + if (!devfreq->governor) { return; } @@ -587,21 +587,29 @@ int devfreq_unregister(FAR struct devfreq_s *devfreq) int devfreq_suspend(FAR struct devfreq_s *devfreq) { nxmutex_lock(&devfreq->lock); + devfreq->suspended = true; + nxmutex_unlock(&devfreq->lock); devfreq_stop_governor(devfreq); if (devfreq->driver->suspend) { - int ret = devfreq->driver->suspend(devfreq); + int ret; + + nxmutex_lock(&devfreq->lock); + ret = devfreq->driver->suspend(devfreq); + nxmutex_unlock(&devfreq->lock); + if (ret < 0) { + nxmutex_lock(&devfreq->lock); + devfreq->suspended = false; nxmutex_unlock(&devfreq->lock); + devfreq_start_governor(devfreq); return ret; } } - devfreq->suspended = true; - nxmutex_unlock(&devfreq->lock); return 0; } @@ -621,22 +629,25 @@ int devfreq_suspend(FAR struct devfreq_s *devfreq) int devfreq_resume(struct devfreq_s *devfreq) { - nxmutex_lock(&devfreq->lock); - if (devfreq->driver->resume) { - int ret = devfreq->driver->resume(devfreq); + int ret; + + nxmutex_lock(&devfreq->lock); + ret = devfreq->driver->resume(devfreq); + nxmutex_unlock(&devfreq->lock); + if (ret < 0) { - nxmutex_unlock(&devfreq->lock); return ret; } } + nxmutex_lock(&devfreq->lock); devfreq->suspended = false; - devfreq_start_governor(devfreq); - nxmutex_unlock(&devfreq->lock); + + devfreq_start_governor(devfreq); return 0; } diff --git a/drivers/devfreq/devfreq_ondemand.c b/drivers/devfreq/devfreq_ondemand.c index 0c1f3a81954..a5b3e524046 100644 --- a/drivers/devfreq/devfreq_ondemand.c +++ b/drivers/devfreq/devfreq_ondemand.c @@ -94,11 +94,19 @@ static uint32_t devfreq_gov_ondemand_cpuload(void) static void devfreq_ondemand_worker(FAR void *arg) { FAR struct devfreq_s *dev = arg; - FAR struct devfreq_ondemand_s *data = dev->governor_data; + FAR struct devfreq_ondemand_s *data; + FAR struct qos_request_s *req; uint32_t cpuload; cpuload = devfreq_gov_ondemand_cpuload(); nxmutex_lock(&dev->lock); + data = dev->governor_data; + if (data == NULL) + { + nxmutex_unlock(&dev->lock); + return; + } + if (cpuload > CONFIG_DEVFREQ_LOAD_THRESHOLD) { if (dev->cur < dev->max) @@ -116,14 +124,21 @@ static void devfreq_ondemand_worker(FAR void *arg) (dev->max - dev->min) / 100; } - nxmutex_unlock(&dev->lock); + /* Re-queue before releasing the lock so that exit/stop can + * reliably cancel the pending work after setting governor_data + * to NULL. All accesses to 'data' must happen while holding + * the lock to avoid use-after-free. + */ - devfreq_qos_update_request(dev, data->req, dev->min, dev->max); + req = data->req; work_queue(HPWORK, &data->work, devfreq_ondemand_worker, dev, data->sample_rate / USEC_PER_TICK); + nxmutex_unlock(&dev->lock); + + devfreq_qos_update_request(dev, req, dev->min, dev->max); } static int devfreq_gov_ondemand_init(FAR struct devfreq_s *dev) @@ -151,6 +166,19 @@ static int devfreq_gov_ondemand_exit(FAR struct devfreq_s *dev) devfreq_qos_remove_request(dev, data->req); + /* First, mark governor_data as NULL so that any in-flight worker + * will see it and bail out without re-queuing. + */ + + nxmutex_lock(&dev->lock); + dev->governor_data = NULL; + nxmutex_unlock(&dev->lock); + + /* Cancel any pending work that was queued before we cleared + * governor_data, then it is safe to free. + */ + + work_cancel_sync(HPWORK, &data->work); kmm_free(data); return 0; }
