From: yu kuai <yukuai3@huawei.com>
To: <axboe@kernel.dk>, <ming.lei@redhat.com>,
<chaitanya.kulkarni@wdc.com>, <damien.lemoal@wdc.com>,
<bvanassche@acm.org>, <dhowells@redhat.com>,
<asml.silence@gmail.com>, <ajay.joshi@wdc.com>
Cc: <linux-block@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<yukuai3@huawei.com>, <yi.zhang@huawei.com>,
<zhangxiaoxu5@huawei.com>, <luoshijie1@huawei.com>
Subject: [PATCH] block: revert pushing the final release of request_queue to a workqueue.
Date: Thu, 6 Feb 2020 19:10:52 +0800 [thread overview]
Message-ID: <20200206111052.45356-1-yukuai3@huawei.com> (raw)
syzbot is reporting use after free bug in debugfs_remove[1].
This is because in request_queue, 'q->debugfs_dir' and
'q->blk_trace->dir' could be the same dir. And in __blk_release_queue(),
blk_mq_debugfs_unregister() will remove everything inside the dir.
With futher investigation of the reporduce repro, the problem can be
reporduced by following procedure:
1. LOOP_CTL_ADD, create a request_queue q1, blk_mq_debugfs_register() will
create the dir.
2. LOOP_CTL_REMOVE, blk_release_queue() will add q1 to release queue.
3. LOOP_CTL_ADD, create another request_queue q2,blk_mq_debugfs_register()
will fail because the dir aready exist.
4. BLKTRACESETUP, create two files(msg and dropped) inside the dir.
5. call __blk_release_queue() for q1, debugfs_remove_recursive() will
delete the files created in step 4.
6. LOOP_CTL_REMOVE, blk_release_queue() will add q2 to release queue.
And when __blk_release_queue() is called for q2, blk_trace_shutdown() will
try to release the two files created in step 4, wich are aready released
in step 5.
|thread1 |kworker |thread2 |
| ----------------------- | ------------------------ | -------------------- |
|loop_control_ioctl | | |
| loop_add | | |
| blk_mq_debugfs_register| | |
| debugfs_create_dir | | |
|loop_control_ioctl | | |
| loop_remove | | |
| blk_release_queue | | |
| schedule_work | | |
| | |loop_control_ioctl |
| | | loop_add |
| | | ... |
| | |blk_trace_ioctl |
| | | __blk_trace_setup |
| | | debugfs_create_file|
| |__blk_release_queue | |
| | blk_mq_debugfs_unregister| |
| | debugfs_remove_recursive| |
| | |loop_control_ioctl |
| | | loop_remove |
| | | ... |
| |__blk_release_queue | |
| | blk_trace_shutdown | |
| | debugfs_remove | |
commit dc9edc44de6c ("block: Fix a blk_exit_rl() regression") pushed the
final release of request_queue to a workqueue, witch is not necessary
since commit 1e9364283764 ("blk-sysfs: Rework documention of
__blk_release_queue").
[1] https://syzkaller.appspot.com/bug?extid=903b72a010ad6b7a40f2
References: CVE-2019-19770
Fixes: commit dc9edc44de6c ("block: Fix a blk_exit_rl() regression")
Reported-by: syzbot <syz...@syzkaller.appspotmail.com>
Signed-off-by: yu kuai <yukuai3@huawei.com>
---
block/blk-sysfs.c | 18 +++++-------------
include/linux/blkdev.h | 2 --
2 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index fca9b158f4a0..3f448292099d 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -862,8 +862,8 @@ static void blk_exit_queue(struct request_queue *q)
/**
- * __blk_release_queue - release a request queue
- * @work: pointer to the release_work member of the request queue to be released
+ * blk_release_queue - release a request queue
+ * @@kobj: the kobj belonging to the request queue to be released
*
* Description:
* This function is called when a block device is being unregistered. The
@@ -873,9 +873,10 @@ static void blk_exit_queue(struct request_queue *q)
* of the request queue reaches zero, blk_release_queue is called to release
* all allocated resources of the request queue.
*/
-static void __blk_release_queue(struct work_struct *work)
+static void blk_release_queue(struct kobject *kobj)
{
- struct request_queue *q = container_of(work, typeof(*q), release_work);
+ struct request_queue *q =
+ container_of(kobj, struct request_queue, kobj);
if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags))
blk_stat_remove_callback(q, q->poll_cb);
@@ -904,15 +905,6 @@ static void __blk_release_queue(struct work_struct *work)
call_rcu(&q->rcu_head, blk_free_queue_rcu);
}
-static void blk_release_queue(struct kobject *kobj)
-{
- struct request_queue *q =
- container_of(kobj, struct request_queue, kobj);
-
- INIT_WORK(&q->release_work, __blk_release_queue);
- schedule_work(&q->release_work);
-}
-
static const struct sysfs_ops queue_sysfs_ops = {
.show = queue_attr_show,
.store = queue_attr_store,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 04cfa798a365..dff4d032c78a 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -580,8 +580,6 @@ struct request_queue {
size_t cmd_size;
- struct work_struct release_work;
-
#define BLK_MAX_WRITE_HINTS 5
u64 write_hints[BLK_MAX_WRITE_HINTS];
};
--
2.17.2
next reply other threads:[~2020-02-06 11:12 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-06 11:10 yu kuai [this message]
2020-02-07 4:09 ` [PATCH] block: revert pushing the final release of request_queue to a workqueue Bart Van Assche
2020-02-07 6:02 ` yukuai (C)
2020-02-07 7:10 ` yukuai (C)
2020-02-07 9:30 ` Ming Lei
2020-02-07 10:26 ` yukuai (C)
2020-02-07 12:24 ` yukuai (C)
2020-02-07 13:04 ` Ming Lei
2020-02-08 6:12 ` yukuai (C)
2020-02-10 1:00 ` Bart Van Assche
2020-02-10 2:13 ` yukuai (C)
2020-02-10 2:32 ` Ming Lei
2020-02-10 3:14 ` Bart Van Assche
2020-02-10 8:49 ` yukuai (C)
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=20200206111052.45356-1-yukuai3@huawei.com \
--to=yukuai3@huawei.com \
--cc=ajay.joshi@wdc.com \
--cc=asml.silence@gmail.com \
--cc=axboe@kernel.dk \
--cc=bvanassche@acm.org \
--cc=chaitanya.kulkarni@wdc.com \
--cc=damien.lemoal@wdc.com \
--cc=dhowells@redhat.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luoshijie1@huawei.com \
--cc=ming.lei@redhat.com \
--cc=yi.zhang@huawei.com \
--cc=zhangxiaoxu5@huawei.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox