Pulls in uaf fix for bfqq->bic along with fixups. I pulled in the backport dependencies that were also present in 5.15-lts.
NeilBrown (1): block/bfq-iosched.c: use "false" rather than "BLK_RW_ASYNC"
Yu Kuai (4): block, bfq: fix possible uaf for 'bfqq->bic' block, bfq: fix uaf for bfqq in bfq_exit_icq_bfqq block, bfq: replace 0/1 with false/true in bic apis block, bfq: fix uaf for bfqq in bic_set_bfqq()
block/bfq-cgroup.c | 8 ++++---- block/bfq-iosched.c | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 10 deletions(-)
From: Yu Kuai yukuai3@huawei.com
[ Upstream commit 64dc8c732f5c2b406cc752e6aaa1bd5471159cab ]
Our test report a uaf for 'bfqq->bic' in 5.10:
================================================================== BUG: KASAN: use-after-free in bfq_select_queue+0x378/0xa30
CPU: 6 PID: 2318352 Comm: fsstress Kdump: loaded Not tainted 5.10.0-60.18.0.50.h602.kasan.eulerosv2r11.x86_64 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58-20220320_160524-szxrtosci10000 04/01/2014 Call Trace: bfq_select_queue+0x378/0xa30 bfq_dispatch_request+0xe8/0x130 blk_mq_do_dispatch_sched+0x62/0xb0 __blk_mq_sched_dispatch_requests+0x215/0x2a0 blk_mq_sched_dispatch_requests+0x8f/0xd0 __blk_mq_run_hw_queue+0x98/0x180 __blk_mq_delay_run_hw_queue+0x22b/0x240 blk_mq_run_hw_queue+0xe3/0x190 blk_mq_sched_insert_requests+0x107/0x200 blk_mq_flush_plug_list+0x26e/0x3c0 blk_finish_plug+0x63/0x90 __iomap_dio_rw+0x7b5/0x910 iomap_dio_rw+0x36/0x80 ext4_dio_read_iter+0x146/0x190 [ext4] ext4_file_read_iter+0x1e2/0x230 [ext4] new_sync_read+0x29f/0x400 vfs_read+0x24e/0x2d0 ksys_read+0xd5/0x1b0 do_syscall_64+0x33/0x40 entry_SYSCALL_64_after_hwframe+0x61/0xc6
Commit 3bc5e683c67d ("bfq: Split shared queues on move between cgroups") changes that move process to a new cgroup will allocate a new bfqq to use, however, the old bfqq and new bfqq can point to the same bic:
1) Initial state, two process with io in the same cgroup.
Process 1 Process 2 (BIC1) (BIC2) | Λ | Λ | | | | V | V | bfqq1 bfqq2
2) bfqq1 is merged to bfqq2.
Process 1 Process 2 (BIC1) (BIC2) | | -------------| V bfqq1 bfqq2(coop)
3) Process 1 exit, then issue new io(denoce IOA) from Process 2.
(BIC2) | Λ | | V | bfqq2(coop)
4) Before IOA is completed, move Process 2 to another cgroup and issue io.
Process 2 (BIC2) Λ |--------------\ | V bfqq2 bfqq3
Now that BIC2 points to bfqq3, while bfqq2 and bfqq3 both point to BIC2. If all the requests are completed, and Process 2 exit, BIC2 will be freed while there is no guarantee that bfqq2 will be freed before BIC2.
Fix the problem by clearing bfqq->bic while bfqq is detached from bic.
Fixes: 3bc5e683c67d ("bfq: Split shared queues on move between cgroups") Suggested-by: Jan Kara jack@suse.cz Signed-off-by: Yu Kuai yukuai3@huawei.com Reviewed-by: Jan Kara jack@suse.cz Link: https://lore.kernel.org/r/20221214030430.3304151-1-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Sasha Levin sashal@kernel.org Signed-off-by: Khazhismel Kumykov khazhy@google.com --- block/bfq-iosched.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 7c4b8d0635eb..afaededb3c49 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -373,6 +373,12 @@ struct bfq_queue *bic_to_bfqq(struct bfq_io_cq *bic, bool is_sync)
void bic_set_bfqq(struct bfq_io_cq *bic, struct bfq_queue *bfqq, bool is_sync) { + struct bfq_queue *old_bfqq = bic->bfqq[is_sync]; + + /* Clear bic pointer if bfqq is detached from this bic */ + if (old_bfqq && old_bfqq->bic == bic) + old_bfqq->bic = NULL; + bic->bfqq[is_sync] = bfqq; }
@@ -4977,7 +4983,6 @@ static void bfq_exit_icq_bfqq(struct bfq_io_cq *bic, bool is_sync) unsigned long flags;
spin_lock_irqsave(&bfqd->lock, flags); - bfqq->bic = NULL; bfq_exit_bfqq(bfqd, bfqq); bic_set_bfqq(bic, NULL, is_sync); spin_unlock_irqrestore(&bfqd->lock, flags);
From: Yu Kuai yukuai3@huawei.com
[ Upstream commit 246cf66e300b76099b5dbd3fdd39e9a5dbc53f02 ]
Commit 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'") will access 'bic->bfqq' in bic_set_bfqq(), however, bfq_exit_icq_bfqq() can free bfqq first, and then call bic_set_bfqq(), which will cause uaf.
Fix the problem by moving bfq_exit_bfqq() behind bic_set_bfqq().
Fixes: 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'") Reported-by: Yi Zhang yi.zhang@redhat.com Signed-off-by: Yu Kuai yukuai3@huawei.com Link: https://lore.kernel.org/r/20221226030605.1437081-1-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Sasha Levin sashal@kernel.org Signed-off-by: Khazhismel Kumykov khazhy@google.com --- block/bfq-iosched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index afaededb3c49..0a53b653a7e2 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -4983,8 +4983,8 @@ static void bfq_exit_icq_bfqq(struct bfq_io_cq *bic, bool is_sync) unsigned long flags;
spin_lock_irqsave(&bfqd->lock, flags); - bfq_exit_bfqq(bfqd, bfqq); bic_set_bfqq(bic, NULL, is_sync); + bfq_exit_bfqq(bfqd, bfqq); spin_unlock_irqrestore(&bfqd->lock, flags); } }
From: NeilBrown neilb@suse.de
[ Upstream commit f6bad159f5d5e5b33531aba3d9b860ad8618afe0 ]
bfq_get_queue() expects a "bool" for the third arg, so pass "false" rather than "BLK_RW_ASYNC" which will soon be removed.
Link: https://lkml.kernel.org/r/164549983746.9187.7949730109246767909.stgit@noble.... Signed-off-by: NeilBrown neilb@suse.de Acked-by: Jens Axboe axboe@kernel.dk Cc: Anna Schumaker Anna.Schumaker@Netapp.com Cc: Chao Yu chao@kernel.org Cc: Darrick J. Wong djwong@kernel.org Cc: Ilya Dryomov idryomov@gmail.com Cc: Jaegeuk Kim jaegeuk@kernel.org Cc: Jan Kara jack@suse.cz Cc: Jeff Layton jlayton@kernel.org Cc: Lars Ellenberg lars.ellenberg@linbit.com Cc: Miklos Szeredi miklos@szeredi.hu Cc: Paolo Valente paolo.valente@linaro.org Cc: Philipp Reisner philipp.reisner@linbit.com Cc: Ryusuke Konishi konishi.ryusuke@gmail.com Cc: Trond Myklebust trond.myklebust@hammerspace.com Cc: Wu Fengguang fengguang.wu@intel.com Signed-off-by: Andrew Morton akpm@linux-foundation.org Signed-off-by: Linus Torvalds torvalds@linux-foundation.org Stable-dep-of: b600de2d7d3a ("block, bfq: fix uaf for bfqq in bic_set_bfqq()") Signed-off-by: Sasha Levin sashal@kernel.org Signed-off-by: Khazhismel Kumykov khazhy@google.com --- block/bfq-iosched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 0a53b653a7e2..35b240cba092 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -5071,7 +5071,7 @@ static void bfq_check_ioprio_change(struct bfq_io_cq *bic, struct bio *bio) bfqq = bic_to_bfqq(bic, false); if (bfqq) { bfq_release_process_ref(bfqd, bfqq); - bfqq = bfq_get_queue(bfqd, bio, BLK_RW_ASYNC, bic); + bfqq = bfq_get_queue(bfqd, bio, false, bic); bic_set_bfqq(bic, bfqq, false); }
From: Yu Kuai yukuai3@huawei.com
[ Upstream commit 337366e02b370d2800110fbc99940f6ddddcbdfa ]
Just to make the code a litter cleaner, there are no functional changes.
Signed-off-by: Yu Kuai yukuai3@huawei.com Reviewed-by: Jan Kara jack@suse.cz Link: https://lore.kernel.org/r/20221214033155.3455754-3-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe axboe@kernel.dk Stable-dep-of: b600de2d7d3a ("block, bfq: fix uaf for bfqq in bic_set_bfqq()") Signed-off-by: Sasha Levin sashal@kernel.org Signed-off-by: Khazhismel Kumykov khazhy@google.com --- block/bfq-cgroup.c | 8 ++++---- block/bfq-iosched.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c index badb90352bf3..2f440b79183d 100644 --- a/block/bfq-cgroup.c +++ b/block/bfq-cgroup.c @@ -705,15 +705,15 @@ static void *__bfq_bic_change_cgroup(struct bfq_data *bfqd, struct bfq_io_cq *bic, struct bfq_group *bfqg) { - struct bfq_queue *async_bfqq = bic_to_bfqq(bic, 0); - struct bfq_queue *sync_bfqq = bic_to_bfqq(bic, 1); + struct bfq_queue *async_bfqq = bic_to_bfqq(bic, false); + struct bfq_queue *sync_bfqq = bic_to_bfqq(bic, true); struct bfq_entity *entity;
if (async_bfqq) { entity = &async_bfqq->entity;
if (entity->sched_data != &bfqg->sched_data) { - bic_set_bfqq(bic, NULL, 0); + bic_set_bfqq(bic, NULL, false); bfq_release_process_ref(bfqd, async_bfqq); } } @@ -749,7 +749,7 @@ static void *__bfq_bic_change_cgroup(struct bfq_data *bfqd, */ bfq_put_cooperator(sync_bfqq); bfq_release_process_ref(bfqd, sync_bfqq); - bic_set_bfqq(bic, NULL, 1); + bic_set_bfqq(bic, NULL, true); } } } diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 35b240cba092..016d7f32af9f 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -2816,7 +2816,7 @@ bfq_merge_bfqqs(struct bfq_data *bfqd, struct bfq_io_cq *bic, /* * Merge queues (that is, let bic redirect its requests to new_bfqq) */ - bic_set_bfqq(bic, new_bfqq, 1); + bic_set_bfqq(bic, new_bfqq, true); bfq_mark_bfqq_coop(new_bfqq); /* * new_bfqq now belongs to at least two bics (it is a shared queue): @@ -6014,7 +6014,7 @@ bfq_split_bfqq(struct bfq_io_cq *bic, struct bfq_queue *bfqq) return bfqq; }
- bic_set_bfqq(bic, NULL, 1); + bic_set_bfqq(bic, NULL, true);
bfq_put_cooperator(bfqq);
From: Yu Kuai yukuai3@huawei.com
[ Upstream commit b600de2d7d3a16f9007fad1bdae82a3951a26af2 ]
After commit 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'"), bic->bfqq will be accessed in bic_set_bfqq(), however, in some context bic->bfqq will be freed, and bic_set_bfqq() is called with the freed bic->bfqq.
Fix the problem by always freeing bfqq after bic_set_bfqq().
Fixes: 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'") Reported-and-tested-by: Shinichiro Kawasaki shinichiro.kawasaki@wdc.com Signed-off-by: Yu Kuai yukuai3@huawei.com Reviewed-by: Jan Kara jack@suse.cz Link: https://lore.kernel.org/r/20230130014136.591038-1-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Sasha Levin sashal@kernel.org Signed-off-by: Khazhismel Kumykov khazhy@google.com --- block/bfq-cgroup.c | 2 +- block/bfq-iosched.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c index 2f440b79183d..1f9ccc661d57 100644 --- a/block/bfq-cgroup.c +++ b/block/bfq-cgroup.c @@ -748,8 +748,8 @@ static void *__bfq_bic_change_cgroup(struct bfq_data *bfqd, * request from the old cgroup. */ bfq_put_cooperator(sync_bfqq); - bfq_release_process_ref(bfqd, sync_bfqq); bic_set_bfqq(bic, NULL, true); + bfq_release_process_ref(bfqd, sync_bfqq); } } } diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 016d7f32af9f..6687b805bab3 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -5070,9 +5070,11 @@ static void bfq_check_ioprio_change(struct bfq_io_cq *bic, struct bio *bio)
bfqq = bic_to_bfqq(bic, false); if (bfqq) { - bfq_release_process_ref(bfqd, bfqq); + struct bfq_queue *old_bfqq = bfqq; + bfqq = bfq_get_queue(bfqd, bio, false, bic); bic_set_bfqq(bic, bfqq, false); + bfq_release_process_ref(bfqd, old_bfqq); }
bfqq = bic_to_bfqq(bic, true);
On Mon, Mar 13, 2023 at 03:27:52PM -0700, Khazhismel Kumykov wrote:
Pulls in uaf fix for bfqq->bic along with fixups. I pulled in the backport dependencies that were also present in 5.15-lts.
NeilBrown (1): block/bfq-iosched.c: use "false" rather than "BLK_RW_ASYNC"
Yu Kuai (4): block, bfq: fix possible uaf for 'bfqq->bic' block, bfq: fix uaf for bfqq in bfq_exit_icq_bfqq block, bfq: replace 0/1 with false/true in bic apis block, bfq: fix uaf for bfqq in bic_set_bfqq()
block/bfq-cgroup.c | 8 ++++---- block/bfq-iosched.c | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 10 deletions(-)
Queued up, thanks!
linux-stable-mirror@lists.linaro.org
-
Khazhismel Kumykov -
Sasha Levin