After a copy pair swap the block device's "device" symlink points to the secondary CCW device, but the gendisk's parent remained the primary, leaving /sys/block/ under the wrong parent. Move the gendisk to the secondary's device with device_move(), keeping the sysfs topology consistent after the swap. Fixes: 413862caad6f ("s390/dasd: add copy pair swap capability") Cc: stable@vger.kernel.org #6.1 Reviewed-by: Jan Hoeppner Signed-off-by: Stefan Haberland --- drivers/s390/block/dasd_eckd.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c index 88fa17aea2ec..ec0c62e5ef73 100644 --- a/drivers/s390/block/dasd_eckd.c +++ b/drivers/s390/block/dasd_eckd.c @@ -6150,6 +6150,7 @@ static int dasd_eckd_copy_pair_swap(struct dasd_device *device, char *prim_busid struct dasd_copy_relation *copy; struct dasd_block *block; struct gendisk *gdp; + int rc; copy = device->copy; if (!copy) @@ -6184,6 +6185,13 @@ static int dasd_eckd_copy_pair_swap(struct dasd_device *device, char *prim_busid /* swap blocklayer device link */ gdp = block->gdp; dasd_add_link_to_gendisk(gdp, secondary); + rc = device_move(disk_to_dev(gdp), &secondary->cdev->dev, DPM_ORDER_NONE); + if (rc) { + dev_err(&primary->cdev->dev, + "copy_pair_swap: moving blockdevice parent %s->%s failed (%d)\n", + dev_name(&primary->cdev->dev), + dev_name(&secondary->cdev->dev), rc); + } /* re-enable device */ dasd_device_remove_stop_bits(primary, DASD_STOPPED_PPRC); -- 2.51.0 The DASD driver only uses the dentry pointers when removing debugfs entries, and debugfs_remove() can safely handle both NULL and ERR_PTR. There is therefore no need to check debugfs_create() return values. This simplifies the debugfs setup code without changing functionality. Suggested-by: Heiko Carstens Reviewed-by: Heiko Carstens Reviewed-by: Jan Hoeppner Signed-off-by: Stefan Haberland --- drivers/s390/block/dasd.c | 64 +++++---------------------------------- 1 file changed, 8 insertions(+), 56 deletions(-) diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c index 7765e40f7cea..496f95745ade 100644 --- a/drivers/s390/block/dasd.c +++ b/drivers/s390/block/dasd.c @@ -207,19 +207,6 @@ static int dasd_state_known_to_new(struct dasd_device *device) return 0; } -static struct dentry *dasd_debugfs_setup(const char *name, - struct dentry *base_dentry) -{ - struct dentry *pde; - - if (!base_dentry) - return NULL; - pde = debugfs_create_dir(name, base_dentry); - if (!pde || IS_ERR(pde)) - return NULL; - return pde; -} - /* * Request the irq line for the device. */ @@ -234,14 +221,14 @@ static int dasd_state_known_to_basic(struct dasd_device *device) if (rc) return rc; block->debugfs_dentry = - dasd_debugfs_setup(block->gdp->disk_name, + debugfs_create_dir(block->gdp->disk_name, dasd_debugfs_root_entry); dasd_profile_init(&block->profile, block->debugfs_dentry); if (dasd_global_profile_level == DASD_PROFILE_ON) dasd_profile_on(&device->block->profile); } device->debugfs_dentry = - dasd_debugfs_setup(dev_name(&device->cdev->dev), + debugfs_create_dir(dev_name(&device->cdev->dev), dasd_debugfs_root_entry); dasd_profile_init(&device->profile, device->debugfs_dentry); dasd_hosts_init(device->debugfs_dentry, device); @@ -1058,19 +1045,9 @@ static const struct file_operations dasd_stats_raw_fops = { static void dasd_profile_init(struct dasd_profile *profile, struct dentry *base_dentry) { - umode_t mode; - struct dentry *pde; - - if (!base_dentry) - return; - profile->dentry = NULL; profile->data = NULL; - mode = (S_IRUSR | S_IWUSR | S_IFREG); - pde = debugfs_create_file("statistics", mode, base_dentry, - profile, &dasd_stats_raw_fops); - if (pde && !IS_ERR(pde)) - profile->dentry = pde; - return; + profile->dentry = debugfs_create_file("statistics", 0600, base_dentry, + profile, &dasd_stats_raw_fops); } static void dasd_profile_exit(struct dasd_profile *profile) @@ -1090,25 +1067,9 @@ static void dasd_statistics_removeroot(void) static void dasd_statistics_createroot(void) { - struct dentry *pde; - - dasd_debugfs_root_entry = NULL; - pde = debugfs_create_dir("dasd", NULL); - if (!pde || IS_ERR(pde)) - goto error; - dasd_debugfs_root_entry = pde; - pde = debugfs_create_dir("global", dasd_debugfs_root_entry); - if (!pde || IS_ERR(pde)) - goto error; - dasd_debugfs_global_entry = pde; + dasd_debugfs_root_entry = debugfs_create_dir("dasd", NULL); + dasd_debugfs_global_entry = debugfs_create_dir("global", dasd_debugfs_root_entry); dasd_profile_init(&dasd_global_profile, dasd_debugfs_global_entry); - return; - -error: - DBF_EVENT(DBF_ERR, "%s", - "Creation of the dasd debugfs interface failed"); - dasd_statistics_removeroot(); - return; } #else @@ -1169,17 +1130,8 @@ static void dasd_hosts_exit(struct dasd_device *device) static void dasd_hosts_init(struct dentry *base_dentry, struct dasd_device *device) { - struct dentry *pde; - umode_t mode; - - if (!base_dentry) - return; - - mode = S_IRUSR | S_IFREG; - pde = debugfs_create_file("host_access_list", mode, base_dentry, - device, &dasd_hosts_fops); - if (pde && !IS_ERR(pde)) - device->hosts_dentry = pde; + device->hosts_dentry = debugfs_create_file("host_access_list", 0400, base_dentry, + device, &dasd_hosts_fops); } struct dasd_ccw_req *dasd_smalloc_request(int magic, int cplength, int datasize, -- 2.51.0 From: Jan Höppner The device name formatting can be generalized and made more readable compared to the current state. SCSI already provides a generalized way to format many devices in the same naming scheme as DASD does, which was introduced with commit 3e1a7ff8a0a7 ("block: allow disk to have extended device number"). Use this much cleaner code from drivers/scsi/sd.c to handle the legacy naming scheme in DASD as a replacement for the current implementation. For easier error handling for the new function, move the gendisk free portion of dasd_gendisk_free() out into a new function dasd_gd_free(). Signed-off-by: Jan Höppner Reviewed-by: Stefan Haberland Signed-off-by: Stefan Haberland --- drivers/s390/block/dasd_genhd.c | 80 ++++++++++++++++++++++----------- 1 file changed, 54 insertions(+), 26 deletions(-) diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c index 28e92fad0ca1..6ee3d952412e 100644 --- a/drivers/s390/block/dasd_genhd.c +++ b/drivers/s390/block/dasd_genhd.c @@ -22,6 +22,7 @@ static unsigned int queue_depth = 32; static unsigned int nr_hw_queues = 4; +static void dasd_gd_free(struct gendisk *gdp); module_param(queue_depth, uint, 0444); MODULE_PARM_DESC(queue_depth, "Default queue depth for new DASD devices"); @@ -29,6 +30,37 @@ MODULE_PARM_DESC(queue_depth, "Default queue depth for new DASD devices"); module_param(nr_hw_queues, uint, 0444); MODULE_PARM_DESC(nr_hw_queues, "Default number of hardware queues for new DASD devices"); +/* + * Set device name. + * dasda - dasdz : 26 devices + * dasdaa - dasdzz : 676 devices, added up = 702 + * dasdaaa - dasdzzz : 17576 devices, added up = 18278 + * dasdaaaa - dasdzzzz : 456976 devices, added up = 475252 + */ +static int dasd_name_format(char *prefix, int index, char *buf, int buflen) +{ + const int base = 'z' - 'a' + 1; + char *begin = buf + strlen(prefix); + char *end = buf + buflen; + char *p; + int unit; + + p = end - 1; + *p = '\0'; + unit = base; + do { + if (p == begin) + return -EINVAL; + *--p = 'a' + (index % unit); + index = (index / unit) - 1; + } while (index >= 0); + + memmove(begin, p, end - p); + memcpy(buf, prefix, strlen(prefix)); + + return 0; +} + /* * Allocate and register gendisk structure for device. */ @@ -45,11 +77,13 @@ int dasd_gendisk_alloc(struct dasd_block *block) }; struct gendisk *gdp; struct dasd_device *base; - int len, rc; + unsigned int devindex; + int rc; /* Make sure the minor for this device exists. */ base = block->base; - if (base->devindex >= DASD_PER_MAJOR) + devindex = base->devindex; + if (devindex >= DASD_PER_MAJOR) return -EBUSY; block->tag_set.ops = &dasd_mq_ops; @@ -69,31 +103,17 @@ int dasd_gendisk_alloc(struct dasd_block *block) /* Initialize gendisk structure. */ gdp->major = DASD_MAJOR; - gdp->first_minor = base->devindex << DASD_PARTN_BITS; + gdp->first_minor = devindex << DASD_PARTN_BITS; gdp->minors = 1 << DASD_PARTN_BITS; gdp->fops = &dasd_device_operations; - /* - * Set device name. - * dasda - dasdz : 26 devices - * dasdaa - dasdzz : 676 devices, added up = 702 - * dasdaaa - dasdzzz : 17576 devices, added up = 18278 - * dasdaaaa - dasdzzzz : 456976 devices, added up = 475252 - */ - len = sprintf(gdp->disk_name, "dasd"); - if (base->devindex > 25) { - if (base->devindex > 701) { - if (base->devindex > 18277) - len += sprintf(gdp->disk_name + len, "%c", - 'a'+(((base->devindex-18278) - /17576)%26)); - len += sprintf(gdp->disk_name + len, "%c", - 'a'+(((base->devindex-702)/676)%26)); - } - len += sprintf(gdp->disk_name + len, "%c", - 'a'+(((base->devindex-26)/26)%26)); + rc = dasd_name_format("dasd", devindex, gdp->disk_name, sizeof(gdp->disk_name)); + if (rc) { + DBF_DEV_EVENT(DBF_ERR, block->base, + "setting disk name failed, rc %d", rc); + dasd_gd_free(gdp); + return rc; } - len += sprintf(gdp->disk_name + len, "%c", 'a'+(base->devindex%26)); if (base->features & DASD_FEATURE_READONLY || test_bit(DASD_FLAG_DEVICE_RO, &base->flags)) @@ -111,15 +131,23 @@ int dasd_gendisk_alloc(struct dasd_block *block) return 0; } +/* + * Free gendisk structure + */ +static void dasd_gd_free(struct gendisk *gd) +{ + del_gendisk(gd); + gd->private_data = NULL; + put_disk(gd); +} + /* * Unregister and free gendisk structure for device. */ void dasd_gendisk_free(struct dasd_block *block) { if (block->gdp) { - del_gendisk(block->gdp); - block->gdp->private_data = NULL; - put_disk(block->gdp); + dasd_gd_free(block->gdp); block->gdp = NULL; blk_mq_free_tag_set(&block->tag_set); } -- 2.51.0 From: Jan Höppner Use scnprintf() instead of sprintf() for those cases where the destination is an array and the size of the array is known at compile time. This prevents theoretical buffer overflows, but also avoids that people again and again spend time to figure out if the code is actually safe. Signed-off-by: Jan Höppner Reviewed-by: Stefan Haberland Signed-off-by: Stefan Haberland --- drivers/s390/block/dasd_devmap.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/s390/block/dasd_devmap.c b/drivers/s390/block/dasd_devmap.c index ddbdf1f85d44..73972900fc55 100644 --- a/drivers/s390/block/dasd_devmap.c +++ b/drivers/s390/block/dasd_devmap.c @@ -355,7 +355,8 @@ static int __init dasd_parse_range(const char *range) /* each device in dasd= parameter should be set initially online */ features |= DASD_FEATURE_INITIAL_ONLINE; while (from <= to) { - sprintf(bus_id, "%01x.%01x.%04x", from_id0, from_id1, from++); + scnprintf(bus_id, sizeof(bus_id), + "%01x.%01x.%04x", from_id0, from_id1, from++); devmap = dasd_add_busid(bus_id, features); if (IS_ERR(devmap)) { rc = PTR_ERR(devmap); -- 2.51.0