[RFC PATCH 6.6.y] atm: suni: synchronize poll timer with device removal

Hongyan Xu posted 1 patch 16 hours ago
drivers/atm/suni.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
[RFC PATCH 6.6.y] atm: suni: synchronize poll timer with device removal
Posted by Hongyan Xu 16 hours ago
suni_hz() walks the global device list and dereferences each private
structure without taking sunis_lock. suni_stop() can therefore unlink and
free a device while the timer callback is still using it. The existing
timer synchronization only runs for the last device and is performed while
holding sunis_lock.

Protect the callback traversal with sunis_lock. Serialize the first/last
timer transitions with a mutex, unlink under the spinlock, then shut the
timer down outside the spinlock before freeing the last device.
timer_shutdown_sync() also prevents the running callback from rearming
itself; a later first start reinitializes the timer with timer_setup().
Move device initialization before list publication so an existing timer
cannot observe a partially initialized entry.

This driver was removed from mainline by commit 6deb53595092 ("net: remove
unused ATM protocols and legacy ATM device drivers"), but is still present
in the 6.6 LTS series.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
---
 drivers/atm/suni.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/atm/suni.c b/drivers/atm/suni.c
index 21e5acc..5fa939b 100644
--- a/drivers/atm/suni.c
+++ b/drivers/atm/suni.c
@@ -14,6 +14,7 @@
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
+#include <linux/mutex.h>
 #include <linux/errno.h>
 #include <linux/atmdev.h>
 #include <linux/sonet.h>
@@ -46,6 +47,7 @@
 static struct timer_list poll_timer;
 static struct suni_priv *sunis = NULL;
 static DEFINE_SPINLOCK(sunis_lock);
+static DEFINE_MUTEX(sunis_mutex);
 
 
 #define ADD_LIMITED(s,v) \
@@ -59,6 +61,7 @@ static void suni_hz(struct timer_list *timer)
 	struct atm_dev *dev;
 	struct k_sonet_stats *stats;
 
+	spin_lock_bh(&sunis_lock);
 	for (walk = sunis; walk; walk = walk->next) {
 		dev = walk->dev;
 		stats = &walk->sonet_stats;
@@ -85,6 +88,7 @@ static void suni_hz(struct timer_list *timer)
 		    ((GET(TACP_TCC) & 0xff) << 8) |
 		    ((GET(TACP_TCCM) & 7) << 16));
 	}
+	spin_unlock_bh(&sunis_lock);
 	if (timer) mod_timer(&poll_timer,jiffies+HZ);
 }
 
@@ -306,14 +310,9 @@ static void suni_int(struct atm_dev *dev)
 
 static int suni_start(struct atm_dev *dev)
 {
-	unsigned long flags;
 	int first;
 
-	spin_lock_irqsave(&sunis_lock,flags);
-	first = !sunis;
-	PRIV(dev)->next = sunis;
-	sunis = PRIV(dev);
-	spin_unlock_irqrestore(&sunis_lock,flags);
+	mutex_lock(&sunis_mutex);
 	memset(&PRIV(dev)->sonet_stats,0,sizeof(struct k_sonet_stats));
 	PUT(GET(RSOP_CIE) | SUNI_RSOP_CIE_LOSE,RSOP_CIE);
 		/* interrupt on loss of signal */
@@ -322,6 +321,11 @@ static int suni_start(struct atm_dev *dev)
 		printk(KERN_WARNING "%s(itf %d): no signal\n",dev->type,
 		    dev->number);
 	PRIV(dev)->loop_mode = ATM_LM_NONE;
+	spin_lock_bh(&sunis_lock);
+	first = !sunis;
+	PRIV(dev)->next = sunis;
+	sunis = PRIV(dev);
+	spin_unlock_bh(&sunis_lock);
 	suni_hz(NULL); /* clear SUNI counters */
 	(void) fetch_stats(dev,NULL,1); /* clear kernel counters */
 	if (first) {
@@ -333,6 +337,7 @@ printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long) poll_timer.list.prev,
 #endif
 		add_timer(&poll_timer);
 	}
+	mutex_unlock(&sunis_mutex);
 	return 0;
 }
 
@@ -340,16 +345,20 @@ printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long) poll_timer.list.prev,
 static int suni_stop(struct atm_dev *dev)
 {
 	struct suni_priv **walk;
-	unsigned long flags;
+	bool last;
 
 	/* let SAR driver worry about stopping interrupts */
-	spin_lock_irqsave(&sunis_lock,flags);
+	mutex_lock(&sunis_mutex);
+	spin_lock_bh(&sunis_lock);
 	for (walk = &sunis; *walk != PRIV(dev);
 	    walk = &PRIV((*walk)->dev)->next);
 	*walk = PRIV((*walk)->dev)->next;
-	if (!sunis) del_timer_sync(&poll_timer);
-	spin_unlock_irqrestore(&sunis_lock,flags);
+	last = !sunis;
+	spin_unlock_bh(&sunis_lock);
+	if (last)
+		timer_shutdown_sync(&poll_timer);
 	kfree(PRIV(dev));
+	mutex_unlock(&sunis_mutex);
 
 	return 0;
 }
-- 
2.50.1.windows.1