From fc750ea71a597c6426ff563a9caf47db9cf090a4 Mon Sep 17 00:00:00 2001
From: y00230200 <yanghongliang.yang@huawei.com>
Date: Thu, 18 Jan 2018 16:05:18 +0800
Subject: [PATCH] blk-mq: fix race between timeout and freeing request
CVE-2015-9016
Inside timeout handler, blk_mq_tag_to_rq() is called
to retrieve the request from one tag. This way is obviously
wrong because the request can be freed any time and some
fiedds of the request can't be trusted, then kernel oops
might be triggered[1].
---
block/blk-flush.c | 15 ++++++++++++++-
block/blk-mq-tag.c | 2 +-
block/blk-mq-tag.h | 12 ++++++++++++
block/blk-mq.c | 16 +---------------
block/blk.h | 6 ++++++
5 files changed, 34 insertions(+), 17 deletions(-)
diff --git a/block/blk-flush.c b/block/blk-flush.c
index 10679fe62c8c..d9a005037179 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -112,6 +112,7 @@
#include "blk.h"
#include "blk-mq.h"
+#include "blk-mq-tag.h"
/* FLUSH/FUA sequences */
enum {
@@ -283,7 +284,12 @@ static void flush_end_io(struct request *flush_rq, int error)
struct blk_flush_queue *fq = blk_get_flush_queue(q, flush_rq->mq_ctx);
if (q->mq_ops) {
+ struct blk_mq_hw_ctx *hctx;
+
+ /* release the tag's ownership to the req cloned from */
spin_lock_irqsave(&fq->mq_flush_lock, flags);
+ hctx = q->mq_ops->map_queue(q, flush_rq->mq_ctx->cpu);
+ blk_mq_tag_set_rq(hctx, flush_rq->tag, fq->orig_rq);
flush_rq->tag = -1;
}
@@ -366,11 +372,18 @@ static bool blk_kick_flush(struct request_queue *q, struct blk_flush_queue *fq)
/*
* Borrow tag from the first request since they can't
- * be in flight at the same time.
+ * be in flight at the same time. And acquire the tag's
+ * ownership for flush req.
*/
if (q->mq_ops) {
+ struct blk_mq_hw_ctx *hctx;
+
flush_rq->mq_ctx = first_rq->mq_ctx;
flush_rq->tag = first_rq->tag;
+ fq->orig_rq = first_rq;
+
+ hctx = q->mq_ops->map_queue(q, first_rq->mq_ctx->cpu);
+ blk_mq_tag_set_rq(hctx, first_rq->tag, flush_rq);
}
flush_rq->cmd_type = REQ_TYPE_FS;
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 702ae29b8d90..5f685cd5c6cd 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -403,7 +403,7 @@ static void bt_for_each(struct blk_mq_hw_ctx *hctx,
for (bit = find_first_bit(&bm->word, bm->depth);
bit < bm->depth;
bit = find_next_bit(&bm->word, bm->depth, bit + 1)) {
- rq = blk_mq_tag_to_rq(hctx->tags, off + bit);
+ rq = hctx->tags->rqs[off + bit];
if (rq->q == hctx->queue)
fn(hctx, rq, data, reserved);
}
diff --git a/block/blk-mq-tag.h b/block/blk-mq-tag.h
index 6206ed17ef76..14c6e4c92556 100644
--- a/block/blk-mq-tag.h
+++ b/block/blk-mq-tag.h
@@ -85,4 +85,16 @@ static inline void blk_mq_tag_idle(struct blk_mq_hw_ctx *hctx)
__blk_mq_tag_idle(hctx);
}
+/*
+ * This helper should only be used for flush request to share tag
+ * with the request cloned from, and both the two requests can't be
+ * in flight at the same time. The caller has to make sure the tag
+ * can't be freed.
+ */
+static inline void blk_mq_tag_set_rq(struct blk_mq_hw_ctx *hctx,
+ unsigned int tag, struct request *rq)
+{
+ hctx->tags->rqs[tag] = rq;
+}
+
#endif
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 691959ecb80f..4b839c117c56 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -498,23 +498,9 @@ void blk_mq_kick_requeue_list(struct request_queue *q)
}
EXPORT_SYMBOL(blk_mq_kick_requeue_list);
-static inline bool is_flush_request(struct request *rq,
- struct blk_flush_queue *fq, unsigned int tag)
-{
- return ((rq->cmd_flags & REQ_FLUSH_SEQ) &&
- fq->flush_rq->tag == tag);
-}
-
struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)
{
- struct request *rq = tags->rqs[tag];
- /* mq_ctx of flush rq is always cloned from the corresponding req */
- struct blk_flush_queue *fq = blk_get_flush_queue(rq->q, rq->mq_ctx);
-
- if (!is_flush_request(rq, fq, tag))
- return rq;
-
- return fq->flush_rq;
+ return tags->rqs[tag];
}
EXPORT_SYMBOL(blk_mq_tag_to_rq);
diff --git a/block/blk.h b/block/blk.h
index 39471974401c..a9cd7d987d54 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -22,6 +22,12 @@ struct blk_flush_queue {
struct list_head flush_queue[2];
struct list_head flush_data_in_flight;
struct request *flush_rq;
+
+ /*
+ * flush_rq shares tag with this rq, both can't be active
+ * at the same time
+ */
+ struct request *orig_rq;
spinlock_t mq_flush_lock;
};
--
GitLab