Skip to contentSkip to conversation

blk-cgroup: fix leaks and online flag on radix_tree_insert failure

PATCH Applied Tao Cui 2 participants 2 messages Jul 15

Subsystems (3)
lore
Share & tools
lore

Tao Cui

author
#0 lore raw

PATCH v4 1/1 blk-cgroup: fix leaks and online flag on radix_tree_insert failure

From: Tao Cui <[email protected]>

When radix_tree_insert() fails in blkg_create(), the error path has two issues:

1. blkg->online is set to true unconditionally, even when the blkg was never fully inserted. Move the assignment inside the success block.

2. The error path calls blkg_put() without first calling percpu_ref_kill(). Because the refcount is still in percpu mode, percpu_ref_put() only does this_cpu_sub() without checking for zero, so blkg_release() is never triggered. This permanently leaks the blkg memory, its percpu iostat, policy data, the parent blkg reference, and the cgroup css reference — the latter preventing the cgroup from ever being destroyed.

Fix by replacing blkg_put() with percpu_ref_kill(), matching the pattern used in blkg_destroy().

---
v4:
- Resend. No functional change. Folded in Acked-by from Tejun Heo and rebased onto current mainline (context-only: the err_put_css label was renamed to err_free_blkg upstream).

v3:
- Remove the redundant blkg_put() after percpu_ref_kill() to avoid a double-put that causes the refcount to go negative and bypass blkg_release(), as pointed out by the sashiko AI review.
v3: https://lore.kernel.org/all/[email protected]/

v2:
- Also fix the percpu_ref leak on the radix_tree_insert() error path by adding percpu_ref_kill() before blkg_put(), as pointed out by the sashiko AI review.
v1: https://lore.kernel.org/all/[email protected]/

---
 block/blk-cgroup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
block/blk-cgroup.c +2 -2
index d2a1f5903f24..d9676126c5b5 100644
@@ -434,15 +434,15 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
 				blkg->pd[i]->online = true;
 			}
 		}
+		blkg->online = true;
 	}
-	blkg->online = true;
 	spin_unlock(&blkcg->lock);
 
 	if (!ret)
 		return blkg;
 
 	/* @blkg failed fully initialized, use the usual release path */
-	blkg_put(blkg);
+	percpu_ref_kill(&blkg->refcnt);
 	return ERR_PTR(ret);
 
 err_free_blkg:
Signature · 1 line
2.43.0
maintainer Applied
#1 lore raw
Quoted 14 lines · Tao Cui

When radix_tree_insert() fails in blkg_create(), the error path has two issues:

1. blkg->online is set to true unconditionally, even when the blkg was never fully inserted. Move the assignment inside the success block.

2. The error path calls blkg_put() without first calling percpu_ref_kill(). Because the refcount is still in percpu mode, percpu_ref_put() only does this_cpu_sub() without checking for zero, so blkg_release() is never triggered. This permanently leaks the blkg memory, its percpu iostat, policy data, the parent blkg reference, and the cgroup css reference — the latter preventing the cgroup from ever being destroyed.

Quoted 1 line

[...]

Applied, thanks!

[1/1] blk-cgroup: fix leaks and online flag on radix_tree_insert failure

      commit: dbbca20764382b4d411ec2918f4e278ffe547acc

Best regards,

Signature · 1 line
Jens Axboe