blk-cgroup: Fix UAF in blkcg_activate_policy() by using blkg_tryget()#837
Open
blktests-ci[bot] wants to merge 1 commit into
Open
blk-cgroup: Fix UAF in blkcg_activate_policy() by using blkg_tryget()#837blktests-ci[bot] wants to merge 1 commit into
blktests-ci[bot] wants to merge 1 commit into
Conversation
Author
|
Upstream branch: aa54b1d |
b1870f6 to
ca57796
Compare
Author
|
Upstream branch: 70eda68 |
[BUG]
Our fuzz testing triggered a blkg use-after-free issue:
BUG: KASAN: slab-use-after-free in percpu_ref_put_many.constprop.0+0xbe/0xe0
Call Trace:
...
blkcg_activate_policy+0x347/0xfa0
bfq_create_group_hierarchy+0x5b/0x140
bfq_init_queue+0xc1b/0x1470
? mutex_init_generic+0x9f/0x100
? elevator_alloc+0x166/0x2b0
blk_mq_init_sched+0x2b0/0x730
elevator_switch+0x188/0x450
elevator_change+0x290/0x470
elv_iosched_store+0x30a/0x3a0
...
[CAUSE]
process1 process2
cgroup_rmdir
...
blkcg_destroy_blkgs
spin_trylock(&q->queue_lock)
blkg_destroy
percpu_ref_kill(&blkg->refcnt)
...
blkg_free
INIT_WORK(xxx, blkg_free_workfn)
schedule_work
spin_unlock(&q->queue_lock)
====================================schedule_work
blkg_free_workfn
elevator_change
...
bfq_create_group_hierarchy
blkcg_activate_policy
spin_lock_irq(&q->queue_lock)
blkg_get // get dead ref !!
pinned_blkg = blkg
spin_unlock_irq(&q->queue_lock)
spin_lock_irq(&q->queue_lock)
list_del_init(&blkg->q_node)
spin_unlock_irq(&q->queue_lock)
kfree(blkg)
blkg_put(pinned_blkg) // UAF !!
A blkg killed by blkg_destroy() stays on q->blkg_list until
blkg_free_workfn() grabs queue_lock and unlinks it. blkg_get() on a dead
percpu_ref does not resurrect the blkg, so the later blkg_put() hits freed
memory and triggers this issue.
[Fix]
Replace blkg_get() with blkg_tryget(), which fails on a dead ref and lets
the loop skip dying blkgs.
Also hoist the ref acquisition to the top of the loop so dying blkgs are
filtered out before a pd is allocated and attached. Otherwise a pd attached
to an already-destroyed blkg would never called pd_offline_fn().
Fixes: 9d179b8 ("blkcg: Fix multiple bugs in blkcg_activate_policy()")
Signed-off-by: Zizhi Wo <wozizhi@huaweicloud.com>
4bd3f82 to
9a563de
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request for series with
subject: blk-cgroup: Fix UAF in blkcg_activate_policy() by using blkg_tryget()
version: 1
url: https://patchwork.kernel.org/project/linux-block/list/?series=1095149