s390/cio: fix use after free in cmb processing
authorSebastian Ott <sebott@linux.vnet.ibm.com>
Mon, 7 Sep 2015 17:51:39 +0000 (19:51 +0200)
committerMartin Schwidefsky <schwidefsky@de.ibm.com>
Wed, 14 Oct 2015 12:32:02 +0000 (14:32 +0200)
Devices with active channel measurement are included in a list. When a
device is removed without deactivating channel measurement first the
list_head is freed but still used. Fix this by making sure that
channel measurement is deactivated during device deregistration.

For devices that we deregister because they are no longer accessible
deactivating channel measurement will fail. In this case we can report
success because the FW will no longer access the measurement block.

In addition to these steps keep an extra device reference while
channel measurement is active.

Signed-off-by: Sebastian Ott <sebott@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
drivers/s390/cio/cmf.c
drivers/s390/cio/device.c

index 5eeb62c3f33a2f327d91750f586ee8d9c9412c16..31677c075a8eee3ff6adfbe28d2cdd811f5e13d3 100644 (file)
@@ -186,9 +186,8 @@ static inline void cmf_activate(void *area, unsigned int onoff)
 static int set_schib(struct ccw_device *cdev, u32 mme, int mbfc,
                     unsigned long address)
 {
-       struct subchannel *sch;
-
-       sch = to_subchannel(cdev->dev.parent);
+       struct subchannel *sch = to_subchannel(cdev->dev.parent);
+       int ret;
 
        sch->config.mme = mme;
        sch->config.mbfc = mbfc;
@@ -198,7 +197,15 @@ static int set_schib(struct ccw_device *cdev, u32 mme, int mbfc,
        else
                sch->config.mbi = address;
 
-       return cio_commit_config(sch);
+       ret = cio_commit_config(sch);
+       if (!mme && ret == -ENODEV) {
+               /*
+                * The task was to disable measurement block updates but
+                * the subchannel is already gone. Report success.
+                */
+               ret = 0;
+       }
+       return ret;
 }
 
 struct set_schib_struct {
@@ -606,12 +613,6 @@ static void free_cmb(struct ccw_device *cdev)
        spin_lock_irq(cdev->ccwlock);
 
        priv = cdev->private;
-
-       if (list_empty(&priv->cmb_list)) {
-               /* already freed */
-               goto out;
-       }
-
        cmb_data = priv->cmb;
        priv->cmb = NULL;
        if (cmb_data)
@@ -626,7 +627,6 @@ static void free_cmb(struct ccw_device *cdev)
                free_pages((unsigned long)cmb_area.mem, get_order(size));
                cmb_area.mem = NULL;
        }
-out:
        spin_unlock_irq(cdev->ccwlock);
        spin_unlock(&cmb_area.lock);
 }
@@ -1227,6 +1227,7 @@ int enable_cmf(struct ccw_device *cdev)
        int ret;
 
        device_lock(&cdev->dev);
+       get_device(&cdev->dev);
        ret = cmbops->alloc(cdev);
        if (ret)
                goto out;
@@ -1242,6 +1243,9 @@ int enable_cmf(struct ccw_device *cdev)
                cmbops->free(cdev);
        }
 out:
+       if (ret)
+               put_device(&cdev->dev);
+
        device_unlock(&cdev->dev);
        return ret;
 }
@@ -1265,6 +1269,7 @@ int __disable_cmf(struct ccw_device *cdev)
 
        sysfs_remove_group(&cdev->dev.kobj, cmbops->attr_group);
        cmbops->free(cdev);
+       put_device(&cdev->dev);
 
        return ret;
 }
index 20b92c7039447c6d9eb233d29adaff1eeefa8c9c..6aae6841280214f2db3a3aac0461f14f9050cf04 100644 (file)
@@ -1787,6 +1787,8 @@ static int ccw_device_remove(struct device *dev)
        cdev->drv = NULL;
        cdev->private->int_class = IRQIO_CIO;
        spin_unlock_irq(cdev->ccwlock);
+       __disable_cmf(cdev);
+
        return 0;
 }