From: John Garry <john.garry@huawei.com>
To: Ming Lei <ming.lei@redhat.com>, Jens Axboe <axboe@kernel.dk>,
"Christoph Hellwig" <hch@lst.de>
Cc: <linux-block@vger.kernel.org>,
<syzbot+77ba3d171a25c56756ea@syzkaller.appspotmail.com>
Subject: Re: [PATCH] blk-mq: fix use-after-free in blk_mq_exit_sched
Date: Wed, 9 Jun 2021 09:59:43 +0100 [thread overview]
Message-ID: <f5fbc650-5bd3-32ee-1d31-8b1dd1d7fa19@huawei.com> (raw)
In-Reply-To: <20210609063046.122843-1-ming.lei@redhat.com>
On 09/06/2021 07:30, Ming Lei wrote:
Thanks for the fix
> tagset can't be used after blk_cleanup_queue() is returned because
> freeing tagset usually follows blk_clenup_queue(). Commit d97e594c5166
> ("blk-mq: Use request queue-wide tags for tagset-wide sbitmap") adds
> check on q->tag_set->flags in blk_mq_exit_sched(), and causes
> use-after-free.
>
> Fixes it by using hctx->flags.
>
The tagset is a member of the Scsi_Host structure. So it is true that
this memory may be freed before the request_queue is exited?
> Reported-by: syzbot+77ba3d171a25c56756ea@syzkaller.appspotmail.com
> Fixes: d97e594c5166 ("blk-mq: Use request queue-wide tags for tagset-wide sbitmap")
> Cc: John Garry <john.garry@huawei.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> block/blk-mq-sched.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index a9182d2f8ad3..80273245d11a 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -680,6 +680,7 @@ void blk_mq_exit_sched(struct request_queue *q, struct elevator_queue *e)
> {
> struct blk_mq_hw_ctx *hctx;
> unsigned int i;
> + unsigned int flags = 0;
>
> queue_for_each_hw_ctx(q, hctx, i) {
> blk_mq_debugfs_unregister_sched_hctx(hctx);
> @@ -687,12 +688,13 @@ void blk_mq_exit_sched(struct request_queue *q, struct elevator_queue *e)
> e->type->ops.exit_hctx(hctx, i);
> hctx->sched_data = NULL;
> }
> + flags = hctx->flags;
I know the choice is limited, but it is unfortunate that we must set
flags in a loop
> }
> blk_mq_debugfs_unregister_sched(q);
> if (e->type->ops.exit_sched)
> e->type->ops.exit_sched(e);
> blk_mq_sched_tags_teardown(q);
> - if (blk_mq_is_sbitmap_shared(q->tag_set->flags))
> + if (blk_mq_is_sbitmap_shared(flags))
> blk_mq_exit_sched_shared_sbitmap(q);
this is
blk_mq_exit_sched_shared_sbitmap(struct request_queue *queue)
{
sbitmap_queue_free(&queue->sched_bitmap_tags);
..
}
And isn't it safe to call sbitmap_queue_free() when
sbitmap_queue_init_node() has not been called?
I'm just wondering if we can always call
blk_mq_exit_sched_shared_sbitmap()? I know it's not an ideal choice either.
Thanks,
John
> q->elevator = NULL;
> }
>
next prev parent reply other threads:[~2021-06-09 9:05 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-09 6:30 [PATCH] blk-mq: fix use-after-free in blk_mq_exit_sched Ming Lei
2021-06-09 8:59 ` John Garry [this message]
2021-06-09 10:16 ` Ming Lei
2021-06-09 11:42 ` John Garry
2021-06-14 22:07 ` Ming Lei
2021-06-15 10:02 ` John Garry
2021-06-18 6:03 ` Ming Lei
2021-06-18 14:50 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f5fbc650-5bd3-32ee-1d31-8b1dd1d7fa19@huawei.com \
--to=john.garry@huawei.com \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=linux-block@vger.kernel.org \
--cc=ming.lei@redhat.com \
--cc=syzbot+77ba3d171a25c56756ea@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.