[dahdi-commits] sruffell: linux/trunk r9353 - /linux/trunk/drivers/dahdi/dahdi-base.c

SVN commits to the DAHDI project dahdi-commits at lists.digium.com
Mon Sep 20 15:32:34 CDT 2010


Author: sruffell
Date: Mon Sep 20 15:32:29 2010
New Revision: 9353

URL: http://svnview.digium.com/svn/dahdi?view=rev&rev=9353
Log:
dahdi: Be more tolerant of surprise removal of channels.

Enable DAHDI to detect if an operation on a file handle refers to a
channel that may have been unregistered. This can occur, for example,
when a board driver is hot-swapped out in a live system.

This patch ensures that file->private_data is always properly set for
any open channel, and it's set back to NULL when a channel is
unregistered.  This way file->private_data can be used to check whether
it's valid to perform an operation on the channel.  (NOTE:  There is
still a race condition here if the driver was unbound on one processor
during the window of time between when file->private_data was checked
and the system call finishes).

Also, since DAHDI should only return -ENODEV on read or write when there
was a surprise device removal on a running system this sleep can prevent
the system from becoming unresponsive if the userspace application does
not check for the -ENODEV error and constantly tries to call read with
elevated privileges.

(issue #17669)
Reported by: tzafrir
Tested by: sruffell

Review: https://reviewboard.asterisk.org/r/905/

Signed-off-by: Shaun Ruffell <sruffell at digium.com>

Modified:
    linux/trunk/drivers/dahdi/dahdi-base.c

Modified: linux/trunk/drivers/dahdi/dahdi-base.c
URL: http://svnview.digium.com/svn/dahdi/linux/trunk/drivers/dahdi/dahdi-base.c?view=diff&rev=9353&r1=9352&r2=9353
==============================================================================
--- linux/trunk/drivers/dahdi/dahdi-base.c (original)
+++ linux/trunk/drivers/dahdi/dahdi-base.c Mon Sep 20 15:32:29 2010
@@ -47,6 +47,7 @@
 #include <linux/moduleparam.h>
 #include <linux/sched.h>
 #include <linux/list.h>
+#include <linux/delay.h>
 
 #ifdef HAVE_UNLOCKED_IOCTL
 #include <linux/smp_lock.h>
@@ -1971,6 +1972,15 @@
 
 	might_sleep();
 
+	/* In the case of surprise removal of hardware, make sure any open
+	 * file handles to this channel are disassociated with the actual
+	 * dahdi_chan. */
+	if (chan->file) {
+		chan->file->private_data = NULL;
+		if (chan->span)
+			module_put(chan->span->ops->owner);
+	}
+
 	release_echocan(chan->ec_factory);
 
 #ifdef CONFIG_DAHDI_NET
@@ -2024,9 +2034,10 @@
 	write_unlock_irqrestore(&chan_lock, flags);
 }
 
