Skip to content

Commit 8443826

Browse files
PlaidCatPlaidCat
committed
kyber: fix out of bounds access when preempted
jira LE-3201 cve CVE-2021-46984 Rebuild_History Non-Buildable kernel-rt-4.18.0-553.22.1.rt7.363.el8_10 commit-author Omar Sandoval <osandov@fb.com> commit efed9a3 Empty-Commit: Cherry-Pick Conflicts during history rebuild. Will be included in final tarball splat. Ref for failed cherry-pick at: ciq/ciq_backports/kernel-rt-4.18.0-553.22.1.rt7.363.el8_10/efed9a33.failed __blk_mq_sched_bio_merge() gets the ctx and hctx for the current CPU and passes the hctx to ->bio_merge(). kyber_bio_merge() then gets the ctx for the current CPU again and uses that to get the corresponding Kyber context in the passed hctx. However, the thread may be preempted between the two calls to blk_mq_get_ctx(), and the ctx returned the second time may no longer correspond to the passed hctx. This "works" accidentally most of the time, but it can cause us to read garbage if the second ctx came from an hctx with more ctx's than the first one (i.e., if ctx->index_hw[hctx->type] > hctx->nr_ctx). This manifested as this UBSAN array index out of bounds error reported by Jakub: UBSAN: array-index-out-of-bounds in ../kernel/locking/qspinlock.c:130:9 index 13106 is out of range for type 'long unsigned int [128]' Call Trace: dump_stack+0xa4/0xe5 ubsan_epilogue+0x5/0x40 __ubsan_handle_out_of_bounds.cold.13+0x2a/0x34 queued_spin_lock_slowpath+0x476/0x480 do_raw_spin_lock+0x1c2/0x1d0 kyber_bio_merge+0x112/0x180 blk_mq_submit_bio+0x1f5/0x1100 submit_bio_noacct+0x7b0/0x870 submit_bio+0xc2/0x3a0 btrfs_map_bio+0x4f0/0x9d0 btrfs_submit_data_bio+0x24e/0x310 submit_one_bio+0x7f/0xb0 submit_extent_page+0xc4/0x440 __extent_writepage_io+0x2b8/0x5e0 __extent_writepage+0x28d/0x6e0 extent_write_cache_pages+0x4d7/0x7a0 extent_writepages+0xa2/0x110 do_writepages+0x8f/0x180 __writeback_single_inode+0x99/0x7f0 writeback_sb_inodes+0x34e/0x790 __writeback_inodes_wb+0x9e/0x120 wb_writeback+0x4d2/0x660 wb_workfn+0x64d/0xa10 process_one_work+0x53a/0xa80 worker_thread+0x69/0x5b0 kthread+0x20b/0x240 ret_from_fork+0x1f/0x30 Only Kyber uses the hctx, so fix it by passing the request_queue to ->bio_merge() instead. BFQ and mq-deadline just use that, and Kyber can map the queues itself to avoid the mismatch. Fixes: a608884 ("block: kyber: make kyber more friendly with merging") Reported-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Omar Sandoval <osandov@fb.com> Link: https://lore.kernel.org/r/c7598605401a48d5cfeadebb678abd10af22b83f.1620691329.git.osandov@fb.com Signed-off-by: Jens Axboe <axboe@kernel.dk> (cherry picked from commit efed9a3) Signed-off-by: Jonathan Maple <jmaple@ciq.com> # Conflicts: # block/bfq-iosched.c # block/blk-mq-sched.c # block/kyber-iosched.c # block/mq-deadline.c # include/linux/elevator.h
1 parent 11547a2 commit 8443826

File tree

1 file changed

+271
-0
lines changed

1 file changed

+271
-0
lines changed

