[PATCH] powerpc/fsl: drain MPIC wakeup work before replacing timer

[PATCH] powerpc/fsl: drain MPIC wakeup work before replacing timer

[Hongyan Xu]

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 <getshell@seu.edu.cn>
---
 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