Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 65f8da47 authored by Stefan Weinhuber's avatar Stefan Weinhuber Committed by Martin Schwidefsky
Browse files

[S390] dasd: fix race between open and offline



The dasd_open function uses the private_data pointer of the gendisk to
find the dasd_block structure that matches the gendisk. When a DASD
device is set offline, we set the private_data pointer of the gendisk
to NULL and later remove the dasd_block structure, but there is still
a small race window, in which dasd_open could first read a pointer
from the private_data field and then try to use it, after the structure
has already been freed.
To close this race window, we will store a pointer to the dasd_devmap
structure of the base device in the private_data field. The devmap
entries are not deleted, and we already have proper locking and
reference counting in place, so that we can safely get from a devmap
pointer to the dasd_device and dasd_block structures of the device.

Signed-off-by: default avatarStefan Weinhuber <wein@de.ibm.com>
Signed-off-by: default avatarMartin Schwidefsky <schwidefsky@de.ibm.com>
parent 2f666bcf
Loading
Loading
Loading
Loading
+22 −18
Original line number Original line Diff line number Diff line
@@ -2314,15 +2314,14 @@ static void dasd_flush_request_queue(struct dasd_block *block)


static int dasd_open(struct block_device *bdev, fmode_t mode)
static int dasd_open(struct block_device *bdev, fmode_t mode)
{
{
	struct dasd_block *block = bdev->bd_disk->private_data;
	struct dasd_device *base;
	struct dasd_device *base;
	int rc;
	int rc;


	if (!block)
	base = dasd_device_from_gendisk(bdev->bd_disk);
	if (!base)
		return -ENODEV;
		return -ENODEV;


	base = block->base;
	atomic_inc(&base->block->open_count);
	atomic_inc(&block->open_count);
	if (test_bit(DASD_FLAG_OFFLINE, &base->flags)) {
	if (test_bit(DASD_FLAG_OFFLINE, &base->flags)) {
		rc = -ENODEV;
		rc = -ENODEV;
		goto unlock;
		goto unlock;
@@ -2355,21 +2354,28 @@ static int dasd_open(struct block_device *bdev, fmode_t mode)
		goto out;
		goto out;
	}
	}


	dasd_put_device(base);
	return 0;
	return 0;


out:
out:
	module_put(base->discipline->owner);
	module_put(base->discipline->owner);
unlock:
unlock:
	atomic_dec(&block->open_count);
	atomic_dec(&base->block->open_count);
	dasd_put_device(base);
	return rc;
	return rc;
}
}


static int dasd_release(struct gendisk *disk, fmode_t mode)
static int dasd_release(struct gendisk *disk, fmode_t mode)
{
{
	struct dasd_block *block = disk->private_data;
	struct dasd_device *base;


	atomic_dec(&block->open_count);
	base = dasd_device_from_gendisk(disk);
	module_put(block->base->discipline->owner);
	if (!base)
		return -ENODEV;

	atomic_dec(&base->block->open_count);
	module_put(base->discipline->owner);
	dasd_put_device(base);
	return 0;
	return 0;
}
}


@@ -2378,20 +2384,20 @@ static int dasd_release(struct gendisk *disk, fmode_t mode)
 */
 */
static int dasd_getgeo(struct block_device *bdev, struct hd_geometry *geo)
static int dasd_getgeo(struct block_device *bdev, struct hd_geometry *geo)
{
{
	struct dasd_block *block;
	struct dasd_device *base;
	struct dasd_device *base;


	block = bdev->bd_disk->private_data;
	base = dasd_device_from_gendisk(bdev->bd_disk);
	if (!block)
	if (!base)
		return -ENODEV;
		return -ENODEV;
	base = block->base;


	if (!base->discipline ||
	if (!base->discipline ||
	    !base->discipline->fill_geometry)
	    !base->discipline->fill_geometry) {
		dasd_put_device(base);
		return -EINVAL;
		return -EINVAL;

	}
	base->discipline->fill_geometry(block, geo);
	base->discipline->fill_geometry(base->block, geo);
	geo->start = get_start_sect(bdev) >> block->s2b_shift;
	geo->start = get_start_sect(bdev) >> base->block->s2b_shift;
	dasd_put_device(base);
	return 0;
	return 0;
}
}


