* [PATCH v4 1/8] blk-mq: Fix a comment in include/linux/blk-mq.h
2020-03-10 4:26 [PATCH v4 0/8] Improve changing the number of hardware queues Bart Van Assche
@ 2020-03-10 4:26 ` Bart Van Assche
2020-03-10 4:26 ` [PATCH v4 2/8] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync Bart Van Assche
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2020-03-10 4:26 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
André Almeida, Chaitanya Kulkarni
The 'hctx_list' member of struct blk_mq_hw_ctx is not a list head but
instead an entry in q->unused_hctx_list. Fix the comment above this
struct member.
Cc: André Almeida <andrealmeid@collabora.com>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Fixes: d386732bc142 ("blk-mq: fill header with kernel-doc")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
include/linux/blk-mq.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 11cfd6470b1a..31344d5f83e2 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -162,7 +162,10 @@ struct blk_mq_hw_ctx {
struct dentry *sched_debugfs_dir;
#endif
- /** @hctx_list: List of all hardware queues. */
+ /**
+ * @hctx_list: if this hctx is not in use, this is an entry in
+ * q->unused_hctx_list.
+ */
struct list_head hctx_list;
/**
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 2/8] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync
2020-03-10 4:26 [PATCH v4 0/8] Improve changing the number of hardware queues Bart Van Assche
2020-03-10 4:26 ` [PATCH v4 1/8] blk-mq: Fix a comment in include/linux/blk-mq.h Bart Van Assche
@ 2020-03-10 4:26 ` Bart Van Assche
2020-03-10 4:26 ` [PATCH v4 3/8] blk-mq: Fix a recently introduced regression in blk_mq_realloc_hw_ctxs() Bart Van Assche
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2020-03-10 4:26 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn,
Chaitanya Kulkarni, syzbot+d44e1b26ce5c3e77458d
blk_mq_map_queues() and multiple .map_queues() implementations expect that
set->map[HCTX_TYPE_DEFAULT].nr_queues is set to the number of hardware
queues. Hence set .nr_queues before calling these functions. This patch
fixes the following kernel warning:
WARNING: CPU: 0 PID: 2501 at include/linux/cpumask.h:137
Call Trace:
blk_mq_run_hw_queue+0x19d/0x350 block/blk-mq.c:1508
blk_mq_run_hw_queues+0x112/0x1a0 block/blk-mq.c:1525
blk_mq_requeue_work+0x502/0x780 block/blk-mq.c:775
process_one_work+0x9af/0x1740 kernel/workqueue.c:2269
worker_thread+0x98/0xe40 kernel/workqueue.c:2415
kthread+0x361/0x430 kernel/kthread.c:255
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Reported-by: syzbot+d44e1b26ce5c3e77458d@syzkaller.appspotmail.com
Fixes: ed76e329d74a ("blk-mq: abstract out queue map") # v5.0
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
block/blk-mq.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 7a16d61e3cd3..f174b6ad94cb 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3027,6 +3027,14 @@ static int blk_mq_alloc_rq_maps(struct blk_mq_tag_set *set)
static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
{
+ /*
+ * blk_mq_map_queues() and multiple .map_queues() implementations
+ * expect that set->map[HCTX_TYPE_DEFAULT].nr_queues is set to the
+ * number of hardware queues.
+ */
+ if (set->nr_maps == 1)
+ set->map[HCTX_TYPE_DEFAULT].nr_queues = set->nr_hw_queues;
+
if (set->ops->map_queues && !is_kdump_kernel()) {
int i;
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 3/8] blk-mq: Fix a recently introduced regression in blk_mq_realloc_hw_ctxs()
2020-03-10 4:26 [PATCH v4 0/8] Improve changing the number of hardware queues Bart Van Assche
2020-03-10 4:26 ` [PATCH v4 1/8] blk-mq: Fix a comment in include/linux/blk-mq.h Bart Van Assche
2020-03-10 4:26 ` [PATCH v4 2/8] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync Bart Van Assche
@ 2020-03-10 4:26 ` Bart Van Assche
2020-03-10 4:26 ` [PATCH v4 4/8] null_blk: Suppress an UBSAN complaint triggered when setting 'memory_backed' Bart Van Assche
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2020-03-10 4:26 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche, Ming Lei,
Christoph Hellwig, Hannes Reinecke, Johannes Thumshirn,
Keith Busch
q->nr_hw_queues must only be updated once it is known that
blk_mq_realloc_hw_ctxs() has succeeded. Otherwise it can happen that
reallocation fails and that q->nr_hw_queues is larger than the number of
allocated hardware queues. This patch fixes the following crash if
increasing the number of hardware queues fails:
BUG: KASAN: null-ptr-deref in blk_mq_map_swqueue+0x775/0x810
Write of size 8 at addr 0000000000000118 by task check/977
CPU: 3 PID: 977 Comm: check Not tainted 5.6.0-rc1-dbg+ #8
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
Call Trace:
dump_stack+0xa5/0xe6
__kasan_report.cold+0x65/0x99
kasan_report+0x16/0x20
check_memory_region+0x140/0x1b0
memset+0x28/0x40
blk_mq_map_swqueue+0x775/0x810
blk_mq_update_nr_hw_queues+0x468/0x710
nullb_device_submit_queues_store+0xf7/0x1a0 [null_blk]
configfs_write_file+0x1c4/0x250 [configfs]
__vfs_write+0x4c/0x90
vfs_write+0x145/0x2c0
ksys_write+0xd7/0x180
__x64_sys_write+0x47/0x50
do_syscall_64+0x6f/0x2f0
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Cc: Keith Busch <kbusch@kernel.org>
Fixes: ac0d6b926e74 ("block: Reduce the amount of memory required per request queue")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
block/blk-mq.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index f174b6ad94cb..41f7264e51c9 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2828,7 +2828,6 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
memcpy(new_hctxs, hctxs, q->nr_hw_queues *
sizeof(*hctxs));
q->queue_hw_ctx = new_hctxs;
- q->nr_hw_queues = set->nr_hw_queues;
kfree(hctxs);
hctxs = new_hctxs;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 4/8] null_blk: Suppress an UBSAN complaint triggered when setting 'memory_backed'
2020-03-10 4:26 [PATCH v4 0/8] Improve changing the number of hardware queues Bart Van Assche
` (2 preceding siblings ...)
2020-03-10 4:26 ` [PATCH v4 3/8] blk-mq: Fix a recently introduced regression in blk_mq_realloc_hw_ctxs() Bart Van Assche
@ 2020-03-10 4:26 ` Bart Van Assche
2020-03-10 4:26 ` [PATCH v4 5/8] null_blk: Fix changing the number of hardware queues Bart Van Assche
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2020-03-10 4:26 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn,
Chaitanya Kulkarni
Although it is not clear to me why UBSAN complains when 'memory_backed'
is set, this patch suppresses the UBSAN complaint that is triggered when
setting that configfs attribute.
UBSAN: Undefined behaviour in drivers/block/null_blk_main.c:327:1
load of value 16 is not a valid value for type '_Bool'
CPU: 2 PID: 8396 Comm: check Not tainted 5.6.0-rc1-dbg+ #14
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
Call Trace:
dump_stack+0xa5/0xe6
ubsan_epilogue+0x9/0x26
__ubsan_handle_load_invalid_value+0x6d/0x76
nullb_device_memory_backed_store.cold+0x2c/0x38 [null_blk]
configfs_write_file+0x1c4/0x250 [configfs]
__vfs_write+0x4c/0x90
vfs_write+0x145/0x2c0
ksys_write+0xd7/0x180
__x64_sys_write+0x47/0x50
do_syscall_64+0x6f/0x2f0
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/null_blk_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 133060431dbd..b8a08b82b882 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -276,7 +276,7 @@ nullb_device_##NAME##_store(struct config_item *item, const char *page, \
{ \
int (*apply_fn)(struct nullb_device *dev, TYPE new_value) = APPLY;\
struct nullb_device *dev = to_nullb_device(item); \
- TYPE uninitialized_var(new_value); \
+ TYPE new_value = 0; \
int ret; \
\
ret = nullb_device_##TYPE##_attr_store(&new_value, page, count);\
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 5/8] null_blk: Fix changing the number of hardware queues
2020-03-10 4:26 [PATCH v4 0/8] Improve changing the number of hardware queues Bart Van Assche
` (3 preceding siblings ...)
2020-03-10 4:26 ` [PATCH v4 4/8] null_blk: Suppress an UBSAN complaint triggered when setting 'memory_backed' Bart Van Assche
@ 2020-03-10 4:26 ` Bart Van Assche
2020-03-10 4:26 ` [PATCH v4 6/8] null_blk: Fix the null_add_dev() error path Bart Van Assche
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2020-03-10 4:26 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn
Instead of initializing null_blk hardware queues explicitly after the
request queue has been created, provide .init_hctx() and .exit_hctx()
callback functions. The latter functions are not only called during
request queue allocation but also when the number of hardware queues
changes. Allocate nr_cpu_ids queues during initialization to support
increasing the number of hardware queues above the initial hardware
queue count.
This change fixes increasing the number of hardware queues above the
initial number of hardware queues and also keeps nullb->nr_queues in
sync with the number of hardware queues.
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/null_blk_main.c | 103 ++++++++++++++++++++++------------
1 file changed, 66 insertions(+), 37 deletions(-)
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index b8a08b82b882..83e31b81db3c 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -302,6 +302,12 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
if (!nullb)
return 0;
+ /*
+ * Make sure that null_init_hctx() does not access nullb->queues[] past
+ * the end of that array.
+ */
+ if (submit_queues > nr_cpu_ids)
+ return -EINVAL;
set = nullb->tag_set;
blk_mq_update_nr_hw_queues(set, submit_queues);
return set->nr_hw_queues == submit_queues ? 0 : -ENOMEM;
@@ -1408,12 +1414,6 @@ static blk_status_t null_queue_rq(struct blk_mq_hw_ctx *hctx,
return null_handle_cmd(cmd, sector, nr_sectors, req_op(bd->rq));
}
-static const struct blk_mq_ops null_mq_ops = {
- .queue_rq = null_queue_rq,
- .complete = null_complete_rq,
- .timeout = null_timeout_rq,
-};
-
static void cleanup_queue(struct nullb_queue *nq)
{
kfree(nq->tag_map);
@@ -1430,6 +1430,43 @@ static void cleanup_queues(struct nullb *nullb)
kfree(nullb->queues);
}
+static void null_exit_hctx(struct blk_mq_hw_ctx *hctx, unsigned int hctx_idx)
+{
+ struct nullb_queue *nq = hctx->driver_data;
+ struct nullb *nullb = nq->dev->nullb;
+
+ nullb->nr_queues--;
+}
+
+static void null_init_queue(struct nullb *nullb, struct nullb_queue *nq)
+{
+ init_waitqueue_head(&nq->wait);
+ nq->queue_depth = nullb->queue_depth;
+ nq->dev = nullb->dev;
+}
+
+static int null_init_hctx(struct blk_mq_hw_ctx *hctx, void *driver_data,
+ unsigned int hctx_idx)
+{
+ struct nullb *nullb = hctx->queue->queuedata;
+ struct nullb_queue *nq;
+
+ nq = &nullb->queues[hctx_idx];
+ hctx->driver_data = nq;
+ null_init_queue(nullb, nq);
+ nullb->nr_queues++;
+
+ return 0;
+}
+
+static const struct blk_mq_ops null_mq_ops = {
+ .queue_rq = null_queue_rq,
+ .complete = null_complete_rq,
+ .timeout = null_timeout_rq,
+ .init_hctx = null_init_hctx,
+ .exit_hctx = null_exit_hctx,
+};
+
static void null_del_dev(struct nullb *nullb)
{
struct nullb_device *dev = nullb->dev;
@@ -1473,33 +1510,6 @@ static const struct block_device_operations null_ops = {
.report_zones = null_report_zones,
};
-static void null_init_queue(struct nullb *nullb, struct nullb_queue *nq)
-{
- BUG_ON(!nullb);
- BUG_ON(!nq);
-
- init_waitqueue_head(&nq->wait);
- nq->queue_depth = nullb->queue_depth;
- nq->dev = nullb->dev;
-}
-
-static void null_init_queues(struct nullb *nullb)
-{
- struct request_queue *q = nullb->q;
- struct blk_mq_hw_ctx *hctx;
- struct nullb_queue *nq;
- int i;
-
- queue_for_each_hw_ctx(q, hctx, i) {
- if (!hctx->nr_ctx || !hctx->tags)
- continue;
- nq = &nullb->queues[i];
- hctx->driver_data = nq;
- null_init_queue(nullb, nq);
- nullb->nr_queues++;
- }
-}
-
static int setup_commands(struct nullb_queue *nq)
{
struct nullb_cmd *cmd;
@@ -1526,8 +1536,7 @@ static int setup_commands(struct nullb_queue *nq)
static int setup_queues(struct nullb *nullb)
{
- nullb->queues = kcalloc(nullb->dev->submit_queues,
- sizeof(struct nullb_queue),
+ nullb->queues = kcalloc(nr_cpu_ids, sizeof(struct nullb_queue),
GFP_KERNEL);
if (!nullb->queues)
return -ENOMEM;
@@ -1673,6 +1682,27 @@ static bool null_setup_fault(void)
return true;
}
+/*
+ * This function is identical to blk_mq_init_queue() except that it sets
+ * queuedata before .init_hctx is called.
+ */
+static struct request_queue *nullb_alloc_queue(struct nullb *nullb)
+{
+ struct request_queue *uninit_q, *q;
+ struct blk_mq_tag_set *set = nullb->tag_set;
+
+ uninit_q = blk_alloc_queue_node(GFP_KERNEL, set->numa_node);
+ if (!uninit_q)
+ return ERR_PTR(-ENOMEM);
+
+ uninit_q->queuedata = nullb;
+ q = blk_mq_init_allocated_queue(set, uninit_q, false);
+ if (IS_ERR(q))
+ blk_cleanup_queue(uninit_q);
+
+ return q;
+}
+
static int null_add_dev(struct nullb_device *dev)
{
struct nullb *nullb;
@@ -1712,12 +1742,11 @@ static int null_add_dev(struct nullb_device *dev)
goto out_cleanup_queues;
nullb->tag_set->timeout = 5 * HZ;
- nullb->q = blk_mq_init_queue(nullb->tag_set);
+ nullb->q = nullb_alloc_queue(nullb);
if (IS_ERR(nullb->q)) {
rv = -ENOMEM;
goto out_cleanup_tags;
}
- null_init_queues(nullb);
} else if (dev->queue_mode == NULL_Q_BIO) {
nullb->q = blk_alloc_queue_node(GFP_KERNEL, dev->home_node);
if (!nullb->q) {
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 6/8] null_blk: Fix the null_add_dev() error path
2020-03-10 4:26 [PATCH v4 0/8] Improve changing the number of hardware queues Bart Van Assche
` (4 preceding siblings ...)
2020-03-10 4:26 ` [PATCH v4 5/8] null_blk: Fix changing the number of hardware queues Bart Van Assche
@ 2020-03-10 4:26 ` Bart Van Assche
2020-03-10 4:26 ` [PATCH v4 7/8] null_blk: Handle null_add_dev() failures properly Bart Van Assche
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2020-03-10 4:26 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn,
Chaitanya Kulkarni
If null_add_dev() fails, clear dev->nullb.
This patch fixes the following KASAN complaint:
BUG: KASAN: use-after-free in nullb_device_submit_queues_store+0xcf/0x160 [null_blk]
Read of size 8 at addr ffff88803280fc30 by task check/8409
Call Trace:
dump_stack+0xa5/0xe6
print_address_description.constprop.0+0x26/0x260
__kasan_report.cold+0x7b/0x99
kasan_report+0x16/0x20
__asan_load8+0x58/0x90
nullb_device_submit_queues_store+0xcf/0x160 [null_blk]
configfs_write_file+0x1c4/0x250 [configfs]
__vfs_write+0x4c/0x90
vfs_write+0x145/0x2c0
ksys_write+0xd7/0x180
__x64_sys_write+0x47/0x50
do_syscall_64+0x6f/0x2f0
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7ff370926317
Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
RSP: 002b:00007fff2dd2da48 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007ff370926317
RDX: 0000000000000002 RSI: 0000559437ef23f0 RDI: 0000000000000001
RBP: 0000559437ef23f0 R08: 000000000000000a R09: 0000000000000001
R10: 0000559436703471 R11: 0000000000000246 R12: 0000000000000002
R13: 00007ff370a006a0 R14: 00007ff370a014a0 R15: 00007ff370a008a0
Allocated by task 8409:
save_stack+0x23/0x90
__kasan_kmalloc.constprop.0+0xcf/0xe0
kasan_kmalloc+0xd/0x10
kmem_cache_alloc_node_trace+0x129/0x4c0
null_add_dev+0x24a/0xe90 [null_blk]
nullb_device_power_store+0x1b6/0x270 [null_blk]
configfs_write_file+0x1c4/0x250 [configfs]
__vfs_write+0x4c/0x90
vfs_write+0x145/0x2c0
ksys_write+0xd7/0x180
__x64_sys_write+0x47/0x50
do_syscall_64+0x6f/0x2f0
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 8409:
save_stack+0x23/0x90
__kasan_slab_free+0x112/0x160
kasan_slab_free+0x12/0x20
kfree+0xdf/0x250
null_add_dev+0xaf3/0xe90 [null_blk]
nullb_device_power_store+0x1b6/0x270 [null_blk]
configfs_write_file+0x1c4/0x250 [configfs]
__vfs_write+0x4c/0x90
vfs_write+0x145/0x2c0
ksys_write+0xd7/0x180
__x64_sys_write+0x47/0x50
do_syscall_64+0x6f/0x2f0
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Fixes: 2984c8684f96 ("nullb: factor disk parameters")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/null_blk_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 83e31b81db3c..80d3dcb8722c 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -1817,6 +1817,7 @@ static int null_add_dev(struct nullb_device *dev)
cleanup_queues(nullb);
out_free_nullb:
kfree(nullb);
+ dev->nullb = NULL;
out:
return rv;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 7/8] null_blk: Handle null_add_dev() failures properly
2020-03-10 4:26 [PATCH v4 0/8] Improve changing the number of hardware queues Bart Van Assche
` (5 preceding siblings ...)
2020-03-10 4:26 ` [PATCH v4 6/8] null_blk: Fix the null_add_dev() error path Bart Van Assche
@ 2020-03-10 4:26 ` Bart Van Assche
2020-03-10 4:26 ` [PATCH v4 8/8] null_blk: Add support for init_hctx() fault injection Bart Van Assche
2020-03-10 13:10 ` [PATCH v4 0/8] Improve changing the number of hardware queues Jens Axboe
8 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2020-03-10 4:26 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn,
Chaitanya Kulkarni
If null_add_dev() fails then null_del_dev() is called with a NULL argument.
Make null_del_dev() handle this scenario correctly. This patch fixes the
following KASAN complaint:
null-ptr-deref in null_del_dev+0x28/0x280 [null_blk]
Read of size 8 at addr 0000000000000000 by task find/1062
Call Trace:
dump_stack+0xa5/0xe6
__kasan_report.cold+0x65/0x99
kasan_report+0x16/0x20
__asan_load8+0x58/0x90
null_del_dev+0x28/0x280 [null_blk]
nullb_group_drop_item+0x7e/0xa0 [null_blk]
client_drop_item+0x53/0x80 [configfs]
configfs_rmdir+0x395/0x4e0 [configfs]
vfs_rmdir+0xb6/0x220
do_rmdir+0x238/0x2c0
__x64_sys_unlinkat+0x75/0x90
do_syscall_64+0x6f/0x2f0
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/null_blk_main.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 80d3dcb8722c..d21c82c8863f 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -1469,7 +1469,12 @@ static const struct blk_mq_ops null_mq_ops = {
static void null_del_dev(struct nullb *nullb)
{
- struct nullb_device *dev = nullb->dev;
+ struct nullb_device *dev;
+
+ if (!nullb)
+ return;
+
+ dev = nullb->dev;
ida_simple_remove(&nullb_indexes, nullb->index);
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v4 8/8] null_blk: Add support for init_hctx() fault injection
2020-03-10 4:26 [PATCH v4 0/8] Improve changing the number of hardware queues Bart Van Assche
` (6 preceding siblings ...)
2020-03-10 4:26 ` [PATCH v4 7/8] null_blk: Handle null_add_dev() failures properly Bart Van Assche
@ 2020-03-10 4:26 ` Bart Van Assche
2020-03-10 13:10 ` [PATCH v4 0/8] Improve changing the number of hardware queues Jens Axboe
8 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2020-03-10 4:26 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn
This makes it possible to test the error path in blk_mq_realloc_hw_ctxs()
and also several error paths in null_blk.
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/null_blk_main.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index d21c82c8863f..89bb16a99007 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -23,6 +23,7 @@
#ifdef CONFIG_BLK_DEV_NULL_BLK_FAULT_INJECTION
static DECLARE_FAULT_ATTR(null_timeout_attr);
static DECLARE_FAULT_ATTR(null_requeue_attr);
+static DECLARE_FAULT_ATTR(null_init_hctx_attr);
#endif
static inline u64 mb_per_tick(int mbps)
@@ -101,6 +102,9 @@ module_param_string(timeout, g_timeout_str, sizeof(g_timeout_str), 0444);
static char g_requeue_str[80];
module_param_string(requeue, g_requeue_str, sizeof(g_requeue_str), 0444);
+
+static char g_init_hctx_str[80];
+module_param_string(init_hctx, g_init_hctx_str, sizeof(g_init_hctx_str), 0444);
#endif
static int g_queue_mode = NULL_Q_MQ;
@@ -1451,6 +1455,11 @@ static int null_init_hctx(struct blk_mq_hw_ctx *hctx, void *driver_data,
struct nullb *nullb = hctx->queue->queuedata;
struct nullb_queue *nq;
+#ifdef CONFIG_BLK_DEV_NULL_BLK_FAULT_INJECTION
+ if (g_init_hctx_str[0] && should_fail(&null_init_hctx_attr, 1))
+ return -EFAULT;
+#endif
+
nq = &nullb->queues[hctx_idx];
hctx->driver_data = nq;
null_init_queue(nullb, nq);
@@ -1683,6 +1692,8 @@ static bool null_setup_fault(void)
return false;
if (!__null_setup_fault(&null_requeue_attr, g_requeue_str))
return false;
+ if (!__null_setup_fault(&null_init_hctx_attr, g_init_hctx_str))
+ return false;
#endif
return true;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v4 0/8] Improve changing the number of hardware queues
2020-03-10 4:26 [PATCH v4 0/8] Improve changing the number of hardware queues Bart Van Assche
` (7 preceding siblings ...)
2020-03-10 4:26 ` [PATCH v4 8/8] null_blk: Add support for init_hctx() fault injection Bart Van Assche
@ 2020-03-10 13:10 ` Jens Axboe
8 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2020-03-10 13:10 UTC (permalink / raw)
To: Bart Van Assche; +Cc: linux-block, Christoph Hellwig
On 3/9/20 10:26 PM, Bart Van Assche wrote:
> Hi Jens,
>
> These patches are what I came up with while analyzing syzbot and blktests
> complaints related to dynamically changing the number of hardware queues.
> Please consider these patches for the upstream kernel.
Applied for 5.7, thanks Bart.
--
Jens Axboe
^ permalink raw reply [flat|nested] 10+ messages in thread