[S390] cio: fix ccwgroup unregistration race condition

A race condition exists in the ccwgroup device unregistration code
which can cause a kernel panic due to a use-after-free bug. This
race condition might be triggered when all ccw devices associated with
a ccwgroup device are removed at the same time (e.g. because the
corresponding channel path becomes no longer available).

Fix this race condition by clearing the references from the associated
ccw devices to the ccw group device during unregistration of the
ccw group device.

Signed-off-by: Peter Oberparleiter <peter.oberparleiter@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

diff --git a/drivers/s390/cio/ccwgroup.c b/drivers/s390/cio/ccwgroup.c
index 97b25d68e3e72c74c53994abf7cc140795774525..2864581d8ecb88a9d9ce362e6fbb170a57a49f46 100644 (file)
--- a/drivers/s390/cio/ccwgroup.c
+++ b/drivers/s390/cio/ccwgroup.c
@@ -66,6 +66,27 @@ __ccwgroup_remove_symlinks(struct ccwgroup_device *gdev)
        
 }
 
+/*
+ * Remove references from ccw devices to ccw group device and from
+ * ccw group device to ccw devices.
+ */
+static void __ccwgroup_remove_cdev_refs(struct ccwgroup_device *gdev)
+{
+       struct ccw_device *cdev;
+       int i;
+
+       for (i = 0; i < gdev->count; i++) {
+               cdev = gdev->cdev[i];
+               if (!cdev)
+                       continue;
+               spin_lock_irq(cdev->ccwlock);
+               dev_set_drvdata(&cdev->dev, NULL);
+               spin_unlock_irq(cdev->ccwlock);
+               gdev->cdev[i] = NULL;
+               put_device(&cdev->dev);
+       }
+}
+
 /*
  * Provide an 'ungroup' attribute so the user can remove group devices no
  * longer needed or accidentially created. Saves memory :)
@@ -78,6 +99,7 @@ static void ccwgroup_ungroup_callback(struct device *dev)
        if (device_is_registered(&gdev->dev)) {
                __ccwgroup_remove_symlinks(gdev);
                device_unregister(dev);
+               __ccwgroup_remove_cdev_refs(gdev);
        }
        mutex_unlock(&gdev->reg_mutex);
 }
@@ -116,21 +138,7 @@ static DEVICE_ATTR(ungroup, 0200, NULL, ccwgroup_ungroup_store);
 static void
 ccwgroup_release (struct device *dev)
 {
-       struct ccwgroup_device *gdev;
-       int i;
-
-       gdev = to_ccwgroupdev(dev);
-
-       for (i = 0; i < gdev->count; i++) {
-               if (gdev->cdev[i]) {
-                       spin_lock_irq(gdev->cdev[i]->ccwlock);
-                       if (dev_get_drvdata(&gdev->cdev[i]->dev) == gdev)
-                               dev_set_drvdata(&gdev->cdev[i]->dev, NULL);
-                       spin_unlock_irq(gdev->cdev[i]->ccwlock);
-                       put_device(&gdev->cdev[i]->dev);
-               }
-       }
-       kfree(gdev);
+       kfree(to_ccwgroupdev(dev));
 }
 
 static int
@@ -639,6 +647,7 @@ void ccwgroup_driver_unregister(struct ccwgroup_driver *cdriver)
                mutex_lock(&gdev->reg_mutex);
                __ccwgroup_remove_symlinks(gdev);
                device_unregister(dev);
+               __ccwgroup_remove_cdev_refs(gdev);
                mutex_unlock(&gdev->reg_mutex);
                put_device(dev);
        }
@@ -660,25 +669,6 @@ int ccwgroup_probe_ccwdev(struct ccw_device *cdev)
        return 0;
 }
 
-static struct ccwgroup_device *
-__ccwgroup_get_gdev_by_cdev(struct ccw_device *cdev)
-{
-       struct ccwgroup_device *gdev;
-
-       gdev = dev_get_drvdata(&cdev->dev);
-       if (gdev) {
-               if (get_device(&gdev->dev)) {
-                       mutex_lock(&gdev->reg_mutex);
-                       if (device_is_registered(&gdev->dev))
-                               return gdev;
-                       mutex_unlock(&gdev->reg_mutex);
-                       put_device(&gdev->dev);
-               }
-               return NULL;
-       }
-       return NULL;
-}
-
 /**
  * ccwgroup_remove_ccwdev() - remove function for slave devices
  * @cdev: ccw device to be removed
@@ -694,13 +684,25 @@ void ccwgroup_remove_ccwdev(struct ccw_device *cdev)
        /* Ignore offlining errors, device is gone anyway. */
        ccw_device_set_offline(cdev);
        /* If one of its devices is gone, the whole group is done for. */
-       gdev = __ccwgroup_get_gdev_by_cdev(cdev);
-       if (gdev) {
+       spin_lock_irq(cdev->ccwlock);
+       gdev = dev_get_drvdata(&cdev->dev);
+       if (!gdev) {
+               spin_unlock_irq(cdev->ccwlock);
+               return;
+       }
+       /* Get ccwgroup device reference for local processing. */
+       get_device(&gdev->dev);
+       spin_unlock_irq(cdev->ccwlock);
+       /* Unregister group device. */
+       mutex_lock(&gdev->reg_mutex);
+       if (device_is_registered(&gdev->dev)) {
                __ccwgroup_remove_symlinks(gdev);
                device_unregister(&gdev->dev);
-               mutex_unlock(&gdev->reg_mutex);
-               put_device(&gdev->dev);
+               __ccwgroup_remove_cdev_refs(gdev);
        }
+       mutex_unlock(&gdev->reg_mutex);
+       /* Release ccwgroup device reference for local processing. */
+       put_device(&gdev->dev);
 }
 
 MODULE_LICENSE("GPL");