@@ -2528,7 +2534,6 @@ void dasd_generic_remove(struct ccw_device *cdev)
	dasd_set_target_state(device, DASD_STATE_NEW);
	dasd_set_target_state(device, DASD_STATE_NEW);
	/* dasd_delete_device destroys the device reference. */
	/* dasd_delete_device destroys the device reference. */
	block = device->block;
	block = device->block;
	device->block = NULL;
	dasd_delete_device(device);
	dasd_delete_device(device);
	/*
	/*
	 * life cycle of block is bound to device, so delete it after
	 * life cycle of block is bound to device, so delete it after
@@ -2650,7 +2655,6 @@ int dasd_generic_set_offline(struct ccw_device *cdev)
	dasd_set_target_state(device, DASD_STATE_NEW);
	dasd_set_target_state(device, DASD_STATE_NEW);
	/* dasd_delete_device destroys the device reference. */
	/* dasd_delete_device destroys the device reference. */
	block = device->block;
	block = device->block;
	device->block = NULL;
	dasd_delete_device(device);
	dasd_delete_device(device);
	/*
	/*
	 * life cycle of block is bound to device, so delete it after
	 * life cycle of block is bound to device, so delete it after
+30 −0
Original line number Original line Diff line number Diff line
@@ -674,6 +674,36 @@ dasd_device_from_cdev(struct ccw_device *cdev)
	return device;
	return device;
}
}


void dasd_add_link_to_gendisk(struct gendisk *gdp, struct dasd_device *device)
{
	struct dasd_devmap *devmap;

	devmap = dasd_find_busid(dev_name(&device->cdev->dev));
	if (IS_ERR(devmap))
		return;
	spin_lock(&dasd_devmap_lock);
	gdp->private_data = devmap;
	spin_unlock(&dasd_devmap_lock);
}

struct dasd_device *dasd_device_from_gendisk(struct gendisk *gdp)
{
	struct dasd_device *device;
	struct dasd_devmap *devmap;

	if (!gdp->private_data)
		return NULL;
	device = NULL;
	spin_lock(&dasd_devmap_lock);
	devmap = gdp->private_data;
	if (devmap && devmap->device) {
		device = devmap->device;
		dasd_get_device(device);
	}
	spin_unlock(&dasd_devmap_lock);
	return device;
}

/*
/*
 * SECTION: files in sysfs
 * SECTION: files in sysfs
 */
 */
+1 −1
Original line number Original line Diff line number Diff line
@@ -73,7 +73,7 @@ int dasd_gendisk_alloc(struct dasd_block *block)
	if (base->features & DASD_FEATURE_READONLY ||
	if (base->features & DASD_FEATURE_READONLY ||
	    test_bit(DASD_FLAG_DEVICE_RO, &base->flags))
	    test_bit(DASD_FLAG_DEVICE_RO, &base->flags))
		set_disk_ro(gdp, 1);
		set_disk_ro(gdp, 1);
	gdp->private_data = block;
	dasd_add_link_to_gendisk(gdp, base);
	gdp->queue = block->request_queue;
	gdp->queue = block->request_queue;
	block->gdp = gdp;
	block->gdp = gdp;
	set_capacity(block->gdp, 0);
	set_capacity(block->gdp, 0);
+3 −0
Original line number Original line Diff line number Diff line
@@ -686,6 +686,9 @@ struct dasd_device *dasd_device_from_cdev(struct ccw_device *);
struct dasd_device *dasd_device_from_cdev_locked(struct ccw_device *);
struct dasd_device *dasd_device_from_cdev_locked(struct ccw_device *);
struct dasd_device *dasd_device_from_devindex(int);
struct dasd_device *dasd_device_from_devindex(int);


void dasd_add_link_to_gendisk(struct gendisk *, struct dasd_device *);
struct dasd_device *dasd_device_from_gendisk(struct gendisk *);

