diff options
author | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2006-10-22 10:16:27 -0400 |
---|---|---|
committer | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2006-12-07 15:32:10 -0500 |
commit | b07375b155a0d2ed21a64db68e737da1f19385f7 (patch) | |
tree | 06669d571a153c7692ca9ed3f18c3686613cb231 /drivers/ieee1394 | |
parent | 7fdfc90945e308dc1be37e3914cd979a535263e9 (diff) |
ieee1394: nodemgr: revise semaphore protection of driver core data
- The list "struct class.children" is supposed to be protected by
class.sem, not by class.subsys.rwsem.
- nodemgr_remove_uds() iterated over nodemgr_ud_class.children without
proper protection. This was never observed as a bug since the code
is usually only accessed by knodemgrd. All knodemgrds are currently
globally serialized. But userspace can trigger this code too by
writing to /sys/bus/ieee1394/destroy_node.
- Clean up access to the FireWire bus type's subsys.rwsem: Access it
uniformly via ieee1394_bus_type. Shrink rwsem protected regions
where possible. Expand them where necessary. The latter wasn't a
problem so far because knodemgr is globally serialized.
This should harden the interaction of ieee1394 with sysfs and lay ground
for deserialized operation of multiple knodemgrds and for implementation
of subthreads for parallelized scanning and probing.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Diffstat (limited to 'drivers/ieee1394')
-rw-r--r-- | drivers/ieee1394/nodemgr.c | 142 |
1 files changed, 92 insertions, 50 deletions
diff --git a/drivers/ieee1394/nodemgr.c b/drivers/ieee1394/nodemgr.c index bffa26e48152..e2ca8e92f43f 100644 --- a/drivers/ieee1394/nodemgr.c +++ b/drivers/ieee1394/nodemgr.c | |||
@@ -374,11 +374,11 @@ static ssize_t fw_set_ignore_driver(struct device *dev, struct device_attribute | |||
374 | int state = simple_strtoul(buf, NULL, 10); | 374 | int state = simple_strtoul(buf, NULL, 10); |
375 | 375 | ||
376 | if (state == 1) { | 376 | if (state == 1) { |
377 | down_write(&dev->bus->subsys.rwsem); | ||
378 | device_release_driver(dev); | ||
379 | ud->ignore_driver = 1; | 377 | ud->ignore_driver = 1; |
380 | up_write(&dev->bus->subsys.rwsem); | 378 | down_write(&ieee1394_bus_type.subsys.rwsem); |
381 | } else if (!state) | 379 | device_release_driver(dev); |
380 | up_write(&ieee1394_bus_type.subsys.rwsem); | ||
381 | } else if (state == 0) | ||
382 | ud->ignore_driver = 0; | 382 | ud->ignore_driver = 0; |
383 | 383 | ||
384 | return count; | 384 | return count; |
@@ -436,7 +436,7 @@ static ssize_t fw_set_ignore_drivers(struct bus_type *bus, const char *buf, size | |||
436 | 436 | ||
437 | if (state == 1) | 437 | if (state == 1) |
438 | ignore_drivers = 1; | 438 | ignore_drivers = 1; |
439 | else if (!state) | 439 | else if (state == 0) |
440 | ignore_drivers = 0; | 440 | ignore_drivers = 0; |
441 | 441 | ||
442 | return count; | 442 | return count; |
@@ -734,20 +734,65 @@ static int nodemgr_bus_match(struct device * dev, struct device_driver * drv) | |||
734 | } | 734 | } |
735 | 735 | ||
736 | 736 | ||
737 | static DEFINE_MUTEX(nodemgr_serialize_remove_uds); | ||
738 | |||
737 | static void nodemgr_remove_uds(struct node_entry *ne) | 739 | static void nodemgr_remove_uds(struct node_entry *ne) |
738 | { | 740 | { |
739 | struct class_device *cdev, *next; | 741 | struct class_device *cdev; |
740 | struct unit_directory *ud; | 742 | struct unit_directory *ud, **unreg; |
743 | size_t i, count; | ||
744 | |||
745 | /* | ||
746 | * This is awkward: | ||
747 | * Iteration over nodemgr_ud_class.children has to be protected by | ||
748 | * nodemgr_ud_class.sem, but class_device_unregister() will eventually | ||
749 | * take nodemgr_ud_class.sem too. Therefore store all uds to be | ||
750 | * unregistered in a temporary array, release the semaphore, and then | ||
751 | * unregister the uds. | ||
752 | * | ||
753 | * Since nodemgr_remove_uds can also run in other contexts than the | ||
754 | * knodemgrds (which are currently globally serialized), protect the | ||
755 | * gap after release of the semaphore by nodemgr_serialize_remove_uds. | ||
756 | */ | ||
741 | 757 | ||
742 | list_for_each_entry_safe(cdev, next, &nodemgr_ud_class.children, node) { | 758 | mutex_lock(&nodemgr_serialize_remove_uds); |
743 | ud = container_of(cdev, struct unit_directory, class_dev); | ||
744 | 759 | ||
745 | if (ud->ne != ne) | 760 | down(&nodemgr_ud_class.sem); |
746 | continue; | 761 | count = 0; |
762 | list_for_each_entry(cdev, &nodemgr_ud_class.children, node) { | ||
763 | ud = container_of(cdev, struct unit_directory, class_dev); | ||
764 | if (ud->ne == ne) | ||
765 | count++; | ||
766 | } | ||
767 | if (!count) { | ||
768 | up(&nodemgr_ud_class.sem); | ||
769 | mutex_unlock(&nodemgr_serialize_remove_uds); | ||
770 | return; | ||
771 | } | ||
772 | unreg = kcalloc(count, sizeof(*unreg), GFP_KERNEL); | ||
773 | if (!unreg) { | ||
774 | HPSB_ERR("NodeMgr: out of memory in nodemgr_remove_uds"); | ||
775 | up(&nodemgr_ud_class.sem); | ||
776 | mutex_unlock(&nodemgr_serialize_remove_uds); | ||
777 | return; | ||
778 | } | ||
779 | i = 0; | ||
780 | list_for_each_entry(cdev, &nodemgr_ud_class.children, node) { | ||
781 | ud = container_of(cdev, struct unit_directory, class_dev); | ||
782 | if (ud->ne == ne) { | ||
783 | BUG_ON(i >= count); | ||
784 | unreg[i++] = ud; | ||
785 | } | ||
786 | } | ||
787 | up(&nodemgr_ud_class.sem); | ||
747 | 788 | ||
748 | class_device_unregister(&ud->class_dev); | 789 | for (i = 0; i < count; i++) { |
749 | device_unregister(&ud->device); | 790 | class_device_unregister(&unreg[i]->class_dev); |
791 | device_unregister(&unreg[i]->device); | ||
750 | } | 792 | } |
793 | kfree(unreg); | ||
794 | |||
795 | mutex_unlock(&nodemgr_serialize_remove_uds); | ||
751 | } | 796 | } |
752 | 797 | ||
753 | 798 | ||
@@ -880,12 +925,11 @@ fail_alloc: | |||
880 | 925 | ||
881 | static struct node_entry *find_entry_by_guid(u64 guid) | 926 | static struct node_entry *find_entry_by_guid(u64 guid) |
882 | { | 927 | { |
883 | struct class *class = &nodemgr_ne_class; | ||
884 | struct class_device *cdev; | 928 | struct class_device *cdev; |
885 | struct node_entry *ne, *ret_ne = NULL; | 929 | struct node_entry *ne, *ret_ne = NULL; |
886 | 930 | ||
887 | down_read(&class->subsys.rwsem); | 931 | down(&nodemgr_ne_class.sem); |
888 | list_for_each_entry(cdev, &class->children, node) { | 932 | list_for_each_entry(cdev, &nodemgr_ne_class.children, node) { |
889 | ne = container_of(cdev, struct node_entry, class_dev); | 933 | ne = container_of(cdev, struct node_entry, class_dev); |
890 | 934 | ||
891 | if (ne->guid == guid) { | 935 | if (ne->guid == guid) { |
@@ -893,20 +937,20 @@ static struct node_entry *find_entry_by_guid(u64 guid) | |||
893 | break; | 937 | break; |
894 | } | 938 | } |
895 | } | 939 | } |
896 | up_read(&class->subsys.rwsem); | 940 | up(&nodemgr_ne_class.sem); |
897 | 941 | ||
898 | return ret_ne; | 942 | return ret_ne; |
899 | } | 943 | } |
900 | 944 | ||
901 | 945 | ||
902 | static struct node_entry *find_entry_by_nodeid(struct hpsb_host *host, nodeid_t nodeid) | 946 | static struct node_entry *find_entry_by_nodeid(struct hpsb_host *host, |
947 | nodeid_t nodeid) | ||
903 | { | 948 | { |
904 | struct class *class = &nodemgr_ne_class; | ||
905 | struct class_device *cdev; | 949 | struct class_device *cdev; |
906 | struct node_entry *ne, *ret_ne = NULL; | 950 | struct node_entry *ne, *ret_ne = NULL; |
907 | 951 | ||
908 | down_read(&class->subsys.rwsem); | 952 | down(&nodemgr_ne_class.sem); |
909 | list_for_each_entry(cdev, &class->children, node) { | 953 | list_for_each_entry(cdev, &nodemgr_ne_class.children, node) { |
910 | ne = container_of(cdev, struct node_entry, class_dev); | 954 | ne = container_of(cdev, struct node_entry, class_dev); |
911 | 955 | ||
912 | if (ne->host == host && ne->nodeid == nodeid) { | 956 | if (ne->host == host && ne->nodeid == nodeid) { |
@@ -914,7 +958,7 @@ static struct node_entry *find_entry_by_nodeid(struct hpsb_host *host, nodeid_t | |||
914 | break; | 958 | break; |
915 | } | 959 | } |
916 | } | 960 | } |
917 | up_read(&class->subsys.rwsem); | 961 | up(&nodemgr_ne_class.sem); |
918 | 962 | ||
919 | return ret_ne; | 963 | return ret_ne; |
920 | } | 964 | } |
@@ -1377,7 +1421,6 @@ static void nodemgr_node_scan(struct host_info *hi, int generation) | |||
1377 | } | 1421 | } |
1378 | 1422 | ||
1379 | 1423 | ||
1380 | /* Caller needs to hold nodemgr_ud_class.subsys.rwsem as reader. */ | ||
1381 | static void nodemgr_suspend_ne(struct node_entry *ne) | 1424 | static void nodemgr_suspend_ne(struct node_entry *ne) |
1382 | { | 1425 | { |
1383 | struct class_device *cdev; | 1426 | struct class_device *cdev; |
@@ -1389,19 +1432,20 @@ static void nodemgr_suspend_ne(struct node_entry *ne) | |||
1389 | ne->in_limbo = 1; | 1432 | ne->in_limbo = 1; |
1390 | WARN_ON(device_create_file(&ne->device, &dev_attr_ne_in_limbo)); | 1433 | WARN_ON(device_create_file(&ne->device, &dev_attr_ne_in_limbo)); |
1391 | 1434 | ||
1392 | down_write(&ne->device.bus->subsys.rwsem); | 1435 | down(&nodemgr_ud_class.sem); |
1393 | list_for_each_entry(cdev, &nodemgr_ud_class.children, node) { | 1436 | list_for_each_entry(cdev, &nodemgr_ud_class.children, node) { |
1394 | ud = container_of(cdev, struct unit_directory, class_dev); | 1437 | ud = container_of(cdev, struct unit_directory, class_dev); |
1395 | |||
1396 | if (ud->ne != ne) | 1438 | if (ud->ne != ne) |
1397 | continue; | 1439 | continue; |
1398 | 1440 | ||
1441 | down_write(&ieee1394_bus_type.subsys.rwsem); | ||
1399 | if (ud->device.driver && | 1442 | if (ud->device.driver && |
1400 | (!ud->device.driver->suspend || | 1443 | (!ud->device.driver->suspend || |
1401 | ud->device.driver->suspend(&ud->device, PMSG_SUSPEND))) | 1444 | ud->device.driver->suspend(&ud->device, PMSG_SUSPEND))) |
1402 | device_release_driver(&ud->device); | 1445 | device_release_driver(&ud->device); |
1446 | up_write(&ieee1394_bus_type.subsys.rwsem); | ||
1403 | } | 1447 | } |
1404 | up_write(&ne->device.bus->subsys.rwsem); | 1448 | up(&nodemgr_ud_class.sem); |
1405 | } | 1449 | } |
1406 | 1450 | ||
1407 | 1451 | ||
@@ -1413,45 +1457,47 @@ static void nodemgr_resume_ne(struct node_entry *ne) | |||
1413 | ne->in_limbo = 0; | 1457 | ne->in_limbo = 0; |
1414 | device_remove_file(&ne->device, &dev_attr_ne_in_limbo); | 1458 | device_remove_file(&ne->device, &dev_attr_ne_in_limbo); |
1415 | 1459 | ||
1416 | down_read(&nodemgr_ud_class.subsys.rwsem); | 1460 | down(&nodemgr_ud_class.sem); |
1417 | down_read(&ne->device.bus->subsys.rwsem); | ||
1418 | list_for_each_entry(cdev, &nodemgr_ud_class.children, node) { | 1461 | list_for_each_entry(cdev, &nodemgr_ud_class.children, node) { |
1419 | ud = container_of(cdev, struct unit_directory, class_dev); | 1462 | ud = container_of(cdev, struct unit_directory, class_dev); |
1420 | |||
1421 | if (ud->ne != ne) | 1463 | if (ud->ne != ne) |
1422 | continue; | 1464 | continue; |
1423 | 1465 | ||
1466 | down_read(&ieee1394_bus_type.subsys.rwsem); | ||
1424 | if (ud->device.driver && ud->device.driver->resume) | 1467 | if (ud->device.driver && ud->device.driver->resume) |
1425 | ud->device.driver->resume(&ud->device); | 1468 | ud->device.driver->resume(&ud->device); |
1469 | up_read(&ieee1394_bus_type.subsys.rwsem); | ||
1426 | } | 1470 | } |
1427 | up_read(&ne->device.bus->subsys.rwsem); | 1471 | up(&nodemgr_ud_class.sem); |
1428 | up_read(&nodemgr_ud_class.subsys.rwsem); | ||
1429 | 1472 | ||
1430 | HPSB_DEBUG("Node resumed: ID:BUS[" NODE_BUS_FMT "] GUID[%016Lx]", | 1473 | HPSB_DEBUG("Node resumed: ID:BUS[" NODE_BUS_FMT "] GUID[%016Lx]", |
1431 | NODE_BUS_ARGS(ne->host, ne->nodeid), (unsigned long long)ne->guid); | 1474 | NODE_BUS_ARGS(ne->host, ne->nodeid), (unsigned long long)ne->guid); |
1432 | } | 1475 | } |
1433 | 1476 | ||
1434 | 1477 | ||
1435 | /* Caller needs to hold nodemgr_ud_class.subsys.rwsem as reader. */ | ||
1436 | static void nodemgr_update_pdrv(struct node_entry *ne) | 1478 | static void nodemgr_update_pdrv(struct node_entry *ne) |
1437 | { | 1479 | { |
1438 | struct unit_directory *ud; | 1480 | struct unit_directory *ud; |
1439 | struct hpsb_protocol_driver *pdrv; | 1481 | struct hpsb_protocol_driver *pdrv; |
1440 | struct class_device *cdev; | 1482 | struct class_device *cdev; |
1441 | 1483 | ||
1484 | down(&nodemgr_ud_class.sem); | ||
1442 | list_for_each_entry(cdev, &nodemgr_ud_class.children, node) { | 1485 | list_for_each_entry(cdev, &nodemgr_ud_class.children, node) { |
1443 | ud = container_of(cdev, struct unit_directory, class_dev); | 1486 | ud = container_of(cdev, struct unit_directory, class_dev); |
1444 | if (ud->ne != ne || !ud->device.driver) | 1487 | if (ud->ne != ne) |
1445 | continue; | 1488 | continue; |
1446 | 1489 | ||
1447 | pdrv = container_of(ud->device.driver, struct hpsb_protocol_driver, driver); | 1490 | down_write(&ieee1394_bus_type.subsys.rwsem); |
1448 | 1491 | if (ud->device.driver) { | |
1449 | if (pdrv->update && pdrv->update(ud)) { | 1492 | pdrv = container_of(ud->device.driver, |
1450 | down_write(&ud->device.bus->subsys.rwsem); | 1493 | struct hpsb_protocol_driver, |
1451 | device_release_driver(&ud->device); | 1494 | driver); |
1452 | up_write(&ud->device.bus->subsys.rwsem); | 1495 | if (pdrv->update && pdrv->update(ud)) |
1496 | device_release_driver(&ud->device); | ||
1453 | } | 1497 | } |
1498 | up_write(&ieee1394_bus_type.subsys.rwsem); | ||
1454 | } | 1499 | } |
1500 | up(&nodemgr_ud_class.sem); | ||
1455 | } | 1501 | } |
1456 | 1502 | ||
1457 | 1503 | ||
@@ -1482,8 +1528,6 @@ static void nodemgr_irm_write_bc(struct node_entry *ne, int generation) | |||
1482 | } | 1528 | } |
1483 | 1529 | ||
1484 | 1530 | ||
1485 | /* Caller needs to hold nodemgr_ud_class.subsys.rwsem as reader because the | ||
1486 | * calls to nodemgr_update_pdrv() and nodemgr_suspend_ne() here require it. */ | ||
1487 | static void nodemgr_probe_ne(struct host_info *hi, struct node_entry *ne, int generation) | 1531 | static void nodemgr_probe_ne(struct host_info *hi, struct node_entry *ne, int generation) |
1488 | { | 1532 | { |
1489 | struct device *dev; | 1533 | struct device *dev; |
@@ -1516,7 +1560,6 @@ static void nodemgr_probe_ne(struct host_info *hi, struct node_entry *ne, int ge | |||
1516 | static void nodemgr_node_probe(struct host_info *hi, int generation) | 1560 | static void nodemgr_node_probe(struct host_info *hi, int generation) |
1517 | { | 1561 | { |
1518 | struct hpsb_host *host = hi->host; | 1562 | struct hpsb_host *host = hi->host; |
1519 | struct class *class = &nodemgr_ne_class; | ||
1520 | struct class_device *cdev; | 1563 | struct class_device *cdev; |
1521 | struct node_entry *ne; | 1564 | struct node_entry *ne; |
1522 | 1565 | ||
@@ -1529,18 +1572,18 @@ static void nodemgr_node_probe(struct host_info *hi, int generation) | |||
1529 | * while probes are time-consuming. (Well, those probes need some | 1572 | * while probes are time-consuming. (Well, those probes need some |
1530 | * improvement...) */ | 1573 | * improvement...) */ |
1531 | 1574 | ||
1532 | down_read(&class->subsys.rwsem); | 1575 | down(&nodemgr_ne_class.sem); |
1533 | list_for_each_entry(cdev, &class->children, node) { | 1576 | list_for_each_entry(cdev, &nodemgr_ne_class.children, node) { |
1534 | ne = container_of(cdev, struct node_entry, class_dev); | 1577 | ne = container_of(cdev, struct node_entry, class_dev); |
1535 | if (!ne->needs_probe) | 1578 | if (!ne->needs_probe) |
1536 | nodemgr_probe_ne(hi, ne, generation); | 1579 | nodemgr_probe_ne(hi, ne, generation); |
1537 | } | 1580 | } |
1538 | list_for_each_entry(cdev, &class->children, node) { | 1581 | list_for_each_entry(cdev, &nodemgr_ne_class.children, node) { |
1539 | ne = container_of(cdev, struct node_entry, class_dev); | 1582 | ne = container_of(cdev, struct node_entry, class_dev); |
1540 | if (ne->needs_probe) | 1583 | if (ne->needs_probe) |
1541 | nodemgr_probe_ne(hi, ne, generation); | 1584 | nodemgr_probe_ne(hi, ne, generation); |
1542 | } | 1585 | } |
1543 | up_read(&class->subsys.rwsem); | 1586 | up(&nodemgr_ne_class.sem); |
1544 | 1587 | ||
1545 | 1588 | ||
1546 | /* If we had a bus reset while we were scanning the bus, it is | 1589 | /* If we had a bus reset while we were scanning the bus, it is |
@@ -1752,19 +1795,18 @@ exit: | |||
1752 | 1795 | ||
1753 | int nodemgr_for_each_host(void *__data, int (*cb)(struct hpsb_host *, void *)) | 1796 | int nodemgr_for_each_host(void *__data, int (*cb)(struct hpsb_host *, void *)) |
1754 | { | 1797 | { |
1755 | struct class *class = &hpsb_host_class; | ||
1756 | struct class_device *cdev; | 1798 | struct class_device *cdev; |
1757 | struct hpsb_host *host; | 1799 | struct hpsb_host *host; |
1758 | int error = 0; | 1800 | int error = 0; |
1759 | 1801 | ||
1760 | down_read(&class->subsys.rwsem); | 1802 | down(&hpsb_host_class.sem); |
1761 | list_for_each_entry(cdev, &class->children, node) { | 1803 | list_for_each_entry(cdev, &hpsb_host_class.children, node) { |
1762 | host = container_of(cdev, struct hpsb_host, class_dev); | 1804 | host = container_of(cdev, struct hpsb_host, class_dev); |
1763 | 1805 | ||
1764 | if ((error = cb(host, __data))) | 1806 | if ((error = cb(host, __data))) |
1765 | break; | 1807 | break; |
1766 | } | 1808 | } |
1767 | up_read(&class->subsys.rwsem); | 1809 | up(&hpsb_host_class.sem); |
1768 | 1810 | ||
1769 | return error; | 1811 | return error; |
1770 | } | 1812 | } |