aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/ieee1394
diff options
context:
space:
mode:
authorStefan Richter <stefanr@s5r6.in-berlin.de>2006-10-22 10:16:27 -0400
committerStefan Richter <stefanr@s5r6.in-berlin.de>2006-12-07 15:32:10 -0500
commitb07375b155a0d2ed21a64db68e737da1f19385f7 (patch)
tree06669d571a153c7692ca9ed3f18c3686613cb231 /drivers/ieee1394
parent7fdfc90945e308dc1be37e3914cd979a535263e9 (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.c142
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
737static DEFINE_MUTEX(nodemgr_serialize_remove_uds);
738
737static void nodemgr_remove_uds(struct node_entry *ne) 739static 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
881static struct node_entry *find_entry_by_guid(u64 guid) 926static 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
902static struct node_entry *find_entry_by_nodeid(struct hpsb_host *host, nodeid_t nodeid) 946static 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. */
1381static void nodemgr_suspend_ne(struct node_entry *ne) 1424static 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. */
1436static void nodemgr_update_pdrv(struct node_entry *ne) 1478static 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. */
1487static void nodemgr_probe_ne(struct host_info *hi, struct node_entry *ne, int generation) 1531static 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
1516static void nodemgr_node_probe(struct host_info *hi, int generation) 1560static 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
1753int nodemgr_for_each_host(void *__data, int (*cb)(struct hpsb_host *, void *)) 1796int 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}