‎ciq/ciq_backports/kernel-rt-4.18.0-553.22.1.rt7.363.el8_10/efed9a33.failed‎

Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
kyber: fix out of bounds access when preempted
2+
3+
jira LE-3201
4+
cve CVE-2021-46984
5+
Rebuild_History Non-Buildable kernel-rt-4.18.0-553.22.1.rt7.363.el8_10
6+
commit-author Omar Sandoval <osandov@fb.com>
7+
commit efed9a3337e341bd0989161b97453b52567bc59d
8+
Empty-Commit: Cherry-Pick Conflicts during history rebuild.
9+
Will be included in final tarball splat. Ref for failed cherry-pick at:
10+
ciq/ciq_backports/kernel-rt-4.18.0-553.22.1.rt7.363.el8_10/efed9a33.failed
11+
12+
__blk_mq_sched_bio_merge() gets the ctx and hctx for the current CPU and
13+
passes the hctx to ->bio_merge(). kyber_bio_merge() then gets the ctx
14+
for the current CPU again and uses that to get the corresponding Kyber
15+
context in the passed hctx. However, the thread may be preempted between
16+
the two calls to blk_mq_get_ctx(), and the ctx returned the second time
17+
may no longer correspond to the passed hctx. This "works" accidentally
18+
most of the time, but it can cause us to read garbage if the second ctx
19+
came from an hctx with more ctx's than the first one (i.e., if
20+
ctx->index_hw[hctx->type] > hctx->nr_ctx).
21+
22+
This manifested as this UBSAN array index out of bounds error reported
23+
by Jakub:
24+
25+
UBSAN: array-index-out-of-bounds in ../kernel/locking/qspinlock.c:130:9
26+
index 13106 is out of range for type 'long unsigned int [128]'
27+
Call Trace:
28+
dump_stack+0xa4/0xe5
29+
ubsan_epilogue+0x5/0x40
30+
__ubsan_handle_out_of_bounds.cold.13+0x2a/0x34
31+
queued_spin_lock_slowpath+0x476/0x480
32+
do_raw_spin_lock+0x1c2/0x1d0
33+
kyber_bio_merge+0x112/0x180
34+
blk_mq_submit_bio+0x1f5/0x1100
35+
submit_bio_noacct+0x7b0/0x870
36+
submit_bio+0xc2/0x3a0
37+
btrfs_map_bio+0x4f0/0x9d0
38+
btrfs_submit_data_bio+0x24e/0x310
39+
submit_one_bio+0x7f/0xb0
40+
submit_extent_page+0xc4/0x440
41+
__extent_writepage_io+0x2b8/0x5e0
42+
__extent_writepage+0x28d/0x6e0
43+
extent_write_cache_pages+0x4d7/0x7a0
44+
extent_writepages+0xa2/0x110
45+
do_writepages+0x8f/0x180
46+
__writeback_single_inode+0x99/0x7f0
47+
writeback_sb_inodes+0x34e/0x790
48+
__writeback_inodes_wb+0x9e/0x120
49+
wb_writeback+0x4d2/0x660
50+
wb_workfn+0x64d/0xa10
51+
process_one_work+0x53a/0xa80
52+
worker_thread+0x69/0x5b0
53+
kthread+0x20b/0x240
54+
ret_from_fork+0x1f/0x30
55+
56+
Only Kyber uses the hctx, so fix it by passing the request_queue to
57+
->bio_merge() instead. BFQ and mq-deadline just use that, and Kyber can
58+
map the queues itself to avoid the mismatch.
59+
60+
Fixes: a6088845c2bf ("block: kyber: make kyber more friendly with merging")
61+
Reported-by: Jakub Kicinski <kuba@kernel.org>
62+
Signed-off-by: Omar Sandoval <osandov@fb.com>
63+
Link: https://lore.kernel.org/r/c7598605401a48d5cfeadebb678abd10af22b83f.1620691329.git.osandov@fb.com
64+
Signed-off-by: Jens Axboe <axboe@kernel.dk>
65+
(cherry picked from commit efed9a3337e341bd0989161b97453b52567bc59d)
66+
Signed-off-by: Jonathan Maple <jmaple@ciq.com>
67+
68+
# Conflicts:
69+
# block/bfq-iosched.c
70+
# block/blk-mq-sched.c
71+
# block/kyber-iosched.c
72+
# block/mq-deadline.c
73+
# include/linux/elevator.h
74+
diff --cc block/bfq-iosched.c
75+
index c49d92b5ef8d,59b2499d3f8b..000000000000
76+
--- a/block/bfq-iosched.c
77+
+++ b/block/bfq-iosched.c
78+
@@@ -2329,9 -2263,9 +2329,13 @@@ static void bfq_remove_request(struct r
79+
80+
}
81+
82+
++<<<<<<< HEAD
83+
+static bool bfq_bio_merge(struct blk_mq_hw_ctx *hctx, struct bio *bio)
84+
++=======
85+
+ static bool bfq_bio_merge(struct request_queue *q, struct bio *bio,
86+
+ unsigned int nr_segs)
87+
++>>>>>>> efed9a3337e3 (kyber: fix out of bounds access when preempted)
88+
{
89+
- struct request_queue *q = hctx->queue;
90+
struct bfq_data *bfqd = q->elevator->elevator_data;
91+
struct request *free = NULL;
92+
/*
93+
diff --cc block/blk-mq-sched.c
94+
index 8f52783b0eda,996a4b2f73aa..000000000000
95+
--- a/block/blk-mq-sched.c
96+
+++ b/block/blk-mq-sched.c
97+
@@@ -370,79 -354,39 +370,85 @@@ void blk_mq_sched_dispatch_requests(str
98+
}
99+
}
100+
101+
-bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,
102+
- unsigned int nr_segs)
103+
+bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
104+
+ struct request **merged_request)
105+
+{
106+
+ struct request *rq;
107+
+
108+
+ switch (elv_merge(q, &rq, bio)) {
109+
+ case ELEVATOR_BACK_MERGE:
110+
+ if (!blk_mq_sched_allow_merge(q, rq, bio))
111+
+ return false;
112+
+ if (!bio_attempt_back_merge(q, rq, bio))
113+
+ return false;
114+
+ *merged_request = attempt_back_merge(q, rq);
115+
+ if (!*merged_request)
116+
+ elv_merged_request(q, rq, ELEVATOR_BACK_MERGE);
117+
+ return true;
118+
+ case ELEVATOR_FRONT_MERGE:
119+
+ if (!blk_mq_sched_allow_merge(q, rq, bio))
120+
+ return false;
121+
+ if (!bio_attempt_front_merge(q, rq, bio))
122+
+ return false;
123+
+ *merged_request = attempt_front_merge(q, rq);
124+
+ if (!*merged_request)
125+
+ elv_merged_request(q, rq, ELEVATOR_FRONT_MERGE);
126+
+ return true;
127+
+ case ELEVATOR_DISCARD_MERGE:
128+
+ return bio_attempt_discard_merge(q, rq, bio);
129+
+ default:
130+
+ return false;
131+
+ }
132+
+}
133+
+EXPORT_SYMBOL_GPL(blk_mq_sched_try_merge);
134+
+
135+
+/*
136+
+ * Reverse check our software queue for entries that we could potentially
137+
+ * merge with. Currently includes a hand-wavy stop count of 8, to not spend
138+
+ * too much time checking for merges.
139+
+ */
140+
+static bool blk_mq_attempt_merge(struct request_queue *q,
141+
+ struct blk_mq_hw_ctx *hctx,
142+
+ struct blk_mq_ctx *ctx, struct bio *bio)
143+
+{
144+
+ enum hctx_type type = hctx->type;
145+
+
146+
+ lockdep_assert_held(&ctx->lock);
147+
+
148+
+ if (blk_bio_list_merge(q, &ctx->rq_lists[type], bio)) {
149+
+ ctx->rq_merged++;
150+
+ return true;
151+
+ }
152+
+
153+
+ return false;
154+
+}
155+
+
156+
+bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio)
157+
{
158+
struct elevator_queue *e = q->elevator;
159+
- struct blk_mq_ctx *ctx = blk_mq_get_ctx(q);
160+
- struct blk_mq_hw_ctx *hctx = blk_mq_map_queue(q, bio->bi_opf, ctx);
161+
+ struct blk_mq_ctx *ctx;
162+
+ struct blk_mq_hw_ctx *hctx;
163+
bool ret = false;
164+
enum hctx_type type;
165+
166+
if (e && e->type->ops.bio_merge)
167+
++<<<<<<< HEAD
168+
+ return e->type->ops.bio_merge(hctx, bio);
169+
++=======
170+
+ return e->type->ops.bio_merge(q, bio, nr_segs);
171+
++>>>>>>> efed9a3337e3 (kyber: fix out of bounds access when preempted)
172+
173+
+ ctx = blk_mq_get_ctx(q);
174+
+ hctx = blk_mq_map_queue(q, bio->bi_opf, ctx);
175+
type = hctx->type;
176+
- if (!(hctx->flags & BLK_MQ_F_SHOULD_MERGE) ||
177+
- list_empty_careful(&ctx->rq_lists[type]))
178+
- return false;
179+
-
180+
- /* default per sw-queue merge */
181+
- spin_lock(&ctx->lock);
182+
- /*
183+
- * Reverse check our software queue for entries that we could
184+
- * potentially merge with. Currently includes a hand-wavy stop
185+
- * count of 8, to not spend too much time checking for merges.
186+
- */
187+
- if (blk_bio_list_merge(q, &ctx->rq_lists[type], bio, nr_segs)) {
188+
- ctx->rq_merged++;
189+
- ret = true;
190+
+ if ((hctx->flags & BLK_MQ_F_SHOULD_MERGE) &&
191+
+ !list_empty_careful(&ctx->rq_lists[type])) {
192+
+ /* default per sw-queue merge */
193+
+ spin_lock(&ctx->lock);
194+
+ ret = blk_mq_attempt_merge(q, hctx, ctx, bio);
195+
+ spin_unlock(&ctx->lock);
196+
}
197+
198+
- spin_unlock(&ctx->lock);
199+
-
200+
return ret;
201+
}
202+
203+
diff --cc block/kyber-iosched.c
204+
index 6df74a3a3cd3,81e3279ecd57..000000000000
205+
--- a/block/kyber-iosched.c
206+
+++ b/block/kyber-iosched.c
207+
@@@ -572,10 -561,12 +572,16 @@@ static void kyber_limit_depth(unsigned
208+
}
209+
}
210+
211+
++<<<<<<< HEAD
212+
+static bool kyber_bio_merge(struct blk_mq_hw_ctx *hctx, struct bio *bio)
213+
++=======
214+
+ static bool kyber_bio_merge(struct request_queue *q, struct bio *bio,
215+
+ unsigned int nr_segs)
216+
++>>>>>>> efed9a3337e3 (kyber: fix out of bounds access when preempted)
217+
{
218+
+ struct blk_mq_ctx *ctx = blk_mq_get_ctx(q);
219+
+ struct blk_mq_hw_ctx *hctx = blk_mq_map_queue(q, bio->bi_opf, ctx);
220+
struct kyber_hctx_data *khd = hctx->sched_data;
221+
- struct blk_mq_ctx *ctx = blk_mq_get_ctx(hctx->queue);
222+
struct kyber_ctx_queue *kcq = &khd->kcqs[ctx->index_hw[hctx->type]];
223+
unsigned int sched_domain = kyber_sched_domain(bio->bi_opf);
224+
struct list_head *rq_list = &kcq->rq_list[sched_domain];
225+
diff --cc block/mq-deadline.c
226+
index 43ce0ae88b4a,8eea2cbf2bf4..000000000000
227+
--- a/block/mq-deadline.c
228+
+++ b/block/mq-deadline.c
229+
@@@ -631,13 -461,9 +631,17 @@@ static int dd_request_merge(struct requ
230+
return ELEVATOR_NO_MERGE;
231+
}
232+
233+
++<<<<<<< HEAD
234+
+/*
235+
+ * Attempt to merge a bio into an existing request. This function is called
236+
+ * before @bio is associated with a request.
237+
+ */
238+
+static bool dd_bio_merge(struct blk_mq_hw_ctx *hctx, struct bio *bio)
239+
++=======
240+
+ static bool dd_bio_merge(struct request_queue *q, struct bio *bio,
241+
+ unsigned int nr_segs)
242+
++>>>>>>> efed9a3337e3 (kyber: fix out of bounds access when preempted)
243+
{
244+
- struct request_queue *q = hctx->queue;
245+
struct deadline_data *dd = q->elevator->elevator_data;
246+
struct request *free = NULL;
247+
bool ret;
248+
diff --cc include/linux/elevator.h
249+
index b4f3bc81ddd3,dcb2f9022c1d..000000000000
250+
--- a/include/linux/elevator.h
251+
+++ b/include/linux/elevator.h
252+
@@@ -31,9 -31,10 +31,13 @@@ struct elevator_mq_ops
253+
void (*exit_sched)(struct elevator_queue *);
254+
int (*init_hctx)(struct blk_mq_hw_ctx *, unsigned int);
255+
void (*exit_hctx)(struct blk_mq_hw_ctx *, unsigned int);
256+
- void (*depth_updated)(struct blk_mq_hw_ctx *);
257+
258+
bool (*allow_merge)(struct request_queue *, struct request *, struct bio *);
259+
++<<<<<<< HEAD
260+
+ bool (*bio_merge)(struct blk_mq_hw_ctx *, struct bio *);
261+
++=======
262+
+ bool (*bio_merge)(struct request_queue *, struct bio *, unsigned int);
263+
++>>>>>>> efed9a3337e3 (kyber: fix out of bounds access when preempted)
264+
int (*request_merge)(struct request_queue *q, struct request **, struct bio *);
265+
void (*request_merged)(struct request_queue *, struct request *, enum elv_merge);
266+
void (*requests_merged)(struct request_queue *, struct request *, struct request *);
267+
* Unmerged path block/bfq-iosched.c
268+
* Unmerged path block/blk-mq-sched.c
269+
* Unmerged path block/kyber-iosched.c
270+
* Unmerged path block/mq-deadline.c
271+
* Unmerged path include/linux/elevator.h

0 commit comments

Comments
 (0)