@@ -153,6 +153,24 @@ struct imon_context {
const struct imon_usb_dev_descr *dev_descr;
/* device description with key */
/* table for front panels */
+ /*
+ * Fields for deferring free_imon_context().
+ *
+ * Since reference to "struct imon_context" is stored into
+ * "struct file_operations"->private_data, we need to remember
+ * how many file descriptors might access this "struct imon_context".
+ */
+ refcount_t users;
+ /*
+ * Use a flag for telling display_open()/vfd_write()/lcd_write() that
+ * imon_disconnect() was already called.
+ */
+ bool disconnected;
+ /*
+ * We need to wait for RCU grace period in order to allow
+ * display_open() to safely check ->disconnected and increment ->users.
+ */
+ struct rcu_head rcu;
};
#define TOUCH_TIMEOUT (HZ/30)
@@ -160,18 +178,18 @@ struct imon_context {
/* vfd character device file operations */
static const struct file_operations vfd_fops = {
.owner = THIS_MODULE,
- .open = &display_open,
- .write = &vfd_write,
- .release = &display_close,
+ .open = display_open,
+ .write = vfd_write,
+ .release = display_close,
.llseek = noop_llseek,
};
/* lcd character device file operations */
static const struct file_operations lcd_fops = {
.owner = THIS_MODULE,
- .open = &display_open,
- .write = &lcd_write,
- .release = &display_close,
+ .open = display_open,
+ .write = lcd_write,
+ .release = display_close,
.llseek = noop_llseek,
};
@@ -439,9 +457,6 @@ static struct usb_driver imon_driver = {
.id_table = imon_usb_id_table,
};
-/* to prevent races between open() and disconnect(), probing, etc */
-static DEFINE_MUTEX(driver_lock);
-
/* Module bookkeeping bits */
MODULE_AUTHOR(MOD_AUTHOR);
MODULE_DESCRIPTION(MOD_DESC);
@@ -481,9 +496,11 @@ static void free_imon_context(struct imon_context *ictx)
struct device *dev = ictx->dev;
usb_free_urb(ictx->tx_urb);
+ WARN_ON(ictx->dev_present_intf0);
usb_free_urb(ictx->rx_urb_intf0);
+ WARN_ON(ictx->dev_present_intf1);
usb_free_urb(ictx->rx_urb_intf1);
- kfree(ictx);
+ kfree_rcu(ictx, rcu);
dev_dbg(dev, "%s: iMON context freed\n", __func__);
}
@@ -499,9 +516,6 @@ static int display_open(struct inode *inode, struct file *file)
int subminor;
int retval = 0;
- /* prevent races with disconnect */
- mutex_lock(&driver_lock);
-
subminor = iminor(inode);
interface = usb_find_interface(&imon_driver, subminor);
if (!interface) {
@@ -509,13 +523,16 @@ static int display_open(struct inode *inode, struct file *file)
retval = -ENODEV;
goto exit;
}
- ictx = usb_get_intfdata(interface);
- if (!ictx) {
+ rcu_read_lock();
+ ictx = usb_get_intfdata(interface);
+ if (!ictx || ictx->disconnected || !refcount_inc_not_zero(&ictx->users)) {
+ rcu_read_unlock();
pr_err("no context found for minor %d\n", subminor);
retval = -ENODEV;
goto exit;
}
+ rcu_read_unlock();
mutex_lock(&ictx->lock);
@@ -533,8 +550,10 @@ static int display_open(struct inode *inode, struct file *file)
mutex_unlock(&ictx->lock);
+ if (retval && refcount_dec_and_test(&ictx->users))
+ free_imon_context(ictx);
+
exit:
- mutex_unlock(&driver_lock);
return retval;
}
@@ -544,16 +563,9 @@ static int display_open(struct inode *inode, struct file *file)
*/
static int display_close(struct inode *inode, struct file *file)
{
- struct imon_context *ictx = NULL;
+ struct imon_context *ictx = file->private_data;
int retval = 0;
- ictx = file->private_data;
-
- if (!ictx) {
- pr_err("no context for device\n");
- return -ENODEV;
- }
-
mutex_lock(&ictx->lock);
if (!ictx->display_supported) {
@@ -568,6 +580,8 @@ static int display_close(struct inode *inode, struct file *file)
}
mutex_unlock(&ictx->lock);
+ if (refcount_dec_and_test(&ictx->users))
+ free_imon_context(ictx);
return retval;
}
@@ -934,15 +948,12 @@ static ssize_t vfd_write(struct file *file, const char __user *buf,
int offset;
int seq;
int retval = 0;
- struct imon_context *ictx;
+ struct imon_context *ictx = file->private_data;
static const unsigned char vfd_packet6[] = {
0x01, 0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF };
- ictx = file->private_data;
- if (!ictx) {
- pr_err_ratelimited("no context for device\n");
+ if (ictx->disconnected)
return -ENODEV;
- }
mutex_lock(&ictx->lock);
@@ -1018,13 +1029,10 @@ static ssize_t lcd_write(struct file *file, const char __user *buf,
size_t n_bytes, loff_t *pos)
{
int retval = 0;
- struct imon_context *ictx;
+ struct imon_context *ictx = file->private_data;
- ictx = file->private_data;
- if (!ictx) {
- pr_err_ratelimited("no context for device\n");
+ if (ictx->disconnected)
return -ENODEV;
- }
mutex_lock(&ictx->lock);
@@ -2404,7 +2412,6 @@ static int imon_probe(struct usb_interface *interface,
int ifnum, sysfs_err;
int ret = 0;
struct imon_context *ictx = NULL;
- struct imon_context *first_if_ctx = NULL;
u16 vendor, product;
usbdev = usb_get_dev(interface_to_usbdev(interface));
@@ -2416,17 +2423,12 @@ static int imon_probe(struct usb_interface *interface,
dev_dbg(dev, "%s: found iMON device (%04x:%04x, intf%d)\n",
__func__, vendor, product, ifnum);
- /* prevent races probing devices w/multiple interfaces */
- mutex_lock(&driver_lock);
-
first_if = usb_ifnum_to_if(usbdev, 0);
if (!first_if) {
ret = -ENODEV;
goto fail;
}
- first_if_ctx = usb_get_intfdata(first_if);
-
if (ifnum == 0) {
ictx = imon_init_intf0(interface, id);
if (!ictx) {
@@ -2434,9 +2436,11 @@ static int imon_probe(struct usb_interface *interface,
ret = -ENODEV;
goto fail;
}
+ refcount_set(&ictx->users, 1);
} else {
/* this is the secondary interface on the device */
+ struct imon_context *first_if_ctx = usb_get_intfdata(first_if);
/* fail early if first intf failed to register */
if (!first_if_ctx) {
@@ -2450,14 +2454,13 @@ static int imon_probe(struct usb_interface *interface,
ret = -ENODEV;
goto fail;
}
+ refcount_inc(&ictx->users);
}
usb_set_intfdata(interface, ictx);
if (ifnum == 0) {
- mutex_lock(&ictx->lock);
-
if (product == 0xffdc && ictx->rf_device) {
sysfs_err = sysfs_create_group(&interface->dev.kobj,
&imon_rf_attr_group);
@@ -2468,21 +2471,17 @@ static int imon_probe(struct usb_interface *interface,
if (ictx->display_supported)
imon_init_display(ictx, interface);
-
- mutex_unlock(&ictx->lock);
}
dev_info(dev, "iMON device (%04x:%04x, intf%d) on usb<%d:%d> initialized\n",
vendor, product, ifnum,
usbdev->bus->busnum, usbdev->devnum);
- mutex_unlock(&driver_lock);
usb_put_dev(usbdev);
return 0;
fail:
- mutex_unlock(&driver_lock);
usb_put_dev(usbdev);
dev_err(dev, "unable to register, err %d\n", ret);
@@ -2498,10 +2497,8 @@ static void imon_disconnect(struct usb_interface *interface)
struct device *dev;
int ifnum;
- /* prevent races with multi-interface device probing and display_open */
- mutex_lock(&driver_lock);
-
ictx = usb_get_intfdata(interface);
+ ictx->disconnected = true;
dev = ictx->dev;
ifnum = interface->cur_altsetting->desc.bInterfaceNumber;
@@ -2542,11 +2539,9 @@ static void imon_disconnect(struct usb_interface *interface)
}
}
- if (!ictx->dev_present_intf0 && !ictx->dev_present_intf1)
+ if (refcount_dec_and_test(&ictx->users))
free_imon_context(ictx);
- mutex_unlock(&driver_lock);
-
dev_dbg(dev, "%s: iMON device (intf%d) disconnected\n",
__func__, ifnum);
}