From 94dace8c85717588c2b4d116759cc3253f47d0eb Mon Sep 17 00:00:00 2001 From: Gioh Kim Date: Mon, 26 Jul 2021 13:59:49 +0200 Subject: block/rnbd-clt: Use put_cpu_ptr after get_cpu_ptr This patch replaces put_cpu_var with put_cpu_ptr because get_cpu_ptr should be paired with put_cpu_ptr. Signed-off-by: Gioh Kim Signed-off-by: Jack Wang Link: https://lore.kernel.org/r/20210726115950.470543-2-jinpu.wang@ionos.com Signed-off-by: Jens Axboe --- drivers/block/rnbd/rnbd-clt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/block') diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c index e9cc413495f0..bd4a41afbbfc 100644 --- a/drivers/block/rnbd/rnbd-clt.c +++ b/drivers/block/rnbd/rnbd-clt.c @@ -271,7 +271,7 @@ unlock: */ if (cpu_q) *cpup = cpu_q->cpu; - put_cpu_var(sess->cpu_rr); + put_cpu_ptr(sess->cpu_rr); if (q) rnbd_clt_dev_requeue(q); -- cgit v1.2.3 From 3087b335b5316cd180aa4c5a28abaa890905634e Mon Sep 17 00:00:00 2001 From: Md Haris Iqbal Date: Mon, 26 Jul 2021 13:59:50 +0200 Subject: block/rnbd: Use sysfs_emit instead of s*printf function for sysfs show sysfs_emit function was added to be aware of the PAGE_SIZE maximum of the temporary buffer used for outputting sysfs content, so there is no possible overruns. So replace the uses of any s*printf functions for the sysfs show functions with sysfs_emit. Signed-off-by: Md Haris Iqbal Signed-off-by: Jack Wang Link: https://lore.kernel.org/r/20210726115950.470543-3-jinpu.wang@ionos.com Signed-off-by: Jens Axboe --- drivers/block/rnbd/rnbd-clt-sysfs.c | 33 +++++++++++++++------------------ drivers/block/rnbd/rnbd-srv-sysfs.c | 14 +++++++------- 2 files changed, 22 insertions(+), 25 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c index 324afdd63a96..4b93fd83bf79 100644 --- a/drivers/block/rnbd/rnbd-clt-sysfs.c +++ b/drivers/block/rnbd/rnbd-clt-sysfs.c @@ -227,17 +227,17 @@ static ssize_t state_show(struct kobject *kobj, switch (dev->dev_state) { case DEV_STATE_INIT: - return snprintf(page, PAGE_SIZE, "init\n"); + return sysfs_emit(page, "init\n"); case DEV_STATE_MAPPED: /* TODO fix cli tool before changing to proper state */ - return snprintf(page, PAGE_SIZE, "open\n"); + return sysfs_emit(page, "open\n"); case DEV_STATE_MAPPED_DISCONNECTED: /* TODO fix cli tool before changing to proper state */ - return snprintf(page, PAGE_SIZE, "closed\n"); + return sysfs_emit(page, "closed\n"); case DEV_STATE_UNMAPPED: - return snprintf(page, PAGE_SIZE, "unmapped\n"); + return sysfs_emit(page, "unmapped\n"); default: - return snprintf(page, PAGE_SIZE, "unknown\n"); + return sysfs_emit(page, "unknown\n"); } } @@ -263,7 +263,7 @@ static ssize_t mapping_path_show(struct kobject *kobj, dev = container_of(kobj, struct rnbd_clt_dev, kobj); - return scnprintf(page, PAGE_SIZE, "%s\n", dev->pathname); + return sysfs_emit(page, "%s\n", dev->pathname); } static struct kobj_attribute rnbd_clt_mapping_path_attr = @@ -276,8 +276,7 @@ static ssize_t access_mode_show(struct kobject *kobj, dev = container_of(kobj, struct rnbd_clt_dev, kobj); - return snprintf(page, PAGE_SIZE, "%s\n", - rnbd_access_mode_str(dev->access_mode)); + return sysfs_emit(page, "%s\n", rnbd_access_mode_str(dev->access_mode)); } static struct kobj_attribute rnbd_clt_access_mode = @@ -286,8 +285,8 @@ static struct kobj_attribute rnbd_clt_access_mode = static ssize_t rnbd_clt_unmap_dev_show(struct kobject *kobj, struct kobj_attribute *attr, char *page) { - return scnprintf(page, PAGE_SIZE, "Usage: echo > %s\n", - attr->attr.name); + return sysfs_emit(page, "Usage: echo > %s\n", + attr->attr.name); } static ssize_t rnbd_clt_unmap_dev_store(struct kobject *kobj, @@ -357,9 +356,8 @@ static ssize_t rnbd_clt_resize_dev_show(struct kobject *kobj, struct kobj_attribute *attr, char *page) { - return scnprintf(page, PAGE_SIZE, - "Usage: echo > %s\n", - attr->attr.name); + return sysfs_emit(page, "Usage: echo > %s\n", + attr->attr.name); } static ssize_t rnbd_clt_resize_dev_store(struct kobject *kobj, @@ -390,8 +388,7 @@ static struct kobj_attribute rnbd_clt_resize_dev_attr = static ssize_t rnbd_clt_remap_dev_show(struct kobject *kobj, struct kobj_attribute *attr, char *page) { - return scnprintf(page, PAGE_SIZE, "Usage: echo <1> > %s\n", - attr->attr.name); + return sysfs_emit(page, "Usage: echo <1> > %s\n", attr->attr.name); } static ssize_t rnbd_clt_remap_dev_store(struct kobject *kobj, @@ -436,7 +433,7 @@ static ssize_t session_show(struct kobject *kobj, struct kobj_attribute *attr, dev = container_of(kobj, struct rnbd_clt_dev, kobj); - return scnprintf(page, PAGE_SIZE, "%s\n", dev->sess->sessname); + return sysfs_emit(page, "%s\n", dev->sess->sessname); } static struct kobj_attribute rnbd_clt_session_attr = @@ -499,8 +496,8 @@ static ssize_t rnbd_clt_map_device_show(struct kobject *kobj, struct kobj_attribute *attr, char *page) { - return scnprintf(page, PAGE_SIZE, - "Usage: echo \"[dest_port=server port number] sessname= path=<[srcaddr@]dstaddr> [path=<[srcaddr@]dstaddr>] device_path= [access_mode=] [nr_poll_queues=]\" > %s\n\naddr ::= [ ip: | ip: | gid: ]\n", + return sysfs_emit(page, + "Usage: echo \"[dest_port=server port number] sessname= path=<[srcaddr@]dstaddr> [path=<[srcaddr@]dstaddr>] device_path= [access_mode=] [nr_poll_queues=]\" > %s\n\naddr ::= [ ip: | ip: | gid: ]\n", attr->attr.name); } diff --git a/drivers/block/rnbd/rnbd-srv-sysfs.c b/drivers/block/rnbd/rnbd-srv-sysfs.c index acf5fced11ef..4db98e0e76f0 100644 --- a/drivers/block/rnbd/rnbd-srv-sysfs.c +++ b/drivers/block/rnbd/rnbd-srv-sysfs.c @@ -90,8 +90,8 @@ static ssize_t read_only_show(struct kobject *kobj, struct kobj_attribute *attr, sess_dev = container_of(kobj, struct rnbd_srv_sess_dev, kobj); - return scnprintf(page, PAGE_SIZE, "%d\n", - !(sess_dev->open_flags & FMODE_WRITE)); + return sysfs_emit(page, "%d\n", + !(sess_dev->open_flags & FMODE_WRITE)); } static struct kobj_attribute rnbd_srv_dev_session_ro_attr = @@ -105,8 +105,8 @@ static ssize_t access_mode_show(struct kobject *kobj, sess_dev = container_of(kobj, struct rnbd_srv_sess_dev, kobj); - return scnprintf(page, PAGE_SIZE, "%s\n", - rnbd_access_mode_str(sess_dev->access_mode)); + return sysfs_emit(page, "%s\n", + rnbd_access_mode_str(sess_dev->access_mode)); } static struct kobj_attribute rnbd_srv_dev_session_access_mode_attr = @@ -119,7 +119,7 @@ static ssize_t mapping_path_show(struct kobject *kobj, sess_dev = container_of(kobj, struct rnbd_srv_sess_dev, kobj); - return scnprintf(page, PAGE_SIZE, "%s\n", sess_dev->pathname); + return sysfs_emit(page, "%s\n", sess_dev->pathname); } static struct kobj_attribute rnbd_srv_dev_session_mapping_path_attr = @@ -128,8 +128,8 @@ static struct kobj_attribute rnbd_srv_dev_session_mapping_path_attr = static ssize_t rnbd_srv_dev_session_force_close_show(struct kobject *kobj, struct kobj_attribute *attr, char *page) { - return scnprintf(page, PAGE_SIZE, "Usage: echo 1 > %s\n", - attr->attr.name); + return sysfs_emit(page, "Usage: echo 1 > %s\n", + attr->attr.name); } static ssize_t rnbd_srv_dev_session_force_close_store(struct kobject *kobj, -- cgit v1.2.3 From da20b58d5bbbb0d23ae9530992a37d0f0d1787a4 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Fri, 6 Aug 2021 12:06:01 +0100 Subject: xen-blkfront: Remove redundant assignment to variable err The variable err is being assigned a value that is never read, the assignment is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King Reviewed-by: Boris Ostrovsky Link: https://lore.kernel.org/r/20210806110601.11386-1-colin.king@canonical.com Signed-off-by: Jens Axboe --- drivers/block/xen-blkfront.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/block') diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index d83fee21f6c5..715bfa8aca7f 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -1092,7 +1092,6 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity, err = xlbd_reserve_minors(minor, nr_minors); if (err) return err; - err = -ENODEV; memset(&info->tag_set, 0, sizeof(info->tag_set)); info->tag_set.ops = &blkfront_mq_ops; -- cgit v1.2.3 From fad7cd3310db3099f95dd34312c77740fbc455e5 Mon Sep 17 00:00:00 2001 From: Baokun Li Date: Wed, 4 Aug 2021 10:12:12 +0800 Subject: nbd: add the check to prevent overflow in __nbd_ioctl() If user specify a large enough value of NBD blocks option, it may trigger signed integer overflow which may lead to nbd->config->bytesize becomes a large or small value, zero in particular. UBSAN: Undefined behaviour in drivers/block/nbd.c:325:31 signed integer overflow: 1024 * 4611686155866341414 cannot be represented in type 'long long int' [...] Call trace: [...] handle_overflow+0x188/0x1dc lib/ubsan.c:192 __ubsan_handle_mul_overflow+0x34/0x44 lib/ubsan.c:213 nbd_size_set drivers/block/nbd.c:325 [inline] __nbd_ioctl drivers/block/nbd.c:1342 [inline] nbd_ioctl+0x998/0xa10 drivers/block/nbd.c:1395 __blkdev_driver_ioctl block/ioctl.c:311 [inline] [...] Although it is not a big deal, still silence the UBSAN by limit the input value. Reported-by: Hulk Robot Signed-off-by: Baokun Li Reviewed-by: Josef Bacik Link: https://lore.kernel.org/r/20210804021212.990223-1-libaokun1@huawei.com [axboe: dropped unlikely()] Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index c38317979f74..f82264835794 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1384,6 +1384,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, unsigned int cmd, unsigned long arg) { struct nbd_config *config = nbd->config; + loff_t bytesize; switch (cmd) { case NBD_DISCONNECT: @@ -1398,8 +1399,9 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, case NBD_SET_SIZE: return nbd_set_size(nbd, arg, config->blksize); case NBD_SET_SIZE_BLOCKS: - return nbd_set_size(nbd, arg * config->blksize, - config->blksize); + if (check_mul_overflow((loff_t)arg, config->blksize, &bytesize)) + return -EINVAL; + return nbd_set_size(nbd, bytesize, config->blksize); case NBD_SET_TIMEOUT: nbd_set_cmd_timeout(nbd, arg); return 0; -- cgit v1.2.3 From 68c9417b193d0d174b0ada013602272177e61303 Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Wed, 11 Aug 2021 14:44:23 +0200 Subject: nbd: do del_gendisk() asynchronously for NBD_DESTROY_ON_DISCONNECT Now open_mutex is used to synchronize partition operations (e.g, blk_drop_partitions() and blkdev_reread_part()), however it makes nbd driver broken, because nbd may call del_gendisk() in nbd_release() or nbd_genl_disconnect() if NBD_CFLAG_DESTROY_ON_DISCONNECT is enabled, and deadlock occurs, as shown below: // AB-BA dead-lock nbd_genl_disconnect blkdev_open nbd_disconnect_and_put lock bd_mutex // last ref nbd_put lock nbd_index_mutex del_gendisk nbd_open try lock nbd_index_mutex try lock bd_mutex or // AA dead-lock nbd_release lock bd_mutex nbd_put try lock bd_mutex Instead of fixing block layer (e.g, introduce another lock), fixing the nbd driver to call del_gendisk() in a kworker when NBD_DESTROY_ON_DISCONNECT is enabled. When NBD_DESTROY_ON_DISCONNECT is disabled, nbd device will always be destroy through module removal, and there is no risky of deadlock. To ensure the reuse of nbd index succeeds, moving the calling of idr_remove() after del_gendisk(), so if the reused index is not found in nbd_index_idr, the old disk must have been deleted. And reusing the existing destroy_complete mechanism to ensure nbd_genl_connect() will wait for the completion of del_gendisk(). Also adding a new workqueue for nbd removal, so nbd_cleanup() can ensure all removals complete before exits. Reported-by: syzbot+0fe7752e52337864d29b@syzkaller.appspotmail.com Fixes: c76f48eb5c08 ("block: take bd_mutex around delete_partitions in del_gendisk") Signed-off-by: Hou Tao Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210811124428.2368491-2-hch@lst.de Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 61 insertions(+), 9 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index f82264835794..deefb2cda9bb 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -49,6 +49,7 @@ static DEFINE_IDR(nbd_index_idr); static DEFINE_MUTEX(nbd_index_mutex); +static struct workqueue_struct *nbd_del_wq; static int nbd_total_devices = 0; struct nbd_sock { @@ -113,6 +114,7 @@ struct nbd_device { struct mutex config_lock; struct gendisk *disk; struct workqueue_struct *recv_workq; + struct work_struct remove_work; struct list_head list; struct task_struct *task_recv; @@ -233,7 +235,7 @@ static const struct device_attribute backend_attr = { .show = backend_show, }; -static void nbd_dev_remove(struct nbd_device *nbd) +static void nbd_del_disk(struct nbd_device *nbd) { struct gendisk *disk = nbd->disk; @@ -242,24 +244,60 @@ static void nbd_dev_remove(struct nbd_device *nbd) blk_cleanup_disk(disk); blk_mq_free_tag_set(&nbd->tag_set); } +} +/* + * Place this in the last just before the nbd is freed to + * make sure that the disk and the related kobject are also + * totally removed to avoid duplicate creation of the same + * one. + */ +static void nbd_notify_destroy_completion(struct nbd_device *nbd) +{ + if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) && + nbd->destroy_complete) + complete(nbd->destroy_complete); +} + +static void nbd_dev_remove_work(struct work_struct *work) +{ + struct nbd_device *nbd = + container_of(work, struct nbd_device, remove_work); + + nbd_del_disk(nbd); + + mutex_lock(&nbd_index_mutex); /* - * Place this in the last just before the nbd is freed to - * make sure that the disk and the related kobject are also - * totally removed to avoid duplicate creation of the same - * one. + * Remove from idr after del_gendisk() completes, + * so if the same id is reused, the following + * add_disk() will succeed. */ - if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) && nbd->destroy_complete) - complete(nbd->destroy_complete); + idr_remove(&nbd_index_idr, nbd->index); + + nbd_notify_destroy_completion(nbd); + mutex_unlock(&nbd_index_mutex); kfree(nbd); } +static void nbd_dev_remove(struct nbd_device *nbd) +{ + /* Call del_gendisk() asynchrounously to prevent deadlock */ + if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags)) { + queue_work(nbd_del_wq, &nbd->remove_work); + return; + } + + nbd_del_disk(nbd); + idr_remove(&nbd_index_idr, nbd->index); + nbd_notify_destroy_completion(nbd); + kfree(nbd); +} + static void nbd_put(struct nbd_device *nbd) { if (refcount_dec_and_mutex_lock(&nbd->refs, &nbd_index_mutex)) { - idr_remove(&nbd_index_idr, nbd->index); nbd_dev_remove(nbd); mutex_unlock(&nbd_index_mutex); } @@ -1681,6 +1719,7 @@ static int nbd_dev_add(int index) nbd->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_BLOCKING; nbd->tag_set.driver_data = nbd; + INIT_WORK(&nbd->remove_work, nbd_dev_remove_work); nbd->destroy_complete = NULL; nbd->backend = NULL; @@ -2418,7 +2457,14 @@ static int __init nbd_init(void) if (register_blkdev(NBD_MAJOR, "nbd")) return -EIO; + nbd_del_wq = alloc_workqueue("nbd-del", WQ_UNBOUND, 0); + if (!nbd_del_wq) { + unregister_blkdev(NBD_MAJOR, "nbd"); + return -ENOMEM; + } + if (genl_register_family(&nbd_genl_family)) { + destroy_workqueue(nbd_del_wq); unregister_blkdev(NBD_MAJOR, "nbd"); return -EINVAL; } @@ -2436,7 +2482,10 @@ static int nbd_exit_cb(int id, void *ptr, void *data) struct list_head *list = (struct list_head *)data; struct nbd_device *nbd = ptr; - list_add_tail(&nbd->list, list); + /* Skip nbd that is being removed asynchronously */ + if (refcount_read(&nbd->refs)) + list_add_tail(&nbd->list, list); + return 0; } @@ -2459,6 +2508,9 @@ static void __exit nbd_cleanup(void) nbd_put(nbd); } + /* Also wait for nbd_dev_remove_work() completes */ + destroy_workqueue(nbd_del_wq); + idr_destroy(&nbd_index_idr); genl_unregister_family(&nbd_genl_family); unregister_blkdev(NBD_MAJOR, "nbd"); -- cgit v1.2.3 From 3f74e0645c52a08f640380c9c46f9a3a172b9389 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 11 Aug 2021 14:44:24 +0200 Subject: nbd: refactor device removal Share common code for the synchronous and workqueue based device removal, and remove the pointless use of refcount_dec_and_mutex_lock. Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210811124428.2368491-3-hch@lst.de Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index deefb2cda9bb..a9883fbed924 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -259,48 +259,37 @@ static void nbd_notify_destroy_completion(struct nbd_device *nbd) complete(nbd->destroy_complete); } -static void nbd_dev_remove_work(struct work_struct *work) +static void nbd_dev_remove(struct nbd_device *nbd) { - struct nbd_device *nbd = - container_of(work, struct nbd_device, remove_work); - nbd_del_disk(nbd); - mutex_lock(&nbd_index_mutex); /* - * Remove from idr after del_gendisk() completes, - * so if the same id is reused, the following - * add_disk() will succeed. + * Remove from idr after del_gendisk() completes, so if the same ID is + * reused, the following add_disk() will succeed. */ + mutex_lock(&nbd_index_mutex); idr_remove(&nbd_index_idr, nbd->index); - nbd_notify_destroy_completion(nbd); mutex_unlock(&nbd_index_mutex); kfree(nbd); } -static void nbd_dev_remove(struct nbd_device *nbd) +static void nbd_dev_remove_work(struct work_struct *work) { - /* Call del_gendisk() asynchrounously to prevent deadlock */ - if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags)) { - queue_work(nbd_del_wq, &nbd->remove_work); - return; - } - - nbd_del_disk(nbd); - idr_remove(&nbd_index_idr, nbd->index); - nbd_notify_destroy_completion(nbd); - kfree(nbd); + nbd_dev_remove(container_of(work, struct nbd_device, remove_work)); } static void nbd_put(struct nbd_device *nbd) { - if (refcount_dec_and_mutex_lock(&nbd->refs, - &nbd_index_mutex)) { + if (!refcount_dec_and_test(&nbd->refs)) + return; + + /* Call del_gendisk() asynchrounously to prevent deadlock */ + if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags)) + queue_work(nbd_del_wq, &nbd->remove_work); + else nbd_dev_remove(nbd); - mutex_unlock(&nbd_index_mutex); - } } static int nbd_disconnected(struct nbd_config *config) -- cgit v1.2.3 From 327b501b1d94342fe17a1b6b1a40746e57ddd472 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 11 Aug 2021 14:44:25 +0200 Subject: nbd: remove nbd_del_disk Fold nbd_del_disk and remove the pointless NULL check on ->disk given that it is always set for a successfully allocated nbd_device structure. Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210811124428.2368491-4-hch@lst.de Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index a9883fbed924..48530fe01c0f 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -235,17 +235,6 @@ static const struct device_attribute backend_attr = { .show = backend_show, }; -static void nbd_del_disk(struct nbd_device *nbd) -{ - struct gendisk *disk = nbd->disk; - - if (disk) { - del_gendisk(disk); - blk_cleanup_disk(disk); - blk_mq_free_tag_set(&nbd->tag_set); - } -} - /* * Place this in the last just before the nbd is freed to * make sure that the disk and the related kobject are also @@ -261,7 +250,11 @@ static void nbd_notify_destroy_completion(struct nbd_device *nbd) static void nbd_dev_remove(struct nbd_device *nbd) { - nbd_del_disk(nbd); + struct gendisk *disk = nbd->disk; + + del_gendisk(disk); + blk_cleanup_disk(disk); + blk_mq_free_tag_set(&nbd->tag_set); /* * Remove from idr after del_gendisk() completes, so if the same ID is -- cgit v1.2.3 From 7bdc00cf7e369b3be17f26e5643da28de98d9d6d Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 11 Aug 2021 14:44:26 +0200 Subject: nbd: return the allocated nbd_device from nbd_dev_add Return the device we just allocated instead of doing an extra search for it in the caller. Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210811124428.2368491-5-hch@lst.de Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 48530fe01c0f..a81b95c66dbf 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1683,7 +1683,7 @@ static const struct blk_mq_ops nbd_mq_ops = { .timeout = nbd_xmit_timeout, }; -static int nbd_dev_add(int index) +static struct nbd_device *nbd_dev_add(int index) { struct nbd_device *nbd; struct gendisk *disk; @@ -1755,7 +1755,7 @@ static int nbd_dev_add(int index) sprintf(disk->disk_name, "nbd%d", index); add_disk(disk); nbd_total_devices++; - return index; + return nbd; out_free_idr: idr_remove(&nbd_index_idr, index); @@ -1764,7 +1764,7 @@ out_free_tags: out_free_nbd: kfree(nbd); out: - return err; + return ERR_PTR(err); } static int find_free_cb(int id, void *ptr, void *data) @@ -1850,25 +1850,22 @@ again: if (index == -1) { ret = idr_for_each(&nbd_index_idr, &find_free_cb, &nbd); if (ret == 0) { - int new_index; - new_index = nbd_dev_add(-1); - if (new_index < 0) { + nbd = nbd_dev_add(-1); + if (IS_ERR(nbd)) { mutex_unlock(&nbd_index_mutex); printk(KERN_ERR "nbd: failed to add new device\n"); - return new_index; + return PTR_ERR(nbd); } - nbd = idr_find(&nbd_index_idr, new_index); } } else { nbd = idr_find(&nbd_index_idr, index); if (!nbd) { - ret = nbd_dev_add(index); - if (ret < 0) { + nbd = nbd_dev_add(index); + if (IS_ERR(nbd)) { mutex_unlock(&nbd_index_mutex); printk(KERN_ERR "nbd: failed to add new device\n"); - return ret; + return PTR_ERR(nbd); } - nbd = idr_find(&nbd_index_idr, index); } } if (!nbd) { -- cgit v1.2.3 From 6177b56c96ff3b5e23d47f6b6c8630f31145da93 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 11 Aug 2021 14:44:27 +0200 Subject: nbd: refactor device search and allocation in nbd_genl_connect Use idr_for_each_entry instead of the awkward callback to find an existing device for the index == -1 case, and de-duplicate the device allocation if no existing device was found. Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210811124428.2368491-6-hch@lst.de Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 45 ++++++++++++++------------------------------- 1 file changed, 14 insertions(+), 31 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index a81b95c66dbf..8c0e334bdfbf 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1767,18 +1767,6 @@ out: return ERR_PTR(err); } -static int find_free_cb(int id, void *ptr, void *data) -{ - struct nbd_device *nbd = ptr; - struct nbd_device **found = data; - - if (!refcount_read(&nbd->config_refs)) { - *found = nbd; - return 1; - } - return 0; -} - /* Netlink interface. */ static const struct nla_policy nbd_attr_policy[NBD_ATTR_MAX + 1] = { [NBD_ATTR_INDEX] = { .type = NLA_U32 }, @@ -1848,31 +1836,26 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) again: mutex_lock(&nbd_index_mutex); if (index == -1) { - ret = idr_for_each(&nbd_index_idr, &find_free_cb, &nbd); - if (ret == 0) { - nbd = nbd_dev_add(-1); - if (IS_ERR(nbd)) { - mutex_unlock(&nbd_index_mutex); - printk(KERN_ERR "nbd: failed to add new device\n"); - return PTR_ERR(nbd); + struct nbd_device *tmp; + int id; + + idr_for_each_entry(&nbd_index_idr, tmp, id) { + if (!refcount_read(&tmp->config_refs)) { + nbd = tmp; + break; } } } else { nbd = idr_find(&nbd_index_idr, index); - if (!nbd) { - nbd = nbd_dev_add(index); - if (IS_ERR(nbd)) { - mutex_unlock(&nbd_index_mutex); - printk(KERN_ERR "nbd: failed to add new device\n"); - return PTR_ERR(nbd); - } - } } + if (!nbd) { - printk(KERN_ERR "nbd: couldn't find device at index %d\n", - index); - mutex_unlock(&nbd_index_mutex); - return -EINVAL; + nbd = nbd_dev_add(index); + if (IS_ERR(nbd)) { + mutex_unlock(&nbd_index_mutex); + pr_err("nbd: failed to add new device\n"); + return PTR_ERR(nbd); + } } if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) && -- cgit v1.2.3 From 6e4df4c6488165637b95b9701cc862a42a3836ba Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 11 Aug 2021 14:44:28 +0200 Subject: nbd: reduce the nbd_index_mutex scope nbd_index_mutex is currently held over add_disk and inside ->open, which leads to lock order reversals. Refactor the device creation code path so that nbd_dev_add is called without nbd_index_mutex lock held and only takes it for the IDR insertation. Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210811124428.2368491-7-hch@lst.de [axboe: fix whitespace] Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 55 +++++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 27 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 8c0e334bdfbf..0fe82626bf70 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1683,7 +1683,7 @@ static const struct blk_mq_ops nbd_mq_ops = { .timeout = nbd_xmit_timeout, }; -static struct nbd_device *nbd_dev_add(int index) +static struct nbd_device *nbd_dev_add(int index, unsigned int refs) { struct nbd_device *nbd; struct gendisk *disk; @@ -1709,6 +1709,7 @@ static struct nbd_device *nbd_dev_add(int index) if (err) goto out_free_nbd; + mutex_lock(&nbd_index_mutex); if (index >= 0) { err = idr_alloc(&nbd_index_idr, nbd, index, index + 1, GFP_KERNEL); @@ -1719,6 +1720,7 @@ static struct nbd_device *nbd_dev_add(int index) if (err >= 0) index = err; } + mutex_unlock(&nbd_index_mutex); if (err < 0) goto out_free_tags; nbd->index = index; @@ -1745,7 +1747,7 @@ static struct nbd_device *nbd_dev_add(int index) mutex_init(&nbd->config_lock); refcount_set(&nbd->config_refs, 0); - refcount_set(&nbd->refs, 1); + refcount_set(&nbd->refs, refs); INIT_LIST_HEAD(&nbd->list); disk->major = NBD_MAJOR; disk->first_minor = index << part_shift; @@ -1849,34 +1851,35 @@ again: nbd = idr_find(&nbd_index_idr, index); } - if (!nbd) { - nbd = nbd_dev_add(index); - if (IS_ERR(nbd)) { + if (nbd) { + if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) && + test_bit(NBD_DISCONNECT_REQUESTED, &nbd->flags)) { + nbd->destroy_complete = &destroy_complete; mutex_unlock(&nbd_index_mutex); - pr_err("nbd: failed to add new device\n"); - return PTR_ERR(nbd); + + /* wait until the nbd device is completely destroyed */ + wait_for_completion(&destroy_complete); + goto again; } - } - if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) && - test_bit(NBD_DISCONNECT_REQUESTED, &nbd->flags)) { - nbd->destroy_complete = &destroy_complete; + if (!refcount_inc_not_zero(&nbd->refs)) { + mutex_unlock(&nbd_index_mutex); + if (index == -1) + goto again; + pr_err("nbd: device at index %d is going down\n", + index); + return -EINVAL; + } mutex_unlock(&nbd_index_mutex); - - /* Wait untill the the nbd stuff is totally destroyed */ - wait_for_completion(&destroy_complete); - goto again; - } - - if (!refcount_inc_not_zero(&nbd->refs)) { + } else { mutex_unlock(&nbd_index_mutex); - if (index == -1) - goto again; - printk(KERN_ERR "nbd: device at index %d is going down\n", - index); - return -EINVAL; + + nbd = nbd_dev_add(index, 2); + if (IS_ERR(nbd)) { + pr_err("nbd: failed to add new device\n"); + return PTR_ERR(nbd); + } } - mutex_unlock(&nbd_index_mutex); mutex_lock(&nbd->config_lock); if (refcount_read(&nbd->config_refs)) { @@ -2432,10 +2435,8 @@ static int __init nbd_init(void) } nbd_dbg_init(); - mutex_lock(&nbd_index_mutex); for (i = 0; i < nbds_max; i++) - nbd_dev_add(i); - mutex_unlock(&nbd_index_mutex); + nbd_dev_add(i, 1); return 0; } -- cgit v1.2.3 From b1a811633f7321cf1ae2bb76a66805b7720e44c9 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Thu, 12 Aug 2021 12:15:01 +0300 Subject: block: nbd: add sanity check for first_minor Syzbot hit WARNING in internal_create_group(). The problem was in too big disk->first_minor. disk->first_minor is initialized by value, which comes from userspace and there wasn't any sanity checks about value correctness. It can cause duplicate creation of sysfs files/links, because disk->first_minor will be passed to MKDEV() which causes truncation to byte. Since maximum minor value is 0xff, let's check if first_minor is correct minor number. NOTE: the root case of the reported warning was in wrong error handling in register_disk(), but we can avoid passing knowingly wrong values to sysfs API, because sysfs error messages can confuse users. For example: user passed 1048576 as index, but sysfs complains about duplicate creation of /dev/block/43:0. It's not obvious how 1048576 becomes 0. Log and reproducer for above example can be found on syzkaller bug report page. Link: https://syzkaller.appspot.com/bug?id=03c2ae9146416edf811958d5fd7acfab75b143d1 Fixes: b0d9111a2d53 ("nbd: use an idr to keep track of nbd devices") Reported-by: syzbot+9937dc42271cd87d4b98@syzkaller.appspotmail.com Reviewed-by: Christoph Hellwig Signed-off-by: Pavel Skripkin Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 0fe82626bf70..379032a64a7c 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1750,7 +1750,17 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs) refcount_set(&nbd->refs, refs); INIT_LIST_HEAD(&nbd->list); disk->major = NBD_MAJOR; + + /* Too big first_minor can cause duplicate creation of + * sysfs files/links, since first_minor will be truncated to + * byte in __device_add_disk(). + */ disk->first_minor = index << part_shift; + if (disk->first_minor > 0xff) { + err = -EINVAL; + goto out_free_idr; + } + disk->minors = 1 << part_shift; disk->fops = &nbd_fops; disk->private_data = nbd; -- cgit v1.2.3 From 93f63bc41f699318807df202a175d564c26bda87 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Wed, 25 Aug 2021 18:31:03 +0200 Subject: nbd: add missing locking to the nbd_dev_add error path idr_remove needs external synchronization. Fixes: 6e4df4c64881 ("nbd: reduce the nbd_index_mutex scope") Signed-off-by: Tetsuo Handa [hch: split from a larger patch] Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210825163108.50713-2-hch@lst.de Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 379032a64a7c..0c1389da3066 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1770,7 +1770,9 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs) return nbd; out_free_idr: + mutex_lock(&nbd_index_mutex); idr_remove(&nbd_index_idr, index); + mutex_unlock(&nbd_index_mutex); out_free_tags: blk_mq_free_tag_set(&nbd->tag_set); out_free_nbd: -- cgit v1.2.3 From 409e0ff10ead30a620ee48acb6d4545d9cb95359 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 25 Aug 2021 18:31:04 +0200 Subject: nbd: reset NBD to NULL when restarting in nbd_genl_connect When nbd_genl_connect restarts to wait for a disconnecting device, nbd needs to be reset to NULL. Do that by facoring out a helper to find an unused device. Fixes: 6177b56c96ff ("nbd: refactor device search and allocation in nbd_genl_connect") Reported-by: Tetsuo Handa Reported-by: Hillf Danton Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210825163108.50713-3-hch@lst.de Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 0c1389da3066..938ca7f5a11f 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1781,6 +1781,20 @@ out: return ERR_PTR(err); } +static struct nbd_device *nbd_find_unused(void) +{ + struct nbd_device *nbd; + int id; + + lockdep_assert_held(&nbd_index_mutex); + + idr_for_each_entry(&nbd_index_idr, nbd, id) + if (!refcount_read(&nbd->config_refs)) + return nbd; + + return NULL; +} + /* Netlink interface. */ static const struct nla_policy nbd_attr_policy[NBD_ATTR_MAX + 1] = { [NBD_ATTR_INDEX] = { .type = NLA_U32 }, @@ -1828,7 +1842,7 @@ static int nbd_genl_size_set(struct genl_info *info, struct nbd_device *nbd) static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) { DECLARE_COMPLETION_ONSTACK(destroy_complete); - struct nbd_device *nbd = NULL; + struct nbd_device *nbd; struct nbd_config *config; int index = -1; int ret; @@ -1849,20 +1863,10 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) } again: mutex_lock(&nbd_index_mutex); - if (index == -1) { - struct nbd_device *tmp; - int id; - - idr_for_each_entry(&nbd_index_idr, tmp, id) { - if (!refcount_read(&tmp->config_refs)) { - nbd = tmp; - break; - } - } - } else { + if (index == -1) + nbd = nbd_find_unused(); + else nbd = idr_find(&nbd_index_idr, index); - } - if (nbd) { if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) && test_bit(NBD_DISCONNECT_REQUESTED, &nbd->flags)) { -- cgit v1.2.3 From 75b7f62aa65d5c496391ec2c3db3561aaf81a403 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Wed, 25 Aug 2021 18:31:05 +0200 Subject: nbd: prevent IDR lookups from finding partially initialized devices Previously nbd_index_mutex was held during whole add/remove/lookup operations in order to guarantee that partially initialized devices are not reachable via idr_find() or idr_for_each(). But now that partially initialized devices become reachable as soon as idr_alloc() succeeds, we need to skip partially initialized devices. Since it seems that all functions use refcount_inc_not_zero(&nbd->refs) in order to skip destroying devices, update nbd->refs from zero to non-zero as the last step of device initialization in order to also skip partially initialized devices. Fixes: 6e4df4c64881 ("nbd: reduce the nbd_index_mutex scope") Signed-off-by: Tetsuo Handa [hch: split from a larger patch, added comments] Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210825163108.50713-4-hch@lst.de Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 938ca7f5a11f..b1ed2360ef32 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1747,7 +1747,11 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs) mutex_init(&nbd->config_lock); refcount_set(&nbd->config_refs, 0); - refcount_set(&nbd->refs, refs); + /* + * Start out with a zero references to keep other threads from using + * this device until it is fully initialized. + */ + refcount_set(&nbd->refs, 0); INIT_LIST_HEAD(&nbd->list); disk->major = NBD_MAJOR; @@ -1766,6 +1770,11 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs) disk->private_data = nbd; sprintf(disk->disk_name, "nbd%d", index); add_disk(disk); + + /* + * Now publish the device. + */ + refcount_set(&nbd->refs, refs); nbd_total_devices++; return nbd; -- cgit v1.2.3 From b190300decb352a0b865d7aa379e89b17d772a43 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Wed, 25 Aug 2021 18:31:06 +0200 Subject: nbd: set nbd->index before releasing nbd_index_mutex Set nbd->index before releasing nbd_index_mutex, as populate_nbd_status() might access nbd->index as soon as nbd_index_mutex is released. Fixes: 6e4df4c64881 ("nbd: reduce the nbd_index_mutex scope") Signed-off-by: Tetsuo Handa [hch: split from a larger patch] Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210825163108.50713-5-hch@lst.de Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index b1ed2360ef32..6a832bf81647 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1720,10 +1720,10 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs) if (err >= 0) index = err; } + nbd->index = index; mutex_unlock(&nbd_index_mutex); if (err < 0) goto out_free_tags; - nbd->index = index; disk = blk_mq_alloc_disk(&nbd->tag_set, NULL); if (IS_ERR(disk)) { -- cgit v1.2.3 From 438cd318c8dfa5228ffd43af1b98d7cd7d92e1c6 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 25 Aug 2021 18:31:07 +0200 Subject: nbd: only return usable devices from nbd_find_unused Device marked as NBD_DESTROY_ON_DISCONNECT can and should be skipped given that they won't survive the disconnect. So skip them and try to grab a reference directly and just continue if the the devices is being torn down or created and thus has a zero refcount. Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210825163108.50713-6-hch@lst.de Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 6a832bf81647..70dc9d80a173 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1790,16 +1790,20 @@ out: return ERR_PTR(err); } -static struct nbd_device *nbd_find_unused(void) +static struct nbd_device *nbd_find_get_unused(void) { struct nbd_device *nbd; int id; lockdep_assert_held(&nbd_index_mutex); - idr_for_each_entry(&nbd_index_idr, nbd, id) - if (!refcount_read(&nbd->config_refs)) + idr_for_each_entry(&nbd_index_idr, nbd, id) { + if (refcount_read(&nbd->config_refs) || + test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags)) + continue; + if (refcount_inc_not_zero(&nbd->refs)) return nbd; + } return NULL; } @@ -1873,10 +1877,10 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) again: mutex_lock(&nbd_index_mutex); if (index == -1) - nbd = nbd_find_unused(); + nbd = nbd_find_get_unused(); else nbd = idr_find(&nbd_index_idr, index); - if (nbd) { + if (nbd && index != -1) { if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) && test_bit(NBD_DISCONNECT_REQUESTED, &nbd->flags)) { nbd->destroy_complete = &destroy_complete; @@ -1889,8 +1893,6 @@ again: if (!refcount_inc_not_zero(&nbd->refs)) { mutex_unlock(&nbd_index_mutex); - if (index == -1) - goto again; pr_err("nbd: device at index %d is going down\n", index); return -EINVAL; -- cgit v1.2.3 From 7ee656c3ac3d047b4cf1269f83ac9d6c0bba916b Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 25 Aug 2021 18:31:08 +0200 Subject: nbd: remove nbd->destroy_complete The nbd->destroy_complete pointer is not really needed. For creating a device without a specific index we now simplify skip devices marked NBD_DESTROY_ON_DISCONNECT as there is not much point to reuse them. For device creation with a specific index there is no real need to treat the case of a requested but not finished disconnect different than any other device that is being shutdown, i.e. we can just return an error, as a slightly different race window would anyway. Fixes: 6e4df4c64881 ("nbd: reduce the nbd_index_mutex scope") Reported-by: Tetsuo Handa Reported-by: syzbot+2c98885bcd769f56b6d6@syzkaller.appspotmail.com Signed-off-by: Christoph Hellwig Link: https://lore.kernel.org/r/20210825163108.50713-7-hch@lst.de Reviewed-by: Josef Bacik Signed-off-by: Jens Axboe --- drivers/block/nbd.c | 52 ++++++++++++++-------------------------------------- 1 file changed, 14 insertions(+), 38 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 70dc9d80a173..44143068c91e 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -120,7 +120,6 @@ struct nbd_device { struct task_struct *task_recv; struct task_struct *task_setup; - struct completion *destroy_complete; unsigned long flags; char *backend; @@ -235,19 +234,6 @@ static const struct device_attribute backend_attr = { .show = backend_show, }; -/* - * Place this in the last just before the nbd is freed to - * make sure that the disk and the related kobject are also - * totally removed to avoid duplicate creation of the same - * one. - */ -static void nbd_notify_destroy_completion(struct nbd_device *nbd) -{ - if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) && - nbd->destroy_complete) - complete(nbd->destroy_complete); -} - static void nbd_dev_remove(struct nbd_device *nbd) { struct gendisk *disk = nbd->disk; @@ -262,7 +248,6 @@ static void nbd_dev_remove(struct nbd_device *nbd) */ mutex_lock(&nbd_index_mutex); idr_remove(&nbd_index_idr, nbd->index); - nbd_notify_destroy_completion(nbd); mutex_unlock(&nbd_index_mutex); kfree(nbd); @@ -1702,7 +1687,6 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs) BLK_MQ_F_BLOCKING; nbd->tag_set.driver_data = nbd; INIT_WORK(&nbd->remove_work, nbd_dev_remove_work); - nbd->destroy_complete = NULL; nbd->backend = NULL; err = blk_mq_alloc_tag_set(&nbd->tag_set); @@ -1854,7 +1838,6 @@ static int nbd_genl_size_set(struct genl_info *info, struct nbd_device *nbd) static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) { - DECLARE_COMPLETION_ONSTACK(destroy_complete); struct nbd_device *nbd; struct nbd_config *config; int index = -1; @@ -1876,31 +1859,24 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) } again: mutex_lock(&nbd_index_mutex); - if (index == -1) + if (index == -1) { nbd = nbd_find_get_unused(); - else + } else { nbd = idr_find(&nbd_index_idr, index); - if (nbd && index != -1) { - if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) && - test_bit(NBD_DISCONNECT_REQUESTED, &nbd->flags)) { - nbd->destroy_complete = &destroy_complete; - mutex_unlock(&nbd_index_mutex); - - /* wait until the nbd device is completely destroyed */ - wait_for_completion(&destroy_complete); - goto again; - } - - if (!refcount_inc_not_zero(&nbd->refs)) { - mutex_unlock(&nbd_index_mutex); - pr_err("nbd: device at index %d is going down\n", - index); - return -EINVAL; + if (nbd) { + if ((test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) && + test_bit(NBD_DISCONNECT_REQUESTED, &nbd->flags)) || + !refcount_inc_not_zero(&nbd->refs)) { + mutex_unlock(&nbd_index_mutex); + pr_err("nbd: device at index %d is going down\n", + index); + return -EINVAL; + } } - mutex_unlock(&nbd_index_mutex); - } else { - mutex_unlock(&nbd_index_mutex); + } + mutex_unlock(&nbd_index_mutex); + if (!nbd) { nbd = nbd_dev_add(index, 2); if (IS_ERR(nbd)) { pr_err("nbd: failed to add new device\n"); -- cgit v1.2.3 From c7e9d0020361f4308a70cdfd6d5335e273eb8717 Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Sat, 7 Aug 2021 10:37:02 +0300 Subject: Revert "floppy: reintroduce O_NDELAY fix" The patch breaks userspace implementations (e.g. fdutils) and introduces regressions in behaviour. Previously, it was possible to O_NDELAY open a floppy device with no media inserted or with write protected media without an error. Some userspace tools use this particular behavior for probing. It's not the first time when we revert this patch. Previous revert is in commit f2791e7eadf4 (Revert "floppy: refactor open() flags handling"). This reverts commit 8a0c014cd20516ade9654fc13b51345ec58e7be8. Link: https://lore.kernel.org/linux-block/de10cb47-34d1-5a88-7751-225ca380f735@compro.net/ Reported-by: Mark Hounschell Cc: Jiri Kosina Cc: Wim Osterholt Cc: Kurt Garloff Cc: Signed-off-by: Denis Efremov --- drivers/block/floppy.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c index 87460e0e5c72..fef79ea52e3e 100644 --- a/drivers/block/floppy.c +++ b/drivers/block/floppy.c @@ -4029,23 +4029,23 @@ static int floppy_open(struct block_device *bdev, fmode_t mode) if (fdc_state[FDC(drive)].rawcmd == 1) fdc_state[FDC(drive)].rawcmd = 2; - if (mode & (FMODE_READ|FMODE_WRITE)) { - drive_state[drive].last_checked = 0; - clear_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags); - if (bdev_check_media_change(bdev)) - floppy_revalidate(bdev->bd_disk); - if (test_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags)) - goto out; - if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags)) + if (!(mode & FMODE_NDELAY)) { + if (mode & (FMODE_READ|FMODE_WRITE)) { + drive_state[drive].last_checked = 0; + clear_bit(FD_OPEN_SHOULD_FAIL_BIT, + &drive_state[drive].flags); + if (bdev_check_media_change(bdev)) + floppy_revalidate(bdev->bd_disk); + if (test_bit(FD_DISK_CHANGED_BIT, &drive_state[drive].flags)) + goto out; + if (test_bit(FD_OPEN_SHOULD_FAIL_BIT, &drive_state[drive].flags)) + goto out; + } + res = -EROFS; + if ((mode & FMODE_WRITE) && + !test_bit(FD_DISK_WRITABLE_BIT, &drive_state[drive].flags)) goto out; } - - res = -EROFS; - - if ((mode & FMODE_WRITE) && - !test_bit(FD_DISK_WRITABLE_BIT, &drive_state[drive].flags)) - goto out; - mutex_unlock(&open_lock); mutex_unlock(&floppy_mutex); return 0; -- cgit v1.2.3