int dasd_parse(void);
int dasd_parse(void);
int dasd_busid_known(const char *);
int dasd_busid_known(const char *);


+87 −41
Original line number Original line Diff line number Diff line
@@ -42,16 +42,22 @@ dasd_ioctl_api_version(void __user *argp)
static int
static int
dasd_ioctl_enable(struct block_device *bdev)
dasd_ioctl_enable(struct block_device *bdev)
{
{
	struct dasd_block *block = bdev->bd_disk->private_data;
	struct dasd_device *base;


	if (!capable(CAP_SYS_ADMIN))
	if (!capable(CAP_SYS_ADMIN))
		return -EACCES;
		return -EACCES;


	dasd_enable_device(block->base);
	base = dasd_device_from_gendisk(bdev->bd_disk);
	if (!base)
		return -ENODEV;

	dasd_enable_device(base);
	/* Formatting the dasd device can change the capacity. */
	/* Formatting the dasd device can change the capacity. */
	mutex_lock(&bdev->bd_mutex);
	mutex_lock(&bdev->bd_mutex);
	i_size_write(bdev->bd_inode, (loff_t)get_capacity(block->gdp) << 9);
	i_size_write(bdev->bd_inode,
		     (loff_t)get_capacity(base->block->gdp) << 9);
	mutex_unlock(&bdev->bd_mutex);
	mutex_unlock(&bdev->bd_mutex);
	dasd_put_device(base);
	return 0;
	return 0;
}
}


