* [PATCH] blk-mq: Fix a recently introduced scsi-mq regression
@ 2015-01-28 18:28 Bart Van Assche
2015-01-28 23:52 ` Ming Lei
0 siblings, 1 reply; 4+ messages in thread
From: Bart Van Assche @ 2015-01-28 18:28 UTC (permalink / raw)
To: Jens Axboe; +Cc: Ming Lei, Sasha Levin, Christoph Hellwig, linux-kernel
This patch fixes a use-after-free that was introduced via patch
"blk-mq: fix hctx/ctx kobject use-after-free" (commit
76d697d10769; kernel v3.19-rc4) and fixes the following crash:
general protection fault: 0000 [#1] SMP
Workqueue: srp_remove srp_remove_work [ib_srp]
task: ffff88083530c880 ti: ffff880835774000 task.ti: ffff880835774000
[<ffffffff8125a43c>] blk_mq_tag_wakeup_all+0x1c/0x90
RDI: 6b6b6b6b6b6b6b6b
Call Trace:
[<ffffffff8125792e>] blk_mq_wake_waiters+0x4e/0x80
[<ffffffff81247e86>] blk_set_queue_dying+0x26/0x90
[<ffffffff8124abe5>] blk_cleanup_queue+0x35/0x250
[<ffffffffa001ce4a>] __scsi_remove_device+0x5a/0xe0 [scsi_mod]
[<ffffffffa001b48f>] scsi_forget_host+0x6f/0x80 [scsi_mod]
[<ffffffffa000d646>] scsi_remove_host+0x86/0x140 [scsi_mod]
[<ffffffffa0884c0b>] srp_remove_work+0x9b/0x210 [ib_srp]
[<ffffffff8106ff48>] process_one_work+0x1d8/0x780
[<ffffffff8107060b>] worker_thread+0x11b/0x460
[<ffffffff81075c8f>] kthread+0xef/0x110
[<ffffffff814dbdac>] ret_from_fork+0x7c/0xb0
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
---
block/blk-mq-sysfs.c | 15 +++++----------
block/blk-mq.c | 12 +++++++-----
2 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 6774a0e..921f7cf 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -19,6 +19,8 @@ static void blk_mq_sysfs_release(struct kobject *kobj)
q = container_of(kobj, struct request_queue, mq_kobj);
free_percpu(q->queue_ctx);
+ kfree(q->queue_hw_ctx);
+ q->queue_hw_ctx = NULL;
}
static void blk_mq_ctx_release(struct kobject *kobj)
@@ -34,6 +36,7 @@ static void blk_mq_hctx_release(struct kobject *kobj)
struct blk_mq_hw_ctx *hctx;
hctx = container_of(kobj, struct blk_mq_hw_ctx, kobj);
+ kfree(hctx->ctxs);
kfree(hctx);
}
@@ -388,21 +391,13 @@ void blk_mq_unregister_disk(struct gendisk *disk)
{
struct request_queue *q = disk->queue;
struct blk_mq_hw_ctx *hctx;
- struct blk_mq_ctx *ctx;
- int i, j;
+ int i;
- queue_for_each_hw_ctx(q, hctx, i) {
+ queue_for_each_hw_ctx(q, hctx, i)
blk_mq_unregister_hctx(hctx);
- hctx_for_each_ctx(hctx, ctx, j)
- kobject_put(&ctx->kobj);
-
- kobject_put(&hctx->kobj);
- }
-
kobject_uevent(&q->mq_kobj, KOBJ_REMOVE);
kobject_del(&q->mq_kobj);
- kobject_put(&q->mq_kobj);
kobject_put(&disk_to_dev(disk)->kobj);
}
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 9ee3b87..6d007a4 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1604,7 +1604,8 @@ static void blk_mq_exit_hctx(struct request_queue *q,
struct blk_mq_tag_set *set,
struct blk_mq_hw_ctx *hctx, unsigned int hctx_idx)
{
- unsigned flush_start_tag = set->queue_depth;
+ struct blk_mq_ctx *ctx;
+ unsigned j, flush_start_tag = set->queue_depth;
blk_mq_tag_idle(hctx);
@@ -1618,8 +1619,10 @@ static void blk_mq_exit_hctx(struct request_queue *q,
blk_mq_unregister_cpu_notifier(&hctx->cpu_notifier);
blk_free_flush_queue(hctx->fq);
- kfree(hctx->ctxs);
blk_mq_free_bitmap(&hctx->ctx_map);
+ hctx_for_each_ctx(hctx, ctx, j)
+ kobject_put(&ctx->kobj);
+ kobject_put(&hctx->kobj);
}
static void blk_mq_exit_hw_queues(struct request_queue *q,
@@ -2000,10 +2003,9 @@ void blk_mq_free_queue(struct request_queue *q)
percpu_ref_exit(&q->mq_usage_counter);
- kfree(q->queue_hw_ctx);
- kfree(q->mq_map);
+ kobject_put(&q->mq_kobj);
- q->queue_hw_ctx = NULL;
+ kfree(q->mq_map);
q->mq_map = NULL;
mutex_lock(&all_q_mutex);
--
2.1.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] blk-mq: Fix a recently introduced scsi-mq regression
2015-01-28 18:28 [PATCH] blk-mq: Fix a recently introduced scsi-mq regression Bart Van Assche
@ 2015-01-28 23:52 ` Ming Lei
2015-01-29 11:27 ` Bart Van Assche
0 siblings, 1 reply; 4+ messages in thread
From: Ming Lei @ 2015-01-28 23:52 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, Sasha Levin, Christoph Hellwig, linux-kernel
On Thu, Jan 29, 2015 at 2:28 AM, Bart Van Assche
<bart.vanassche@sandisk.com> wrote:
> This patch fixes a use-after-free that was introduced via patch
> "blk-mq: fix hctx/ctx kobject use-after-free" (commit
> 76d697d10769; kernel v3.19-rc4) and fixes the following crash:
Sorry, my fault, I shouldn't have linked mq's release with mq_kobj,
which may be released before running blk_cleanup_queue().
Another approach may be to revert the faulty commit and release all
mq stuff in request queue's release handler(blk_release_queue)
which should be more safe and simple.
>
> general protection fault: 0000 [#1] SMP
> Workqueue: srp_remove srp_remove_work [ib_srp]
> task: ffff88083530c880 ti: ffff880835774000 task.ti: ffff880835774000
> [<ffffffff8125a43c>] blk_mq_tag_wakeup_all+0x1c/0x90
> RDI: 6b6b6b6b6b6b6b6b
> Call Trace:
> [<ffffffff8125792e>] blk_mq_wake_waiters+0x4e/0x80
> [<ffffffff81247e86>] blk_set_queue_dying+0x26/0x90
> [<ffffffff8124abe5>] blk_cleanup_queue+0x35/0x250
> [<ffffffffa001ce4a>] __scsi_remove_device+0x5a/0xe0 [scsi_mod]
> [<ffffffffa001b48f>] scsi_forget_host+0x6f/0x80 [scsi_mod]
> [<ffffffffa000d646>] scsi_remove_host+0x86/0x140 [scsi_mod]
> [<ffffffffa0884c0b>] srp_remove_work+0x9b/0x210 [ib_srp]
> [<ffffffff8106ff48>] process_one_work+0x1d8/0x780
> [<ffffffff8107060b>] worker_thread+0x11b/0x460
> [<ffffffff81075c8f>] kthread+0xef/0x110
> [<ffffffff814dbdac>] ret_from_fork+0x7c/0xb0
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Ming Lei <ming.lei@canonical.com>
> Cc: Sasha Levin <sasha.levin@oracle.com>
> Cc: Christoph Hellwig <hch@lst.de>
> ---
> block/blk-mq-sysfs.c | 15 +++++----------
> block/blk-mq.c | 12 +++++++-----
> 2 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
> index 6774a0e..921f7cf 100644
> --- a/block/blk-mq-sysfs.c
> +++ b/block/blk-mq-sysfs.c
> @@ -19,6 +19,8 @@ static void blk_mq_sysfs_release(struct kobject *kobj)
>
> q = container_of(kobj, struct request_queue, mq_kobj);
> free_percpu(q->queue_ctx);
> + kfree(q->queue_hw_ctx);
> + q->queue_hw_ctx = NULL;
> }
>
> static void blk_mq_ctx_release(struct kobject *kobj)
> @@ -34,6 +36,7 @@ static void blk_mq_hctx_release(struct kobject *kobj)
> struct blk_mq_hw_ctx *hctx;
>
> hctx = container_of(kobj, struct blk_mq_hw_ctx, kobj);
> + kfree(hctx->ctxs);
> kfree(hctx);
> }
>
> @@ -388,21 +391,13 @@ void blk_mq_unregister_disk(struct gendisk *disk)
> {
> struct request_queue *q = disk->queue;
> struct blk_mq_hw_ctx *hctx;
> - struct blk_mq_ctx *ctx;
> - int i, j;
> + int i;
>
> - queue_for_each_hw_ctx(q, hctx, i) {
> + queue_for_each_hw_ctx(q, hctx, i)
> blk_mq_unregister_hctx(hctx);
>
> - hctx_for_each_ctx(hctx, ctx, j)
> - kobject_put(&ctx->kobj);
> -
> - kobject_put(&hctx->kobj);
> - }
> -
> kobject_uevent(&q->mq_kobj, KOBJ_REMOVE);
> kobject_del(&q->mq_kobj);
> - kobject_put(&q->mq_kobj);
>
> kobject_put(&disk_to_dev(disk)->kobj);
> }
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 9ee3b87..6d007a4 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1604,7 +1604,8 @@ static void blk_mq_exit_hctx(struct request_queue *q,
> struct blk_mq_tag_set *set,
> struct blk_mq_hw_ctx *hctx, unsigned int hctx_idx)
> {
> - unsigned flush_start_tag = set->queue_depth;
> + struct blk_mq_ctx *ctx;
> + unsigned j, flush_start_tag = set->queue_depth;
>
> blk_mq_tag_idle(hctx);
>
> @@ -1618,8 +1619,10 @@ static void blk_mq_exit_hctx(struct request_queue *q,
>
> blk_mq_unregister_cpu_notifier(&hctx->cpu_notifier);
> blk_free_flush_queue(hctx->fq);
> - kfree(hctx->ctxs);
> blk_mq_free_bitmap(&hctx->ctx_map);
> + hctx_for_each_ctx(hctx, ctx, j)
> + kobject_put(&ctx->kobj);
> + kobject_put(&hctx->kobj);
> }
>
> static void blk_mq_exit_hw_queues(struct request_queue *q,
> @@ -2000,10 +2003,9 @@ void blk_mq_free_queue(struct request_queue *q)
>
> percpu_ref_exit(&q->mq_usage_counter);
>
> - kfree(q->queue_hw_ctx);
> - kfree(q->mq_map);
> + kobject_put(&q->mq_kobj);
>
> - q->queue_hw_ctx = NULL;
> + kfree(q->mq_map);
> q->mq_map = NULL;
>
> mutex_lock(&all_q_mutex);
> --
> 2.1.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] blk-mq: Fix a recently introduced scsi-mq regression
2015-01-28 23:52 ` Ming Lei
@ 2015-01-29 11:27 ` Bart Van Assche
2015-01-29 11:39 ` Ming Lei
0 siblings, 1 reply; 4+ messages in thread
From: Bart Van Assche @ 2015-01-29 11:27 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, Sasha Levin, Christoph Hellwig, linux-kernel
On 01/29/15 00:52, Ming Lei wrote:
> On Thu, Jan 29, 2015 at 2:28 AM, Bart Van Assche
> <bart.vanassche@sandisk.com> wrote:
>> This patch fixes a use-after-free that was introduced via patch
>> "blk-mq: fix hctx/ctx kobject use-after-free" (commit
>> 76d697d10769; kernel v3.19-rc4) and fixes the following crash:
>
> Sorry, my fault, I shouldn't have linked mq's release with mq_kobj,
> which may be released before running blk_cleanup_queue().
>
> Another approach may be to revert the faulty commit and release all
> mq stuff in request queue's release handler(blk_release_queue)
> which should be more safe and simple.
Hello Ming,
How important is the patch with commit ID 76d697d10769 ? Does it have to
be included in kernel v3.19 or can it be postponed until kernel v3.19.1
or kernel v3.19.2 ? If so, how about reverting that patch for kernel
v3.19 and sending a reworked patch to Jens with a "Cc: stable" tag after
the release of kernel v3.19 ? That last patch will then get included in
a stable version of kernel v3.19.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] blk-mq: Fix a recently introduced scsi-mq regression
2015-01-29 11:27 ` Bart Van Assche
@ 2015-01-29 11:39 ` Ming Lei
0 siblings, 0 replies; 4+ messages in thread
From: Ming Lei @ 2015-01-29 11:39 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, Sasha Levin, Christoph Hellwig, linux-kernel
Hi Bart,
On Thu, Jan 29, 2015 at 7:27 PM, Bart Van Assche
<bart.vanassche@sandisk.com> wrote:
> On 01/29/15 00:52, Ming Lei wrote:
>> On Thu, Jan 29, 2015 at 2:28 AM, Bart Van Assche
>> <bart.vanassche@sandisk.com> wrote:
>>> This patch fixes a use-after-free that was introduced via patch
>>> "blk-mq: fix hctx/ctx kobject use-after-free" (commit
>>> 76d697d10769; kernel v3.19-rc4) and fixes the following crash:
>>
>> Sorry, my fault, I shouldn't have linked mq's release with mq_kobj,
>> which may be released before running blk_cleanup_queue().
>>
>> Another approach may be to revert the faulty commit and release all
>> mq stuff in request queue's release handler(blk_release_queue)
>> which should be more safe and simple.
>
> Hello Ming,
>
> How important is the patch with commit ID 76d697d10769 ? Does it have to
> be included in kernel v3.19 or can it be postponed until kernel v3.19.1
> or kernel v3.19.2 ? If so, how about reverting that patch for kernel
The patch has been in linus tree, but it isn't marked as -stable.
> v3.19 and sending a reworked patch to Jens with a "Cc: stable" tag after
> the release of kernel v3.19 ? That last patch will then get included in
> a stable version of kernel v3.19.
Anyway, it need to be fixed, either reverted or a new patch.
I will post out one revert and another fix candidate, and let's
discuss further.
Thanks,
Ming Lei
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-29 11:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-28 18:28 [PATCH] blk-mq: Fix a recently introduced scsi-mq regression Bart Van Assche
2015-01-28 23:52 ` Ming Lei
2015-01-29 11:27 ` Bart Van Assche
2015-01-29 11:39 ` Ming Lei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).