s390/dasd: Fix locking issue when changing EER attribute

The reference to a device in question may get lost when the extended
error reporting (EER) attribute is being enabled/disabled while the
device is set offline at the same time. This is due to missing
refcounting and incorrect locking. Fix this by the following:

- In dasd_eer_store() get the device directly and handle the refcount
  accordingly.
- Move the lock in dasd_eer_enable() up so we can ensure safe
  processing.
- Check if the device is being set offline and return with -EBUSY if so.
- While at it, change the return code from -EPERM to -EMEDIUMTYPE as
  suggested by a FIXME, since that is what we're actually checking.

Reviewed-by: Stefan Haberland <sth@linux.vnet.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.vnet.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

diff --git a/drivers/s390/block/dasd_devmap.c b/drivers/s390/block/dasd_devmap.c
index 221d8c8..c1979ec 100644 (file)
--- a/drivers/s390/block/dasd_devmap.c
+++ b/drivers/s390/block/dasd_devmap.c
@@ -1167,26 +1167,25 @@ static ssize_t
 dasd_eer_store(struct device *dev, struct device_attribute *attr,
               const char *buf, size_t count)
 {
-       struct dasd_devmap *devmap;
+       struct dasd_device *device;
        unsigned int val;
-       int rc;
+       int rc = 0;
 
-       devmap = dasd_devmap_from_cdev(to_ccwdev(dev));
-       if (IS_ERR(devmap))
-               return PTR_ERR(devmap);
-       if (!devmap->device)
-               return -ENODEV;
+       device = dasd_device_from_cdev(to_ccwdev(dev));
+       if (IS_ERR(device))
+               return PTR_ERR(device);
 
        if (kstrtouint(buf, 0, &val) || val > 1)
                return -EINVAL;
 
-       if (val) {
-               rc = dasd_eer_enable(devmap->device);
-               if (rc)
-                       return rc;
-       } else
-               dasd_eer_disable(devmap->device);
-       return count;
+       if (val)
+               rc = dasd_eer_enable(device);
+       else
+               dasd_eer_disable(device);
+
+       dasd_put_device(device);
+
+       return rc ? : count;
 }
 
 static DEVICE_ATTR(eer_enabled, 0644, dasd_eer_show, dasd_eer_store);

diff --git a/drivers/s390/block/dasd_eer.c b/drivers/s390/block/dasd_eer.c
index 21ef63c..6c5d671 100644 (file)
--- a/drivers/s390/block/dasd_eer.c
+++ b/drivers/s390/block/dasd_eer.c
@@ -454,20 +454,30 @@ static void dasd_eer_snss_cb(struct dasd_ccw_req *cqr, void *data)
  */
 int dasd_eer_enable(struct dasd_device *device)
 {
-       struct dasd_ccw_req *cqr;
+       struct dasd_ccw_req *cqr = NULL;
        unsigned long flags;
        struct ccw1 *ccw;
+       int rc = 0;
 
+       spin_lock_irqsave(get_ccwdev_lock(device->cdev), flags);
        if (device->eer_cqr)
-               return 0;
+               goto out;
+       else if (!device->discipline ||
+                strcmp(device->discipline->name, "ECKD"))
+               rc = -EMEDIUMTYPE;
+       else if (test_bit(DASD_FLAG_OFFLINE, &device->flags))
+               rc = -EBUSY;
 
-       if (!device->discipline || strcmp(device->discipline->name, "ECKD"))
-               return -EPERM;  /* FIXME: -EMEDIUMTYPE ? */
+       if (rc)
+               goto out;
 
        cqr = dasd_kmalloc_request(DASD_ECKD_MAGIC, 1 /* SNSS */,
                                   SNSS_DATA_SIZE, device);
-       if (IS_ERR(cqr))
-               return -ENOMEM;
+       if (IS_ERR(cqr)) {
+               rc = -ENOMEM;
+               cqr = NULL;
+               goto out;
+       }
 
        cqr->startdev = device;
        cqr->retries = 255;
@@ -485,15 +495,18 @@ int dasd_eer_enable(struct dasd_device *device)
        cqr->status = DASD_CQR_FILLED;
        cqr->callback = dasd_eer_snss_cb;
 
-       spin_lock_irqsave(get_ccwdev_lock(device->cdev), flags);
        if (!device->eer_cqr) {
                device->eer_cqr = cqr;
                cqr = NULL;
        }
+
+out:
        spin_unlock_irqrestore(get_ccwdev_lock(device->cdev), flags);
+
        if (cqr)
                dasd_kfree_request(cqr, device);
-       return 0;
+
+       return rc;
 }
 
 /*