[PATCH v4 0/4] blk-cgroup: fix blkg list and policy data races

[PATCH v4 0/4] blk-cgroup: fix blkg list and policy data races

[Yu Kuai]

From: Yu Kuai <yukuai@fygo.io>

This set fix some problems that are reported long time ago with minimal changes,
the blkcg_mutex refactor I'm working on can fix these problems as well, but it's
complicated and may not land in this merge window. So I think this set should land
in this merge window first.

Patch 1 protects blkg_destroy_all()'s q->blkg_list walk with
blkcg_mutex.

Patches 2-3 fix races between blkcg_activate_policy() and concurrent
blkg destruction.

Patch 4 factors the policy data teardown loop into a helper after the
race fixes.

Changes since v3:
- Rebase on the latest for-7.3/block branch.

Changes since v2:
- Rebase on the latest block-7.2 branch.

Changes since v1:
- Drop the BFQ q->blkg_list patch because the current block tree already
  has a stronger fix in commit 17b2d950a3c0 ("block, bfq: protect async
  queue reset with blkcg locks").
- Add Reviewed-by tags from Tang Yizhou.

Yu Kuai (1):
  blk-cgroup: protect q->blkg_list iteration in blkg_destroy_all() with
    blkcg_mutex

Zheng Qixing (3):
  blk-cgroup: fix race between policy activation and blkg destruction
  blk-cgroup: skip dying blkg in blkcg_activate_policy()
  blk-cgroup: factor policy pd teardown loop into helper

 block/blk-cgroup.c | 65 +++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 30 deletions(-)

-- 
2.51.0

[PATCH v4 1/4] blk-cgroup: protect q->blkg_list iteration in blkg_destroy_all() with blkcg_mutex

[Yu Kuai]

From: Yu Kuai <yukuai@fygo.io>

blkg_destroy_all() iterates q->blkg_list without holding blkcg_mutex,
which can race with blkg_free_workfn() that removes blkgs from the list
while holding blkcg_mutex.

Add blkcg_mutex protection around the q->blkg_list iteration to prevent
potential list corruption or use-after-free issues.

Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
 block/blk-cgroup.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index d9676126c5b5..eb0cfb10b859 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -569,6 +569,7 @@ static void blkg_destroy_all(struct gendisk *disk)
 	int i;
 
 restart:
+	mutex_lock(&q->blkcg_mutex);
 	spin_lock_irq(&q->queue_lock);
 	list_for_each_entry(blkg, &q->blkg_list, q_node) {
 		struct blkcg *blkcg = blkg->blkcg;
@@ -587,6 +588,7 @@ static void blkg_destroy_all(struct gendisk *disk)
 		if (!(--count)) {
 			count = BLKG_DESTROY_BATCH_SIZE;
 			spin_unlock_irq(&q->queue_lock);
+			mutex_unlock(&q->blkcg_mutex);
 			cond_resched();
 			goto restart;
 		}
@@ -606,6 +608,7 @@ static void blkg_destroy_all(struct gendisk *disk)
 
 	q->root_blkg = NULL;
 	spin_unlock_irq(&q->queue_lock);
+	mutex_unlock(&q->blkcg_mutex);
 
 	wake_up_var(&q->root_blkg);
 }
-- 
2.51.0

Re: [PATCH v4 1/4] blk-cgroup: protect q->blkg_list iteration in blkg_destroy_all() with blkcg_mutex

[Tao Cui]

于 2026年8月2日 GMT+08:00 19:25:17，Yu Kuai <yukuai@kernel.org> 写道：
>From: Yu Kuai <yukuai@fygo.io>
>
>blkg_destroy_all() iterates q->blkg_list without holding blkcg_mutex,
>which can race with blkg_free_workfn() that removes blkgs from the list
>while holding blkcg_mutex.
>
>Add blkcg_mutex protection around the q->blkg_list iteration to prevent
>potential list corruption or use-after-free issues.
>
>Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
>Signed-off-by: Yu Kuai <yukuai@fygo.io>
>---
> block/blk-cgroup.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
>index d9676126c5b5..eb0cfb10b859 100644
>--- a/block/blk-cgroup.c
>+++ b/block/blk-cgroup.c
>@@ -569,6 +569,7 @@ static void blkg_destroy_all(struct gendisk *disk)
> 	int i;
> 
> restart:
>+	mutex_lock(&q->blkcg_mutex);
> 	spin_lock_irq(&q->queue_lock);
> 	list_for_each_entry(blkg, &q->blkg_list, q_node) {
> 		struct blkcg *blkcg = blkg->blkcg;
>@@ -587,6 +588,7 @@ static void blkg_destroy_all(struct gendisk *disk)
> 		if (!(--count)) {
> 			count = BLKG_DESTROY_BATCH_SIZE;
> 			spin_unlock_irq(&q->queue_lock);
>+			mutex_unlock(&q->blkcg_mutex);
> 			cond_resched();
> 			goto restart;
> 		}
>@@ -606,6 +608,7 @@ static void blkg_destroy_all(struct gendisk *disk)
> 
> 	q->root_blkg = NULL;
> 	spin_unlock_irq(&q->queue_lock);
>+	mutex_unlock(&q->blkcg_mutex);
> 
> 	wake_up_var(&q->root_blkg);
> }
Reviewed-by: Tao Cui <cuitao@kylinos.cn>

[PATCH v4 2/4] blk-cgroup: fix race between policy activation and blkg destruction

[Yu Kuai]

From: Zheng Qixing <zhengqixing@huawei.com>

When switching an IO scheduler on a block device, blkcg_activate_policy()
allocates blkg_policy_data (pd) for all blkgs attached to the queue.
However, blkcg_activate_policy() may race with concurrent blkcg deletion,
leading to use-after-free and memory leak issues.

The use-after-free occurs in the following race:

T1 (blkcg_activate_policy):
  - Successfully allocates pd for blkg1 (loop0->queue, blkcgA)
  - Fails to allocate pd for blkg2 (loop0->queue, blkcgB)
  - Enters the enomem rollback path to release blkg1 resources

T2 (blkcg deletion):
  - blkcgA is deleted concurrently
  - blkg1 is freed via blkg_free_workfn()
  - blkg1->pd is freed

T1 (continued):
  - Rollback path accesses blkg1->pd->online after pd is freed
  - Triggers use-after-free

In addition, blkg_free_workfn() frees pd before removing the blkg from
q->blkg_list. This allows blkcg_activate_policy() to allocate a new pd
for a blkg that is being destroyed, leaving the newly allocated pd
unreachable when the blkg is finally freed.

Fix these races by extending blkcg_mutex coverage to serialize
blkcg_activate_policy() rollback and blkg destruction, ensuring pd
lifecycle is synchronized with blkg list visibility.

Fixes: f1c006f1c685 ("blk-cgroup: synchronize pd_free_fn() from blkg_free_workfn() and blkcg_deactivate_policy()")
Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
 block/blk-cgroup.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index eb0cfb10b859..047bb42c282b 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1566,6 +1566,8 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
 
 	if (queue_is_mq(q))
 		memflags = blk_mq_freeze_queue(q);
+
+	mutex_lock(&q->blkcg_mutex);
 retry:
 	spin_lock_irq(&q->queue_lock);
 
@@ -1628,6 +1630,7 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
 
 	spin_unlock_irq(&q->queue_lock);
 out:
+	mutex_unlock(&q->blkcg_mutex);
 	if (queue_is_mq(q))
 		blk_mq_unfreeze_queue(q, memflags);
 	if (pinned_blkg)
-- 
2.51.0

Re: [PATCH v4 2/4] blk-cgroup: fix race between policy activation and blkg destruction

[Tao Cui]

于 2026年8月2日 GMT+08:00 19:25:18，Yu Kuai <yukuai@kernel.org> 写道：
>From: Zheng Qixing <zhengqixing@huawei.com>
>
>When switching an IO scheduler on a block device, blkcg_activate_policy()
>allocates blkg_policy_data (pd) for all blkgs attached to the queue.
>However, blkcg_activate_policy() may race with concurrent blkcg deletion,
>leading to use-after-free and memory leak issues.
>
>The use-after-free occurs in the following race:
>
>T1 (blkcg_activate_policy):
>  - Successfully allocates pd for blkg1 (loop0->queue, blkcgA)
>  - Fails to allocate pd for blkg2 (loop0->queue, blkcgB)
>  - Enters the enomem rollback path to release blkg1 resources
>
>T2 (blkcg deletion):
>  - blkcgA is deleted concurrently
>  - blkg1 is freed via blkg_free_workfn()
>  - blkg1->pd is freed
>
>T1 (continued):
>  - Rollback path accesses blkg1->pd->online after pd is freed
>  - Triggers use-after-free
>
>In addition, blkg_free_workfn() frees pd before removing the blkg from
>q->blkg_list. This allows blkcg_activate_policy() to allocate a new pd
>for a blkg that is being destroyed, leaving the newly allocated pd
>unreachable when the blkg is finally freed.
>
>Fix these races by extending blkcg_mutex coverage to serialize
>blkcg_activate_policy() rollback and blkg destruction, ensuring pd
>lifecycle is synchronized with blkg list visibility.
>
>Fixes: f1c006f1c685 ("blk-cgroup: synchronize pd_free_fn() from blkg_free_workfn() and blkcg_deactivate_policy()")
>Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
>Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
>Signed-off-by: Yu Kuai <yukuai@fygo.io>
>---
> block/blk-cgroup.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
>index eb0cfb10b859..047bb42c282b 100644
>--- a/block/blk-cgroup.c
>+++ b/block/blk-cgroup.c
>@@ -1566,6 +1566,8 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
> 
> 	if (queue_is_mq(q))
> 		memflags = blk_mq_freeze_queue(q);
>+
>+	mutex_lock(&q->blkcg_mutex);
> retry:
> 	spin_lock_irq(&q->queue_lock);
> 
>@@ -1628,6 +1630,7 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
> 
> 	spin_unlock_irq(&q->queue_lock);
> out:
>+	mutex_unlock(&q->blkcg_mutex);
> 	if (queue_is_mq(q))
> 		blk_mq_unfreeze_queue(q, memflags);
> 	if (pinned_blkg)
Reviewed-by: Tao Cui <cuitao@kylinos.cn>

[PATCH v4 3/4] blk-cgroup: skip dying blkg in blkcg_activate_policy()

[Yu Kuai]

From: Zheng Qixing <zhengqixing@huawei.com>

When switching IO schedulers on a block device, blkcg_activate_policy()
can race with concurrent blkcg deletion, leading to a use-after-free in
rcu_accelerate_cbs.

T1:                               T2:
                                  blkg_destroy
                                  kill(&blkg->refcnt) // blkg->refcnt=1->0
                                  blkg_release // call_rcu(__blkg_release)
                                  ...
                                  blkg_free_workfn
                                  ->pd_free_fn(pd)
elv_iosched_store
elevator_switch
...
iterate blkg list
blkg_get(blkg) // blkg->refcnt=0->1
                                  list_del_init(&blkg->q_node)
blkg_put(pinned_blkg) // blkg->refcnt=1->0
blkg_release // call_rcu again
rcu_accelerate_cbs // uaf

Fix this by checking hlist_unhashed(&blkg->blkcg_node) before getting
a reference to the blkg. This is the same check used in blkg_destroy()
to detect if a blkg has already been destroyed. If the blkg is already
unhashed, skip processing it since it's being destroyed.

Fixes: f1c006f1c685 ("blk-cgroup: synchronize pd_free_fn() from blkg_free_workfn() and blkcg_deactivate_policy()")
Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
 block/blk-cgroup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 047bb42c282b..d1895bc60fcf 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1577,6 +1577,8 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
 
 		if (blkg->pd[pol->plid])
 			continue;
+		if (hlist_unhashed(&blkg->blkcg_node))
+			continue;
 
 		/* If prealloc matches, use it; otherwise try GFP_NOWAIT */
 		if (blkg == pinned_blkg) {
-- 
2.51.0

Re: [PATCH v4 3/4] blk-cgroup: skip dying blkg in blkcg_activate_policy()

[Tao Cui]

于 2026年8月2日 GMT+08:00 19:25:19，Yu Kuai <yukuai@kernel.org> 写道：
>From: Zheng Qixing <zhengqixing@huawei.com>
>
>When switching IO schedulers on a block device, blkcg_activate_policy()
>can race with concurrent blkcg deletion, leading to a use-after-free in
>rcu_accelerate_cbs.
>
>T1:                               T2:
>                                  blkg_destroy
>                                  kill(&blkg->refcnt) // blkg->refcnt=1->0
>                                  blkg_release // call_rcu(__blkg_release)
>                                  ...
>                                  blkg_free_workfn
>                                  ->pd_free_fn(pd)
>elv_iosched_store
>elevator_switch
>...
>iterate blkg list
>blkg_get(blkg) // blkg->refcnt=0->1
>                                  list_del_init(&blkg->q_node)
>blkg_put(pinned_blkg) // blkg->refcnt=1->0
>blkg_release // call_rcu again
>rcu_accelerate_cbs // uaf
>
>Fix this by checking hlist_unhashed(&blkg->blkcg_node) before getting
>a reference to the blkg. This is the same check used in blkg_destroy()
>to detect if a blkg has already been destroyed. If the blkg is already
>unhashed, skip processing it since it's being destroyed.
>
>Fixes: f1c006f1c685 ("blk-cgroup: synchronize pd_free_fn() from blkg_free_workfn() and blkcg_deactivate_policy()")
>Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
>Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
>Signed-off-by: Yu Kuai <yukuai@fygo.io>
>---
> block/blk-cgroup.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
>index 047bb42c282b..d1895bc60fcf 100644
>--- a/block/blk-cgroup.c
>+++ b/block/blk-cgroup.c
>@@ -1577,6 +1577,8 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
> 
> 		if (blkg->pd[pol->plid])
> 			continue;
>+		if (hlist_unhashed(&blkg->blkcg_node))
>+			continue;
> 
> 		/* If prealloc matches, use it; otherwise try GFP_NOWAIT */
> 		if (blkg == pinned_blkg) {
Reviewed-by: Tao Cui <cuitao@kylinos.cn>

[PATCH v4 4/4] blk-cgroup: factor policy pd teardown loop into helper

[Yu Kuai]

From: Zheng Qixing <zhengqixing@huawei.com>

Move the teardown sequence which offlines and frees per-policy
blkg_policy_data (pd) into a helper for readability.

No functional change intended.

Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
 block/blk-cgroup.c | 57 ++++++++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 30 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index d1895bc60fcf..354637f3b158 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1529,6 +1529,31 @@ struct cgroup_subsys io_cgrp_subsys = {
 };
 EXPORT_SYMBOL_GPL(io_cgrp_subsys);
 
+/*
+ * Tear down per-blkg policy data for @pol on @q.
+ */
+static void blkcg_policy_teardown_pds(struct request_queue *q,
+				      const struct blkcg_policy *pol)
+{
+	struct blkcg_gq *blkg;
+
+	list_for_each_entry(blkg, &q->blkg_list, q_node) {
+		struct blkcg *blkcg = blkg->blkcg;
+		struct blkg_policy_data *pd;
+
+		spin_lock(&blkcg->lock);
+		pd = blkg->pd[pol->plid];
+		if (pd) {
+			if (pd->online && pol->pd_offline_fn)
+				pol->pd_offline_fn(pd);
+			pd->online = false;
+			pol->pd_free_fn(pd);
+			WRITE_ONCE(blkg->pd[pol->plid], NULL);
+		}
+		spin_unlock(&blkcg->lock);
+	}
+}
+
 /**
  * blkcg_activate_policy - activate a blkcg policy on a gendisk
  * @disk: gendisk of interest
@@ -1644,21 +1669,7 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
 enomem:
 	/* alloc failed, take down everything */
 	spin_lock_irq(&q->queue_lock);
-	list_for_each_entry(blkg, &q->blkg_list, q_node) {
-		struct blkcg *blkcg = blkg->blkcg;
-		struct blkg_policy_data *pd;
-
-		spin_lock(&blkcg->lock);
-		pd = blkg->pd[pol->plid];
-		if (pd) {
-			if (pd->online && pol->pd_offline_fn)
-				pol->pd_offline_fn(pd);
-			pd->online = false;
-			pol->pd_free_fn(pd);
-			WRITE_ONCE(blkg->pd[pol->plid], NULL);
-		}
-		spin_unlock(&blkcg->lock);
-	}
+	blkcg_policy_teardown_pds(q, pol);
 	spin_unlock_irq(&q->queue_lock);
 	ret = -ENOMEM;
 	goto out;
@@ -1677,7 +1688,6 @@ void blkcg_deactivate_policy(struct gendisk *disk,
 			     const struct blkcg_policy *pol)
 {
 	struct request_queue *q = disk->queue;
-	struct blkcg_gq *blkg;
 	unsigned int memflags;
 
 	if (!blkcg_policy_enabled(q, pol))
@@ -1690,20 +1700,7 @@ void blkcg_deactivate_policy(struct gendisk *disk,
 	spin_lock_irq(&q->queue_lock);
 
 	__clear_bit(pol->plid, q->blkcg_pols);
-
-	list_for_each_entry(blkg, &q->blkg_list, q_node) {
-		struct blkcg *blkcg = blkg->blkcg;
-
-		spin_lock(&blkcg->lock);
-		if (blkg->pd[pol->plid]) {
-			if (blkg->pd[pol->plid]->online && pol->pd_offline_fn)
-				pol->pd_offline_fn(blkg->pd[pol->plid]);
-			pol->pd_free_fn(blkg->pd[pol->plid]);
-			blkg->pd[pol->plid] = NULL;
-		}
-		spin_unlock(&blkcg->lock);
-	}
-
+	blkcg_policy_teardown_pds(q, pol);
 	spin_unlock_irq(&q->queue_lock);
 	mutex_unlock(&q->blkcg_mutex);
 
-- 
2.51.0

[PATCH v4 0/4] blk-cgroup: fix blkg list and policy data races

[Yu Kuai]

From: Yu Kuai <yukuai@fygo.io>

This set fix some problems that are reported long time ago with minimal changes,
the blkcg_mutex refactor I'm working on can fix these problems as well, but it's
complicated and may not land in this merge window. So I think this set should land
in this merge window first.

Patch 1 protects blkg_destroy_all()'s q->blkg_list walk with
blkcg_mutex.

Patches 2-3 fix races between blkcg_activate_policy() and concurrent
blkg destruction.

Patch 4 factors the policy data teardown loop into a helper after the
race fixes.

Changes since v3:
- Rebase on the latest for-7.3/block branch.

Changes since v2:
- Rebase on the latest block-7.2 branch.

Changes since v1:
- Drop the BFQ q->blkg_list patch because the current block tree already
  has a stronger fix in commit 17b2d950a3c0 ("block, bfq: protect async
  queue reset with blkcg locks").
- Add Reviewed-by tags from Tang Yizhou.

Yu Kuai (1):
  blk-cgroup: protect q->blkg_list iteration in blkg_destroy_all() with
    blkcg_mutex

Zheng Qixing (3):
  blk-cgroup: fix race between policy activation and blkg destruction
  blk-cgroup: skip dying blkg in blkcg_activate_policy()
  blk-cgroup: factor policy pd teardown loop into helper

 block/blk-cgroup.c | 65 +++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 30 deletions(-)

-- 
2.51.0