Do not call dahdi_span_ops.open with spinlock held.

This makes the open symmetrical with the close. It is also considered good
practice to not call through callbacks with spinlocks held.

I modified all the drivers where I could not tell whether it was necessary to
hold the chan->lock with interrupts disabled to simply take the lock inside the
callback.

Signed-off-by: Shaun Ruffell <sruffell@digium.com>
Signed-off-by: Russ Meyerriecks <rmeyerriecks@digium.com>

drivers/dahdi/dahdi-base.c

@@ -3057,57 +3057,64 @@ static const struct file_operations dahdi_chan_fops;
 
 static int dahdi_specchan_open(struct file *file)
 {
-	int res = 0;
+	int res = -ENXIO;
 	struct dahdi_chan *const chan = chan_from_file(file);
 
-	if (chan && chan->sig) {
+	if (!chan || !chan->sig)
-		/* Make sure we're not already open, a net device, or a slave device */
+		return -ENXIO;
-		if (dahdi_have_netdev(chan))
+
-			res = -EBUSY;
+	/* Make sure we're not already open, a net device, or a slave
-		else if (chan->master != chan)
+	 * device */
-			res = -EBUSY;
+	if (dahdi_have_netdev(chan))
-		else if ((chan->sig & __DAHDI_SIG_DACS) == __DAHDI_SIG_DACS)
+		res = -EBUSY;
-			res = -EBUSY;
+	else if (chan->master != chan)
-		else if (!test_and_set_bit(DAHDI_FLAGBIT_OPEN, &chan->flags)) {
+		res = -EBUSY;
-			unsigned long flags;
+	else if ((chan->sig & __DAHDI_SIG_DACS) == __DAHDI_SIG_DACS)
-			res = initialize_channel(chan);
+		res = -EBUSY;
-			if (res) {
+	else if (!test_and_set_bit(DAHDI_FLAGBIT_OPEN, &chan->flags)) {
-				/* Reallocbufs must have failed */
+		unsigned long flags;
-				clear_bit(DAHDI_FLAGBIT_OPEN, &chan->flags);
+		const struct dahdi_span_ops *const ops =
-				return res;
+		       (!is_pseudo_chan(chan)) ? chan->span->ops : NULL;
-			}
+
-			spin_lock_irqsave(&chan->lock, flags);
+		if (ops && !try_module_get(ops->owner)) {
-			if (is_pseudo_chan(chan))
+			clear_bit(DAHDI_FLAGBIT_OPEN, &chan->flags);
-				chan->flags |= DAHDI_FLAG_AUDIO;
+			return -ENXIO;
-			if (chan->span) {
-				const struct dahdi_span_ops *const ops =
-								chan->span->ops;
-				if (!try_module_get(ops->owner)) {
-					res = -ENXIO;
-				} else if (ops->open) {
-					res = ops->open(chan);
-					if (res)
-						module_put(ops->owner);
-				}
-			}
-			if (!res) {
-				chan->file = file;
-				file->private_data = chan;
-				/* Since we know we're a channel now, we can
-				 * update the f_op pointer and bypass a few of
-				 * the checks on the minor number. */
-				file->f_op = &dahdi_chan_fops;
-				spin_unlock_irqrestore(&chan->lock, flags);
-			} else {
-				spin_unlock_irqrestore(&chan->lock, flags);
-				close_channel(chan);
-				clear_bit(DAHDI_FLAGBIT_OPEN, &chan->flags);
-			}
-		} else {
-			res = -EBUSY;
 		}
-	} else
+
-		res = -ENXIO;
+		res = initialize_channel(chan);
+		if (res) {
+			/* Reallocbufs must have failed */
+			clear_bit(DAHDI_FLAGBIT_OPEN, &chan->flags);
+			return res;
+		}
+
+		spin_lock_irqsave(&chan->lock, flags);
+		if (is_pseudo_chan(chan))
+			chan->flags |= DAHDI_FLAG_AUDIO;
+		chan->file = file;
+		file->private_data = chan;
+		/* Since we know we're a channel now, we can
+		 * update the f_op pointer and bypass a few of
+		 * the checks on the minor number. */
+		file->f_op = &dahdi_chan_fops;
+		spin_unlock_irqrestore(&chan->lock, flags);
+
+		if (ops && ops->open) {
+			res = ops->open(chan);
+			if (res) {
+				spin_lock_irqsave(&chan->lock, flags);
+				chan->file = NULL;
+				file->private_data = NULL;
+				spin_unlock_irqrestore(&chan->lock, flags);
+				module_put(ops->owner);
+				close_channel(chan);
+				clear_bit(DAHDI_FLAGBIT_OPEN,
+					  &chan->flags);
+			}
+		}
+	} else {
+		res = -EBUSY;
+	}
 	return res;
 }
 

drivers/dahdi/wcb4xxp/base.c

@@ -2430,7 +2430,7 @@ b4xxp_chanconfig(struct file *file, struct dahdi_chan *chan, int sigtype)
 	return 0;
 }
 