-static ssize_t dahdi_chan_read(struct file *file, char __user *usrbuf, size_t count, int unit)
-{
-	struct dahdi_chan *chan = chans[unit];
+static ssize_t dahdi_chan_read(struct file *file, char __user *usrbuf,
+			       size_t count)
+{
+	struct dahdi_chan *chan = file->private_data;
 	int amnt;
 	int res, rv;
 	int oldbuf,x;
@@ -2035,10 +2046,19 @@
 	/* Make sure count never exceeds 65k, and make sure it's unsigned */
 	count &= 0xffff;
 
-	if (!chan)
-		return -EINVAL;
-
-	if (count < 1)
+	if (unlikely(!chan)) {
+		/* We would typically be here because of surprise hardware
+		 * removal or driver unbinding while a user space application
+		 * has a channel open.  Most telephony applications are run at
+		 * elevated priorities so this sleep can prevent the high
+		 * priority threads from consuming the CPU if they're not
+		 * expecting surprise device removal.
+		 */
+		msleep(5);
+		return -ENODEV;
+	}
+
+	if (unlikely(count < 1))
 		return -EINVAL;
 
 	for (;;) {
@@ -2150,21 +2170,30 @@
 	return range1 + range2;
 }
 
-static ssize_t dahdi_chan_write(struct file *file, const char __user *usrbuf, size_t count, int unit)
+static ssize_t dahdi_chan_write(struct file *file, const char __user *usrbuf,
+				size_t count)
 {
 	unsigned long flags;
-	struct dahdi_chan *chan = chans[unit];
+	struct dahdi_chan *chan = file->private_data;
 	int res, amnt, oldbuf, rv, x;
 
 	/* Make sure count never exceeds 65k, and make sure it's unsigned */
 	count &= 0xffff;
 
-	if (!chan)
+	if (unlikely(!chan)) {
+		/* We would typically be here because of surprise hardware
+		 * removal or driver unbinding while a user space application
+		 * has a channel open.  Most telephony applications are run at
+		 * elevated priorities so this sleep can prevent the high
+		 * priority threads from consuming the CPU if they're not
+		 * expecting surprise device removal.
+		 */
+		msleep(5);
+		return -ENODEV;
+	}
+
+	if (unlikely(count < 1))
 		return -EINVAL;
-
-	if (count < 1) {
-		return -EINVAL;
-	}
 
 	for (;;) {
 		spin_lock_irqsave(&chan->lock, flags);
@@ -2748,6 +2777,7 @@
 			}
 			if (!res) {
 				chan->file = file;
+				file->private_data = chan;
 				spin_unlock_irqrestore(&chan->lock, flags);
 			} else {
 				spin_unlock_irqrestore(&chan->lock, flags);
@@ -2912,7 +2942,7 @@
 		chan = file->private_data;
 		if (!chan)
 			return -EINVAL;
-		return dahdi_chan_read(file, usrbuf, count, chan->channo);
+		return dahdi_chan_read(file, usrbuf, count);
 	}
 
 	if (unit == 255) {
@@ -2921,12 +2951,12 @@
 			module_printk(KERN_NOTICE, "No pseudo channel structure to read?\n");
 			return -EINVAL;
 		}
-		return dahdi_chan_read(file, usrbuf, count, chan->channo);
+		return dahdi_chan_read(file, usrbuf, count);
 	}
 	if (count < 0)
 		return -EINVAL;
 
-	return dahdi_chan_read(file, usrbuf, count, unit);
+	return dahdi_chan_read(file, usrbuf, count);
 }
 
 static ssize_t dahdi_write(struct file *file, const char __user *usrbuf, size_t count, loff_t *ppos)
@@ -2944,7 +2974,7 @@
 		chan = file->private_data;
 		if (!chan)
 			return -EINVAL;
-		return dahdi_chan_write(file, usrbuf, count, chan->channo);
+		return dahdi_chan_write(file, usrbuf, count);
 	}
 	if (unit == 255) {
 		chan = file->private_data;
@@ -2952,9 +2982,9 @@
 			module_printk(KERN_NOTICE, "No pseudo channel structure to read?\n");
 			return -EINVAL;
 		}
-		return dahdi_chan_write(file, usrbuf, count, chan->channo);
-	}
-	return dahdi_chan_write(file, usrbuf, count, unit);
+		return dahdi_chan_write(file, usrbuf, count);
+	}
+	return dahdi_chan_write(file, usrbuf, count);
 
 }
 
@@ -5891,6 +5921,12 @@
 		ret = dahdi_chanandpseudo_ioctl(file, cmd, data, chan->channo);
 		goto unlock_exit;
 	}
+
+	if (!file->private_data) {
+		ret = -ENXIO;
+		goto unlock_exit;
+	}
+
 	ret = dahdi_chan_ioctl(file, cmd, data, unit);
 
 unlock_exit:




More information about the dahdi-commits mailing list