Skip to contentSkip to conversation

TTY/n_gsm: potential double lock

PATCH In review Dan Carpenter 2 participants 2 messages May 25, 2010

Subsystems (2)
Kernel janitors Char and misc drivers
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 TTY/n_gsm: potential double lock

In gsm_dlci_data_kick() we call gsm_dlci_data_sweep() with the "gsm->tx_lock" held so we can't lock it again inside gsm_dlci_data_sweep(). I removed that lock from and added one to gsmld_write_wakeup() instead. The sweep function is only called from those two places.

Signed-off-by Dan Carpenter <[email protected]>
drivers/char/n_gsm.c +5 -4
index c4161d5..e4089c4 100644
@@ -904,9 +904,7 @@ static void gsm_dlci_data_sweep(struct gsm_mux *gsm)
 	int len;
 	/* Priority ordering: We should do priority with RR of the groups */
 	int i = 1;
-	unsigned long flags;
 
-	spin_lock_irqsave(&gsm->tx_lock, flags);
 	while (i < NUM_DLCI) {
 		struct gsm_dlci *dlci;
 

@@ -927,7 +925,6 @@ static void gsm_dlci_data_sweep(struct gsm_mux *gsm)
 		if (len = 0)
 			i++;
 	}
-	spin_unlock_irqrestore(&gsm->tx_lock, flags);
 }
 
 /**

@@ -2230,12 +2227,16 @@ static int gsmld_open(struct tty_struct *tty)
 static void gsmld_write_wakeup(struct tty_struct *tty)
 {
 	struct gsm_mux *gsm = tty->disc_data;
+	unsigned long flags;
 
 	/* Queue poll */
 	clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 	gsm_data_kick(gsm);
-	if (gsm->tx_bytes < TX_THRESH_LO)
+	if (gsm->tx_bytes < TX_THRESH_LO) {
+		spin_lock_irqsave(&gsm->tx_lock, flags);
 		gsm_dlci_data_sweep(gsm);
+		spin_unlock_irqrestore(&gsm->tx_lock, flags);
+	}
 }
 
 /**
Acked-by on v1 1/1
#1 lore raw
Quoted 5 lines · On Tue, 25 May 2010 11:37:17 +0200 Dan Carpenter <[email protected]> wrote:

In gsm_dlci_data_kick() we call gsm_dlci_data_sweep() with the "gsm->tx_lock" held so we can't lock it again inside gsm_dlci_data_sweep(). I removed that lock from and added one to gsmld_write_wakeup() instead. The sweep function is only called from those two places.

Well spotted - I'm surprised that escaped as I've been using the code a fair bit in that exact form. Not immediately obvious that the case should be that hard to trigger.

Quoted 1 line
Signed-off-by Dan Carpenter <[email protected]>
Acked-by Alan Cox <[email protected]>