-static int b4xxp_open(struct dahdi_chan *chan)
+static int _b4xxp_open(struct dahdi_chan *chan)
 {
 	struct b4xxp *b4 = chan->pvt;
 	struct b4xxp_span *bspan = &b4->spans[chan->span->offset];
@@ -2444,6 +2444,15 @@ static int b4xxp_open(struct dahdi_chan *chan)
 	return 0;
 }
 
+static int b4xxp_open(struct dahdi_chan *chan)
+{
+	unsigned long flags;
+	int res;
+	spin_lock_irqsave(&chan->lock, flags);
+	res = _b4xxp_open(chan);
+	spin_unlock_irqrestore(&chan->lock, flags);
+	return res;
+}
 
 static int b4xxp_close(struct dahdi_chan *chan)
 {

drivers/dahdi/wcfxo.c

@@ -570,7 +570,7 @@ static inline struct wcfxo *wcfxo_from_span(struct dahdi_span *span)
 	return container_of(span, struct wcfxo, span);
 }
 
-static int wcfxo_open(struct dahdi_chan *chan)
+static int _wcfxo_open(struct dahdi_chan *chan)
 {
 	struct wcfxo *wc = chan->pvt;
 	if (wc->dead)
@@ -579,6 +579,16 @@ static int wcfxo_open(struct dahdi_chan *chan)
 	return 0;
 }
 
+static int wcfxo_open(struct dahdi_chan *chan)
+{
+	int res;
+	unsigned long flags;
+	spin_lock_irqsave(&chan->lock, flags);
+	res = _wcfxo_open(chan);
+	spin_unlock_irqrestore(&chan->lock, flags);
+	return res;
+}
+
 static int wcfxo_watchdog(struct dahdi_span *span, int event)
 {
 	printk(KERN_INFO "FXO: Restarting DMA\n");

drivers/dahdi/wctdm.c

@@ -2144,7 +2144,7 @@ static int wctdm_ioctl(struct dahdi_chan *chan, unsigned int cmd, unsigned long
 
 }
 
-static int wctdm_open(struct dahdi_chan *chan)
+static int _wctdm_open(struct dahdi_chan *chan)
 {
 	struct wctdm *wc = chan->pvt;
 	if (!(wc->cardflag & (1 << (chan->chanpos - 1))))
@@ -2155,6 +2155,16 @@ static int wctdm_open(struct dahdi_chan *chan)
 	return 0;
 }
 
+static int wctdm_open(struct dahdi_chan *chan)
+{
+	unsigned long flags;
+	int res;
+	spin_lock_irqsave(&chan->lock, flags);
+	res = _wctdm_open(chan);
+	spin_unlock_irqrestore(&chan->lock, flags);
+	return res;
+}
+
 static inline struct wctdm *wctdm_from_span(struct dahdi_span *span)
 {
 	return container_of(span, struct wctdm, span);

drivers/dahdi/wcte11xp.c

@@ -225,7 +225,7 @@ static inline void __select_control(struct t1 *wc)
 	}
 }
 
-static int t1xxp_open(struct dahdi_chan *chan)
+static int _t1xxp_open(struct dahdi_chan *chan)
 {
 	struct t1 *wc = chan->pvt;
 	if (wc->dead)
@@ -235,6 +235,16 @@ static int t1xxp_open(struct dahdi_chan *chan)
 	return 0;
 }
 
+static int t1xxp_open(struct dahdi_chan *chan)
+{
+	unsigned long flags;
+	int res;
+	spin_lock_irqsave(&chan->lock, flags);
+	res = _t1xxp_open(chan);
+	spin_unlock_irqrestore(&chan->lock, flags);
+	return res;
+}
+
 static int __control_set_reg(struct t1 *wc, int reg, unsigned char val)
 {
 	__select_control(wc);

drivers/dahdi/xpp/xpp_dahdi.c

@@ -714,10 +714,10 @@ EXPORT_SYMBOL(hookstate_changed);
 /*------------------------- Dahdi Interfaces -----------------------*/
 
 /*
- * Called from dahdi with spinlock held on chan. Must not call back
+ * Called with spinlock held on chan. Must not call back
  * dahdi functions.
  */
-int xpp_open(struct dahdi_chan *chan)
+static int _xpp_open(struct dahdi_chan *chan)
 {
 	xpd_t *xpd;
 	xbus_t *xbus;
@@ -750,6 +750,16 @@ int xpp_open(struct dahdi_chan *chan)
 		CALL_PHONE_METHOD(card_open, xpd, pos);
 	return 0;
 }
+
+int xpp_open(struct dahdi_chan *chan)
+{
+	unsigned long flags;
+	int res;
+	spin_lock_irqsave(&chan->lock, flags);
+	res = _xpp_open(chan);
+	spin_unlock_irqrestore(&chan->lock, flags);
+	return res;
+}
 EXPORT_SYMBOL(xpp_open);
 
 int xpp_close(struct dahdi_chan *chan)