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>

1 files changed, 25 insertions, 34 deletions

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 1f5e2ce327d6..1cf32a7814d0 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -225,8 +225,10 @@ struct atkbd {
 
         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 @@ static void atkbd_event_work(struct work_struct *work)
 {
         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 @@ static void atkbd_event_work(struct work_struct *work)
                         atkbd_set_repeat_rate(atkbd);
         }
 
-        mutex_unlock(&atkbd->event_mutex);
+        mutex_unlock(&atkbd->mutex);
 }
 
 /*
@@ -612,7 +614,7 @@ static void atkbd_schedule_event_work(struct atkbd *atkbd, int event_bit)
 
         atkbd->event_jiffies = jiffies;
         set_bit(event_bit, &atkbd->event_mask);
-        wmb();
+        mb();
         schedule_delayed_work(&atkbd->event_work, delay);
 }
 
@@ -849,12 +851,13 @@ static void atkbd_disconnect(struct serio *serio)
 {
         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 @@ static int atkbd_connect(struct serio *serio, struct serio_driver *drv)
         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 @@ static int atkbd_reconnect(struct serio *serio)
 {
         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 @@ static int atkbd_reconnect(struct serio *serio)
         }
 
         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 @@ static ssize_t atkbd_attr_show_helper(struct device *dev, char *buf,
                                 ssize_t (*handler)(struct atkbd *, char *))
 {
         struct serio *serio = to_serio_port(dev);
-        int retval;
-
-        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);
+        struct atkbd *atkbd = serio_get_drvdata(serio);
 
-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;
 }