Commit 9de67725 authored by Jan Höppner's avatar Jan Höppner Committed by Martin Schwidefsky
Browse files

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: default avatarStefan Haberland <sth@linux.vnet.ibm.com>
Signed-off-by: default avatarJan Höppner <hoeppner@linux.vnet.ibm.com>
Signed-off-by: default avatarMartin Schwidefsky <schwidefsky@de.ibm.com>
parent 0f57c97f

drivers/s390/block/dasd_devmap.c

+13 −14
Original line number Diff line number Diff line
@@ -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);

drivers/s390/block/dasd_eer.c

+21 −8
Original line number Diff line number Diff line
@@ -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;

}



/*

CRA Git | Maintained and supported by SUSTech CRA and CCSE