The timer interrupt queues free_work. mpic_free_timer() synchronizes the
interrupt, but it does not drain work queued before the IRQ was freed.
Module exit can therefore free fsl_wakeup before the worker uses it.
The same race exists when a sysfs write replaces the timer. A stale
worker can wait on sysfs_lock while the store path frees the old timer
and publishes a new one, then wake and free the new timer instead.
Serialize stores and exit with timer_store_lock. Clear the old timer under
sysfs_lock, drop that lock, drain free_work, and only then publish a new
timer. Dropping sysfs_lock before cancel_work_sync() avoids deadlocking a
worker already waiting for that lock.
Fixes: a63b3bc7db32 ("powerpc/fsl: add MPIC timer wakeup support")
Signed-off-by: Hongyan Xu <[email protected]>
---
arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c | 28 +++++++++++++++------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
b/arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
index f63b89adf9f3..51710c7cdaf5 100644
--- a/arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
+++ b/arch/powerpc/sysdev/fsl_mpic_timer_wakeup.c
@@ -23,6 +23,7 @@ struct fsl_mpic_timer_wakeup {
static struct fsl_mpic_timer_wakeup *fsl_wakeup;
static DEFINE_MUTEX(sysfs_lock);
+static DEFINE_MUTEX(timer_store_lock);
static void fsl_free_resource(struct work_struct *ws)
{
@@ -76,32 +77,43 @@ static ssize_t fsl_timer_wakeup_store(struct device *dev,
if (kstrtoll(buf, 0, &interval))
return -EINVAL;
- guard(mutex)(&sysfs_lock);
+ guard(mutex)(&timer_store_lock);
+
+ mutex_lock(&sysfs_lock);
if (fsl_wakeup->timer) {
disable_irq_wake(fsl_wakeup->timer->irq);
mpic_free_timer(fsl_wakeup->timer);
fsl_wakeup->timer = NULL;
}
+ mutex_unlock(&sysfs_lock);
+
+ cancel_work_sync(&fsl_wakeup->free_work);
if (!interval)
return count;
+ mutex_lock(&sysfs_lock);
fsl_wakeup->timer = mpic_request_timer(fsl_mpic_timer_irq,
fsl_wakeup, interval);
- if (!fsl_wakeup->timer)
- return -EINVAL;
+ if (!fsl_wakeup->timer) {
+ ret = -EINVAL;
+ goto unlock;
+ }
ret = enable_irq_wake(fsl_wakeup->timer->irq);
if (ret) {
mpic_free_timer(fsl_wakeup->timer);
fsl_wakeup->timer = NULL;
- return ret;
+ goto unlock;
}
mpic_start_timer(fsl_wakeup->timer);
+ ret = count;
- return count;
+unlock:
+ mutex_unlock(&sysfs_lock);
+ return ret;
}
static struct device_attribute mpic_attributes = __ATTR(timer_wakeup, 0644,
@@ -139,16 +151,18 @@ static void __exit fsl_wakeup_sys_exit(void)
put_device(dev_root);
}
+ guard(mutex)(&timer_store_lock);
mutex_lock(&sysfs_lock);
if (fsl_wakeup->timer) {
disable_irq_wake(fsl_wakeup->timer->irq);
mpic_free_timer(fsl_wakeup->timer);
+ fsl_wakeup->timer = NULL;
}
- kfree(fsl_wakeup);
-
mutex_unlock(&sysfs_lock);
+ cancel_work_sync(&fsl_wakeup->free_work);
+ kfree(fsl_wakeup);
}
module_init(fsl_wakeup_sys_init);
--
2.50.1.windows.1
