diff options
author | Eric W. Biederman <ebiederm@xmission.com> | 2010-01-05 20:56:02 -0500 |
---|---|---|
committer | Dmitry Torokhov <dmitry.torokhov@gmail.com> | 2010-01-06 03:14:32 -0500 |
commit | 59b015133cd0034f5904a76969d73476380aac46 (patch) | |
tree | 578643cc919b7e62b5086718d5c3f9b0fee836a9 /drivers | |
parent | abf2a117c67a67fbb611913a31109d0ff66ab073 (diff) |
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>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/input/keyboard/atkbd.c | 59 | ||||
-rw-r--r-- | drivers/input/mouse/psmouse-base.c | 32 |
2 files changed, 28 insertions, 63 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 { | |||
225 | 225 | ||
226 | struct delayed_work event_work; | 226 | struct delayed_work event_work; |
227 | unsigned long event_jiffies; | 227 | unsigned long event_jiffies; |
228 | struct mutex event_mutex; | ||
229 | unsigned long event_mask; | 228 | unsigned long event_mask; |
229 | |||
230 | /* Serializes reconnect(), attr->set() and event work */ | ||
231 | struct mutex mutex; | ||
230 | }; | 232 | }; |
231 | 233 | ||
232 | /* | 234 | /* |
@@ -577,7 +579,7 @@ static void atkbd_event_work(struct work_struct *work) | |||
577 | { | 579 | { |
578 | struct atkbd *atkbd = container_of(work, struct atkbd, event_work.work); | 580 | struct atkbd *atkbd = container_of(work, struct atkbd, event_work.work); |
579 | 581 | ||
580 | mutex_lock(&atkbd->event_mutex); | 582 | mutex_lock(&atkbd->mutex); |
581 | 583 | ||
582 | if (!atkbd->enabled) { | 584 | if (!atkbd->enabled) { |
583 | /* | 585 | /* |
@@ -596,7 +598,7 @@ static void atkbd_event_work(struct work_struct *work) | |||
596 | atkbd_set_repeat_rate(atkbd); | 598 | atkbd_set_repeat_rate(atkbd); |
597 | } | 599 | } |
598 | 600 | ||
599 | mutex_unlock(&atkbd->event_mutex); | 601 | mutex_unlock(&atkbd->mutex); |
600 | } | 602 | } |
601 | 603 | ||
602 | /* | 604 | /* |
@@ -612,7 +614,7 @@ static void atkbd_schedule_event_work(struct atkbd *atkbd, int event_bit) | |||
612 | 614 | ||
613 | atkbd->event_jiffies = jiffies; | 615 | atkbd->event_jiffies = jiffies; |
614 | set_bit(event_bit, &atkbd->event_mask); | 616 | set_bit(event_bit, &atkbd->event_mask); |
615 | wmb(); | 617 | mb(); |
616 | schedule_delayed_work(&atkbd->event_work, delay); | 618 | schedule_delayed_work(&atkbd->event_work, delay); |
617 | } | 619 | } |
618 | 620 | ||
@@ -849,12 +851,13 @@ static void atkbd_disconnect(struct serio *serio) | |||
849 | { | 851 | { |
850 | struct atkbd *atkbd = serio_get_drvdata(serio); | 852 | struct atkbd *atkbd = serio_get_drvdata(serio); |
851 | 853 | ||
854 | sysfs_remove_group(&serio->dev.kobj, &atkbd_attribute_group); | ||
855 | |||
852 | atkbd_disable(atkbd); | 856 | atkbd_disable(atkbd); |
853 | 857 | ||
854 | /* make sure we don't have a command in flight */ | 858 | /* make sure we don't have a command in flight */ |
855 | cancel_delayed_work_sync(&atkbd->event_work); | 859 | cancel_delayed_work_sync(&atkbd->event_work); |
856 | 860 | ||
857 | sysfs_remove_group(&serio->dev.kobj, &atkbd_attribute_group); | ||
858 | input_unregister_device(atkbd->dev); | 861 | input_unregister_device(atkbd->dev); |
859 | serio_close(serio); | 862 | serio_close(serio); |
860 | serio_set_drvdata(serio, NULL); | 863 | serio_set_drvdata(serio, NULL); |
@@ -1087,7 +1090,7 @@ static int atkbd_connect(struct serio *serio, struct serio_driver *drv) | |||
1087 | atkbd->dev = dev; | 1090 | atkbd->dev = dev; |
1088 | ps2_init(&atkbd->ps2dev, serio); | 1091 | ps2_init(&atkbd->ps2dev, serio); |
1089 | INIT_DELAYED_WORK(&atkbd->event_work, atkbd_event_work); | 1092 | INIT_DELAYED_WORK(&atkbd->event_work, atkbd_event_work); |
1090 | mutex_init(&atkbd->event_mutex); | 1093 | mutex_init(&atkbd->mutex); |
1091 | 1094 | ||
1092 | switch (serio->id.type) { | 1095 | switch (serio->id.type) { |
1093 | 1096 | ||
@@ -1160,19 +1163,23 @@ static int atkbd_reconnect(struct serio *serio) | |||
1160 | { | 1163 | { |
1161 | struct atkbd *atkbd = serio_get_drvdata(serio); | 1164 | struct atkbd *atkbd = serio_get_drvdata(serio); |
1162 | struct serio_driver *drv = serio->drv; | 1165 | struct serio_driver *drv = serio->drv; |
1166 | int retval = -1; | ||
1163 | 1167 | ||
1164 | if (!atkbd || !drv) { | 1168 | if (!atkbd || !drv) { |
1165 | printk(KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n"); | 1169 | printk(KERN_DEBUG "atkbd: reconnect request, but serio is disconnected, ignoring...\n"); |
1166 | return -1; | 1170 | return -1; |
1167 | } | 1171 | } |
1168 | 1172 | ||
1173 | mutex_lock(&atkbd->mutex); | ||
1174 | |||
1169 | atkbd_disable(atkbd); | 1175 | atkbd_disable(atkbd); |
1170 | 1176 | ||
1171 | if (atkbd->write) { | 1177 | if (atkbd->write) { |
1172 | if (atkbd_probe(atkbd)) | 1178 | if (atkbd_probe(atkbd)) |
1173 | return -1; | 1179 | goto out; |
1180 | |||
1174 | if (atkbd->set != atkbd_select_set(atkbd, atkbd->set, atkbd->extra)) | 1181 | if (atkbd->set != atkbd_select_set(atkbd, atkbd->set, atkbd->extra)) |
1175 | return -1; | 1182 | goto out; |
1176 | 1183 | ||
1177 | atkbd_activate(atkbd); | 1184 | atkbd_activate(atkbd); |
1178 | 1185 | ||
@@ -1190,8 +1197,11 @@ static int atkbd_reconnect(struct serio *serio) | |||
1190 | } | 1197 | } |
1191 | 1198 | ||
1192 | atkbd_enable(atkbd); | 1199 | atkbd_enable(atkbd); |
1200 | retval = 0; | ||
1193 | 1201 | ||
1194 | return 0; | 1202 | out: |
1203 | mutex_unlock(&atkbd->mutex); | ||
1204 | return retval; | ||
1195 | } | 1205 | } |
1196 | 1206 | ||
1197 | static struct serio_device_id atkbd_serio_ids[] = { | 1207 | static struct serio_device_id atkbd_serio_ids[] = { |
@@ -1235,47 +1245,28 @@ static ssize_t atkbd_attr_show_helper(struct device *dev, char *buf, | |||
1235 | ssize_t (*handler)(struct atkbd *, char *)) | 1245 | ssize_t (*handler)(struct atkbd *, char *)) |
1236 | { | 1246 | { |
1237 | struct serio *serio = to_serio_port(dev); | 1247 | struct serio *serio = to_serio_port(dev); |
1238 | int retval; | 1248 | struct atkbd *atkbd = serio_get_drvdata(serio); |
1239 | |||
1240 | retval = serio_pin_driver(serio); | ||
1241 | if (retval) | ||
1242 | return retval; | ||
1243 | |||
1244 | if (serio->drv != &atkbd_drv) { | ||
1245 | retval = -ENODEV; | ||
1246 | goto out; | ||
1247 | } | ||
1248 | |||
1249 | retval = handler((struct atkbd *)serio_get_drvdata(serio), buf); | ||
1250 | 1249 | ||
1251 | out: | 1250 | return handler(atkbd, buf); |
1252 | serio_unpin_driver(serio); | ||
1253 | return retval; | ||
1254 | } | 1251 | } |
1255 | 1252 | ||
1256 | static ssize_t atkbd_attr_set_helper(struct device *dev, const char *buf, size_t count, | 1253 | static ssize_t atkbd_attr_set_helper(struct device *dev, const char *buf, size_t count, |
1257 | ssize_t (*handler)(struct atkbd *, const char *, size_t)) | 1254 | ssize_t (*handler)(struct atkbd *, const char *, size_t)) |
1258 | { | 1255 | { |
1259 | struct serio *serio = to_serio_port(dev); | 1256 | struct serio *serio = to_serio_port(dev); |
1260 | struct atkbd *atkbd; | 1257 | struct atkbd *atkbd = serio_get_drvdata(serio); |
1261 | int retval; | 1258 | int retval; |
1262 | 1259 | ||
1263 | retval = serio_pin_driver(serio); | 1260 | retval = mutex_lock_interruptible(&atkbd->mutex); |
1264 | if (retval) | 1261 | if (retval) |
1265 | return retval; | 1262 | return retval; |
1266 | 1263 | ||
1267 | if (serio->drv != &atkbd_drv) { | ||
1268 | retval = -ENODEV; | ||
1269 | goto out; | ||
1270 | } | ||
1271 | |||
1272 | atkbd = serio_get_drvdata(serio); | ||
1273 | atkbd_disable(atkbd); | 1264 | atkbd_disable(atkbd); |
1274 | retval = handler(atkbd, buf, count); | 1265 | retval = handler(atkbd, buf, count); |
1275 | atkbd_enable(atkbd); | 1266 | atkbd_enable(atkbd); |
1276 | 1267 | ||
1277 | out: | 1268 | mutex_unlock(&atkbd->mutex); |
1278 | serio_unpin_driver(serio); | 1269 | |
1279 | return retval; | 1270 | return retval; |
1280 | } | 1271 | } |
1281 | 1272 | ||
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c index 401ac6b6edd4..d59e18b24ede 100644 --- a/drivers/input/mouse/psmouse-base.c +++ b/drivers/input/mouse/psmouse-base.c | |||
@@ -1450,24 +1450,10 @@ ssize_t psmouse_attr_show_helper(struct device *dev, struct device_attribute *de | |||
1450 | struct serio *serio = to_serio_port(dev); | 1450 | struct serio *serio = to_serio_port(dev); |
1451 | struct psmouse_attribute *attr = to_psmouse_attr(devattr); | 1451 | struct psmouse_attribute *attr = to_psmouse_attr(devattr); |
1452 | struct psmouse *psmouse; | 1452 | struct psmouse *psmouse; |
1453 | int retval; | ||
1454 | |||
1455 | retval = serio_pin_driver(serio); | ||
1456 | if (retval) | ||
1457 | return retval; | ||
1458 | |||
1459 | if (serio->drv != &psmouse_drv) { | ||
1460 | retval = -ENODEV; | ||
1461 | goto out; | ||
1462 | } | ||
1463 | 1453 | ||
1464 | psmouse = serio_get_drvdata(serio); | 1454 | psmouse = serio_get_drvdata(serio); |
1465 | 1455 | ||
1466 | retval = attr->show(psmouse, attr->data, buf); | 1456 | return attr->show(psmouse, attr->data, buf); |
1467 | |||
1468 | out: | ||
1469 | serio_unpin_driver(serio); | ||
1470 | return retval; | ||
1471 | } | 1457 | } |
1472 | 1458 | ||
1473 | ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *devattr, | 1459 | ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *devattr, |
@@ -1478,18 +1464,9 @@ ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *dev | |||
1478 | struct psmouse *psmouse, *parent = NULL; | 1464 | struct psmouse *psmouse, *parent = NULL; |
1479 | int retval; | 1465 | int retval; |
1480 | 1466 | ||
1481 | retval = serio_pin_driver(serio); | ||
1482 | if (retval) | ||
1483 | return retval; | ||
1484 | |||
1485 | if (serio->drv != &psmouse_drv) { | ||
1486 | retval = -ENODEV; | ||
1487 | goto out_unpin; | ||
1488 | } | ||
1489 | |||
1490 | retval = mutex_lock_interruptible(&psmouse_mutex); | 1467 | retval = mutex_lock_interruptible(&psmouse_mutex); |
1491 | if (retval) | 1468 | if (retval) |
1492 | goto out_unpin; | 1469 | goto out; |
1493 | 1470 | ||
1494 | psmouse = serio_get_drvdata(serio); | 1471 | psmouse = serio_get_drvdata(serio); |
1495 | 1472 | ||
@@ -1519,8 +1496,7 @@ ssize_t psmouse_attr_set_helper(struct device *dev, struct device_attribute *dev | |||
1519 | 1496 | ||
1520 | out_unlock: | 1497 | out_unlock: |
1521 | mutex_unlock(&psmouse_mutex); | 1498 | mutex_unlock(&psmouse_mutex); |
1522 | out_unpin: | 1499 | out: |
1523 | serio_unpin_driver(serio); | ||
1524 | return retval; | 1500 | return retval; |
1525 | } | 1501 | } |
1526 | 1502 | ||
@@ -1582,9 +1558,7 @@ static ssize_t psmouse_attr_set_protocol(struct psmouse *psmouse, void *data, co | |||
1582 | } | 1558 | } |
1583 | 1559 | ||
1584 | mutex_unlock(&psmouse_mutex); | 1560 | mutex_unlock(&psmouse_mutex); |
1585 | serio_unpin_driver(serio); | ||
1586 | serio_unregister_child_port(serio); | 1561 | serio_unregister_child_port(serio); |
1587 | serio_pin_driver_uninterruptible(serio); | ||
1588 | mutex_lock(&psmouse_mutex); | 1562 | mutex_lock(&psmouse_mutex); |
1589 | 1563 | ||
1590 | if (serio->drv != &psmouse_drv) { | 1564 | if (serio->drv != &psmouse_drv) { |