@@ -62,11 +68,14 @@ dasd_ioctl_enable(struct block_device *bdev)
static int
static int
dasd_ioctl_disable(struct block_device *bdev)
dasd_ioctl_disable(struct block_device *bdev)
{
{
	struct dasd_block *block = bdev->bd_disk->private_data;
	struct dasd_device *base;


	if (!capable(CAP_SYS_ADMIN))
	if (!capable(CAP_SYS_ADMIN))
		return -EACCES;
		return -EACCES;


	base = dasd_device_from_gendisk(bdev->bd_disk);
	if (!base)
		return -ENODEV;
	/*
	/*
	 * Man this is sick. We don't do a real disable but only downgrade
	 * Man this is sick. We don't do a real disable but only downgrade
	 * the device to DASD_STATE_BASIC. The reason is that dasdfmt uses
	 * the device to DASD_STATE_BASIC. The reason is that dasdfmt uses
@@ -75,7 +84,7 @@ dasd_ioctl_disable(struct block_device *bdev)
	 * using the BIODASDFMT ioctl. Therefore the correct state for the
	 * using the BIODASDFMT ioctl. Therefore the correct state for the
	 * device is DASD_STATE_BASIC that allows to do basic i/o.
	 * device is DASD_STATE_BASIC that allows to do basic i/o.
	 */
	 */
	dasd_set_target_state(block->base, DASD_STATE_BASIC);
	dasd_set_target_state(base, DASD_STATE_BASIC);
	/*
	/*
	 * Set i_size to zero, since read, write, etc. check against this
	 * Set i_size to zero, since read, write, etc. check against this
	 * value.
	 * value.
@@ -83,6 +92,7 @@ dasd_ioctl_disable(struct block_device *bdev)
	mutex_lock(&bdev->bd_mutex);
	mutex_lock(&bdev->bd_mutex);
	i_size_write(bdev->bd_inode, 0);
	i_size_write(bdev->bd_inode, 0);
	mutex_unlock(&bdev->bd_mutex);
	mutex_unlock(&bdev->bd_mutex);
	dasd_put_device(base);
	return 0;
	return 0;
}
}


@@ -191,26 +201,36 @@ static int dasd_format(struct dasd_block *block, struct format_data_t *fdata)
static int
static int
dasd_ioctl_format(struct block_device *bdev, void __user *argp)
dasd_ioctl_format(struct block_device *bdev, void __user *argp)
{
{
	struct dasd_block *block = bdev->bd_disk->private_data;
	struct dasd_device *base;
	struct format_data_t fdata;
	struct format_data_t fdata;
	int rc;


	if (!capable(CAP_SYS_ADMIN))
	if (!capable(CAP_SYS_ADMIN))
		return -EACCES;
		return -EACCES;
	if (!argp)
	if (!argp)
		return -EINVAL;
		return -EINVAL;

	base = dasd_device_from_gendisk(bdev->bd_disk);
	if (block->base->features & DASD_FEATURE_READONLY ||
	if (!base)
	    test_bit(DASD_FLAG_DEVICE_RO, &block->base->flags))
		return -ENODEV;
	if (base->features & DASD_FEATURE_READONLY ||
	    test_bit(DASD_FLAG_DEVICE_RO, &base->flags)) {
		dasd_put_device(base);
		return -EROFS;
		return -EROFS;
	if (copy_from_user(&fdata, argp, sizeof(struct format_data_t)))
	}
	if (copy_from_user(&fdata, argp, sizeof(struct format_data_t))) {
		dasd_put_device(base);
		return -EFAULT;
		return -EFAULT;
	}
	if (bdev != bdev->bd_contains) {
	if (bdev != bdev->bd_contains) {
		pr_warning("%s: The specified DASD is a partition and cannot "
		pr_warning("%s: The specified DASD is a partition and cannot "
			   "be formatted\n",
			   "be formatted\n",
			   dev_name(&block->base->cdev->dev));
			   dev_name(&base->cdev->dev));
		dasd_put_device(base);
		return -EINVAL;
		return -EINVAL;
	}
	}
	return dasd_format(block, &fdata);
	rc = dasd_format(base->block, &fdata);
	dasd_put_device(base);
	return rc;
}
}


#ifdef CONFIG_DASD_PROFILE
#ifdef CONFIG_DASD_PROFILE
@@ -340,8 +360,8 @@ static int dasd_ioctl_information(struct dasd_block *block,
static int
static int
dasd_ioctl_set_ro(struct block_device *bdev, void __user *argp)
dasd_ioctl_set_ro(struct block_device *bdev, void __user *argp)
{
{
	struct dasd_block *block =  bdev->bd_disk->private_data;
	struct dasd_device *base;
	int intval;
	int intval, rc;


	if (!capable(CAP_SYS_ADMIN))
	if (!capable(CAP_SYS_ADMIN))
		return -EACCES;
		return -EACCES;
@@ -350,10 +370,17 @@ dasd_ioctl_set_ro(struct block_device *bdev, void __user *argp)
		return -EINVAL;
		return -EINVAL;
	if (get_user(intval, (int __user *)argp))
	if (get_user(intval, (int __user *)argp))
		return -EFAULT;
		return -EFAULT;
	if (!intval && test_bit(DASD_FLAG_DEVICE_RO, &block->base->flags))
	base = dasd_device_from_gendisk(bdev->bd_disk);
	if (!base)
		return -ENODEV;
	if (!intval && test_bit(DASD_FLAG_DEVICE_RO, &base->flags)) {
		dasd_put_device(base);
		return -EROFS;
		return -EROFS;
	}
	set_disk_ro(bdev->bd_disk, intval);
	set_disk_ro(bdev->bd_disk, intval);
	return dasd_set_feature(block->base->cdev, DASD_FEATURE_READONLY, intval);
	rc = dasd_set_feature(base->cdev, DASD_FEATURE_READONLY, intval);
	dasd_put_device(base);
	return rc;
}
}


static int dasd_ioctl_readall_cmb(struct dasd_block *block, unsigned int cmd,
static int dasd_ioctl_readall_cmb(struct dasd_block *block, unsigned int cmd,
@@ -372,59 +399,78 @@ static int dasd_ioctl_readall_cmb(struct dasd_block *block, unsigned int cmd,
int dasd_ioctl(struct block_device *bdev, fmode_t mode,
int dasd_ioctl(struct block_device *bdev, fmode_t mode,
	       unsigned int cmd, unsigned long arg)
	       unsigned int cmd, unsigned long arg)
{
{
	struct dasd_block *block = bdev->bd_disk->private_data;
	struct dasd_block *block;
	struct dasd_device *base;
	void __user *argp;
	void __user *argp;
	int rc;


	if (is_compat_task())
	if (is_compat_task())
		argp = compat_ptr(arg);
		argp = compat_ptr(arg);
	else
	else
		argp = (void __user *)arg;
		argp = (void __user *)arg;


	if (!block)
                return -ENODEV;

	if ((_IOC_DIR(cmd) != _IOC_NONE) && !arg) {
	if ((_IOC_DIR(cmd) != _IOC_NONE) && !arg) {
		PRINT_DEBUG("empty data ptr");
		PRINT_DEBUG("empty data ptr");
		return -EINVAL;
		return -EINVAL;
	}
	}


	base = dasd_device_from_gendisk(bdev->bd_disk);
	if (!base)
		return -ENODEV;
	block = base->block;
	rc = 0;
	switch (cmd) {
	switch (cmd) {
	case BIODASDDISABLE:
	case BIODASDDISABLE:
		return dasd_ioctl_disable(bdev);
		rc = dasd_ioctl_disable(bdev);
		break;
	case BIODASDENABLE:
	case BIODASDENABLE:
		return dasd_ioctl_enable(bdev);
		rc = dasd_ioctl_enable(bdev);
		break;
	case BIODASDQUIESCE:
	case BIODASDQUIESCE:
		return dasd_ioctl_quiesce(block);
		rc = dasd_ioctl_quiesce(block);
		break;
	case BIODASDRESUME:
	case BIODASDRESUME:
		return dasd_ioctl_resume(block);
		rc = dasd_ioctl_resume(block);
		break;
	case BIODASDFMT:
	case BIODASDFMT:
		return dasd_ioctl_format(bdev, argp);
		rc = dasd_ioctl_format(bdev, argp);
		break;
	case BIODASDINFO:
	case BIODASDINFO:
		return dasd_ioctl_information(block, cmd, argp);
		rc = dasd_ioctl_information(block, cmd, argp);
		break;
	case BIODASDINFO2:
	case BIODASDINFO2:
		return dasd_ioctl_information(block, cmd, argp);
		rc = dasd_ioctl_information(block, cmd, argp);
		break;
	case BIODASDPRRD:
	case BIODASDPRRD:
		return dasd_ioctl_read_profile(block, argp);
		rc = dasd_ioctl_read_profile(block, argp);
		break;
	case BIODASDPRRST:
	case BIODASDPRRST:
		return dasd_ioctl_reset_profile(block);
		rc = dasd_ioctl_reset_profile(block);
		break;
	case BLKROSET:
	case BLKROSET:
		return dasd_ioctl_set_ro(bdev, argp);
		rc = dasd_ioctl_set_ro(bdev, argp);
		break;
	case DASDAPIVER:
	case DASDAPIVER:
		return dasd_ioctl_api_version(argp);
		rc = dasd_ioctl_api_version(argp);
		break;
	case BIODASDCMFENABLE:
	case BIODASDCMFENABLE:
		return enable_cmf(block->base->cdev);
		rc = enable_cmf(base->cdev);
		break;
	case BIODASDCMFDISABLE:
	case BIODASDCMFDISABLE:
		return disable_cmf(block->base->cdev);
		rc = disable_cmf(base->cdev);
		break;
	case BIODASDREADALLCMB:
	case BIODASDREADALLCMB:
		return dasd_ioctl_readall_cmb(block, cmd, argp);
		rc = dasd_ioctl_readall_cmb(block, cmd, argp);
		break;
	default:
	default:
		/* if the discipline has an ioctl method try it. */
		/* if the discipline has an ioctl method try it. */
		if (block->base->discipline->ioctl) {
		if (base->discipline->ioctl) {
			int rval = block->base->discipline->ioctl(block, cmd, argp);
			rc = base->discipline->ioctl(block, cmd, argp);
			if (rval != -ENOIOCTLCMD)
			if (rc == -ENOIOCTLCMD)
				return rval;
				rc = -EINVAL;
		}
		} else

			rc = -EINVAL;
		return -EINVAL;
	}
	}
	dasd_put_device(base);
	return rc;
}
}