Input: serio - fix potential deadlock when unbinding drivers
sysfs_remove_group() waits for sysfs attributes to be removed, therefore
we do not need to worry about driver-specific attributes being accessed
after driver has been detached from the device. In fact, attempts to take
serio->drv_mutex in attribute methods may lead to the following deadlock:
sysfs_read_file()
fill_read_buffer()
sysfs_get_active_two()
psmouse_attr_show_helper()
serio_pin_driver()
serio_disconnect_driver()
mutex_lock(&serio->drv_mutex);
<--------> mutex_lock(&serio_drv_mutex);
psmouse_disconnect()
sysfs_remove_group(... psmouse_attr_group);
....
sysfs_deactivate();
wait_for_completion();
Fix this by removing calls to serio_[un]pin_driver() and functions themselves
and using driver-private mutexes to serialize access to attribute's set()
methods that may change device state.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 1f5e2ce..1cf32a7 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -225,8 +225,10 @@
struct delayed_work event_work;
unsigned long event_jiffies;
- struct mutex event_mutex;
unsigned long event_mask;
+
+ /* Serializes reconnect(), attr->set() and event work */
+ struct mutex mutex;
};
/*
@@ -577,7 +579,7 @@
{
struct atkbd *atkbd = container_of(work, struct atkbd, event_work.work);
- mutex_lock(&atkbd->event_mutex);
+ mutex_lock(&atkbd->mutex);
if (!atkbd->enabled) {
/*
@@ -596,7 +598,7 @@
atkbd_set_repeat_rate(atkbd);
}
- mutex_unlock(&atkbd->event_mutex);
+ mutex_unlock(&atkbd->mutex);
}
/*
@@ -612,7 +614,7 @@
atkbd->event_jiffies = jiffies;
set_bit(event_bit, &atkbd->event_mask);
- wmb();
+ mb();
schedule_delayed_work(&atkbd->event_work, delay);
}
@@ -849,12 +851,13 @@
{
struct atkbd *atkbd = serio_get_drvdata(serio);
+ sysfs_remove_group(&serio->dev.kobj, &atkbd_attribute_group);
+
atkbd_disable(atkbd);
/* make sure we don't have a command in flight */
cancel_delayed_work_sync(&atkbd->event_work);
- sysfs_remove_group(&serio->dev.kobj, &atkbd_attribute_group);
input_unregister_device(atkbd->dev);
serio_close(serio);
serio_set_drvdata(serio, NULL);
@@ -1087,7 +1090,7 @@
atkbd->dev = dev;
ps2_init(&atkbd->ps2dev, serio);
INIT_DELAYED_WORK(&atkbd->event_work, atkbd_event_work);
- mutex_init(&atkbd->event_mutex);
+ mutex_init(&atkbd->mutex);
switch (serio->id.type) {
@@ -1160,19 +1163,23 @@
{
struct atkbd *atkbd = serio_get_drvdata(serio);
struct serio_driver *drv = serio->drv;
+ int retval = -1;
if (!atkbd || !drv) {
printk(KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n");
return -1;
}
+ mutex_lock(&atkbd->mutex);
+
atkbd_disable(atkbd);
if (atkbd->write) {
if (atkbd_probe(atkbd))
- return -1;
+ goto out;
+
if (atkbd->set != atkbd_select_set(atkbd, atkbd->set, atkbd->extra))
- return -1;
+ goto out;
atkbd_activate(atkbd);
@@ -1190,8 +1197,11 @@
}
atkbd_enable(atkbd);
+ retval = 0;
- return 0;
+ out:
+ mutex_unlock(&atkbd->mutex);
+ return retval;
}
static struct serio_device_id atkbd_serio_ids[] = {
@@ -1235,47 +1245,28 @@
ssize_t (*handler)(struct atkbd *, char *))
{
struct serio *serio = to_serio_port(dev);
- int retval;
+ struct atkbd *atkbd = serio_get_drvdata(serio);
- retval = serio_pin_driver(serio);
- if (retval)
- return retval;
-
- if (serio->drv != &atkbd_drv) {
- retval = -ENODEV;
- goto out;
- }
-
- retval = handler((struct atkbd *)serio_get_drvdata(serio), buf);
-
-out:
- serio_unpin_driver(serio);
- return retval;
+ return handler(atkbd, buf);
}
static ssize_t atkbd_attr_set_helper(struct device *dev, const char *buf, size_t count,
ssize_t (*handler)(struct atkbd *, const char *, size_t))
{
struct serio *serio = to_serio_port(dev);
- struct atkbd *atkbd;
+ struct atkbd *atkbd = serio_get_drvdata(serio);
int retval;
- retval = serio_pin_driver(serio);
+ retval = mutex_lock_interruptible(&atkbd->mutex);
if (retval)
return retval;
- if (serio->drv != &atkbd_drv) {
- retval = -ENODEV;
- goto out;
- }
-
- atkbd = serio_get_drvdata(serio);
atkbd_disable(atkbd);
retval = handler(atkbd, buf, count);
atkbd_enable(atkbd);
-out:
- serio_unpin_driver(serio);
+ mutex_unlock(&atkbd->mutex);
+
return retval;
}
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 401ac6b..d59e18b 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -1450,24 +1450,10 @@
struct serio *serio = to_serio_port(dev);
struct psmouse_attribute *attr = to_psmouse_attr(devattr);
struct psmouse *psmouse;
- int retval;
-
- retval = serio_pin_driver(serio);
- if (retval)
- return retval;
-
- if (serio->drv != &psmouse_drv) {
- retval = -ENODEV;
- goto out;
- }
psmouse = serio_get_drvdata(serio);
- retval = attr->show(psmouse, attr->data, buf);
-
-out:
- serio_unpin_driver(serio);
- return retval;
+ return attr->show(psmouse, attr->data, buf);
}
ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *devattr,
@@ -1478,18 +1464,9 @@
struct psmouse *psmouse, *parent = NULL;
int retval;
- retval = serio_pin_driver(serio);
- if (retval)
- return retval;
-
- if (serio->drv != &psmouse_drv) {
- retval = -ENODEV;
- goto out_unpin;
- }
-
retval = mutex_lock_interruptible(&psmouse_mutex);
if (retval)
- goto out_unpin;
+ goto out;
psmouse = serio_get_drvdata(serio);
@@ -1519,8 +1496,7 @@
out_unlock:
mutex_unlock(&psmouse_mutex);
- out_unpin:
- serio_unpin_driver(serio);
+ out:
return retval;
}
@@ -1582,9 +1558,7 @@
}
mutex_unlock(&psmouse_mutex);
- serio_unpin_driver(serio);
serio_unregister_child_port(serio);
- serio_pin_driver_uninterruptible(serio);
mutex_lock(&psmouse_mutex);
if (serio->drv != &psmouse_drv) {
diff --git a/include/linux/serio.h b/include/linux/serio.h
index e2f3044..813d26c 100644
--- a/include/linux/serio.h
+++ b/include/linux/serio.h
@@ -136,25 +136,6 @@
spin_unlock_irq(&serio->lock);
}
-/*
- * Use the following functions to pin serio's driver in process context
- */
-static inline int serio_pin_driver(struct serio *serio)
-{
- return mutex_lock_interruptible(&serio->drv_mutex);
-}
-
-static inline void serio_pin_driver_uninterruptible(struct serio *serio)
-{
- mutex_lock(&serio->drv_mutex);
-}
-
-static inline void serio_unpin_driver(struct serio *serio)
-{
- mutex_unlock(&serio->drv_mutex);
-}
-
-
#endif
/*