Linux cgroups development
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.18-5.10] blk-cgroup: fix leaks and online flag on radix_tree_insert failure
       [not found] <20260831133314.4125787-1-sashal@kernel.org>
@ 2026-08-31 13:25 ` Sasha Levin
  2026-08-31 13:28 ` [PATCH AUTOSEL 6.18-6.1] blk-cgroup: protect iterating blkgs with blkcg->lock in blkcg_print_stat() Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-08-31 13:25 UTC (permalink / raw)
  To: patches, stable
  Cc: Tao Cui, Tejun Heo, Jens Axboe, Sasha Levin, josef, cgroups,
	linux-block, linux-kernel

From: Tao Cui <cuitao@kylinos.cn>

[ Upstream commit dbbca20764382b4d411ec2918f4e278ffe547acc ]

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().

Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Link: https://patch.msgid.link/20260715132407.1469777-1-cui.tao@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

# Stable Backport Analysis: `blk-cgroup: fix leaks and online flag on
radix_tree_insert failure`

**Local tree:** `v6.18.44` (Linux 6.18.44 stable)

---

## PHASE 1: COMMIT MESSAGE FORENSICS

**Step 1.1 — Subject line**

Record: `[blk-cgroup]` `[fix]` — fixes resource leaks and incorrect
`online` flag when `radix_tree_insert()` fails in `blkg_create()`.

**Step 1.2 — Tags**

Record:
- **Acked-by:** Tejun Heo `<tj@kernel.org>` (cgroup/block-cgroup
  maintainer)
- **Signed-off-by:** Tao Cui `<cuitao@kylinos.cn>` (author)
- **Signed-off-by:** Jens Axboe `<axboe@kernel.dk>` (block layer
  maintainer)
- **Link:**
  https://patch.msgid.link/20260715132407.1469777-1-cui.tao@linux.dev
- No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, or Cc: stable tags
- (Ignoring pipeline-added Signed-off-by: Sasha Levin per instructions)

**Step 1.3 — Body analysis**

Record:
- **Bug:** When `radix_tree_insert()` fails in `blkg_create()`, two
  errors occur:
  1. `blkg->online = true` is set even though the blkg was never
     inserted into the tree.
  2. Error path calls `blkg_put()` without `percpu_ref_kill()`. While
     the refcount is still in percpu mode, `percpu_ref_put()` only
     decrements a per-CPU counter and never checks for zero, so
     `blkg_release()` is never called.
- **Symptom/failure mode:** Permanent leak of blkg memory, percpu
  iostat, policy data, parent blkg reference, and cgroup css reference —
  the css leak prevents the cgroup from ever being destroyed.
- **Root cause:** Wrong teardown primitive on the error path;
  `blkg_destroy()` correctly uses `percpu_ref_kill()`.

**Step 1.4 — Hidden bug fix?**

Record: No — this is an explicit bug fix, not disguised cleanup.

---

## PHASE 2: DIFF ANALYSIS

**Step 2.1 — Inventory**

Record:
- **Files:** `block/blk-cgroup.c` only (+2 / −2 lines, 4 lines touched)
- **Function:** `blkg_create()`
- **Scope:** Single-file, surgical fix

**Step 2.2 — Code flow change**

Record:
- **Hunk 1:** `blkg->online = true` moved inside the `if (likely(!ret))`
  success block.
  - Before: online set unconditionally after insert attempt.
  - After: online only set when insert succeeds.
- **Hunk 2:** Error path changed from `blkg_put(blkg)` to
  `percpu_ref_kill(&blkg->refcnt)`.
  - Before: percpu-mode put never triggers release callback.
  - After: switches to atomic mode and triggers `blkg_release()` →
    `__blkg_release()` → `css_put()` + `blkg_free()`.

**Step 2.3 — Bug mechanism**

Record: **Reference counting / resource leak fix.** Category (a) error-
path leak + (g) logic correctness (online flag). The percpu_ref
lifecycle requires `percpu_ref_kill()` before the final drop can trigger
the release function — documented in `include/linux/percpu-refcount.h`
lines 19–24.

**Step 2.4 — Fix quality**

Record: Obviously correct — mirrors `blkg_destroy()` at line 568.
Minimal change. Very low regression risk; only affects the rare
`radix_tree_insert()` failure path.

---

## PHASE 3: GIT HISTORY INVESTIGATION

**Step 3.1 — Blame**

Record: Buggy lines in this tree all from `5d324e5159d9e` (v6.18 merge,
Nov 2025). Same pattern present in `v6.12` and `v6.17` per `git show`.

**Step 3.2 — Fixes: tag**

Record: Not applicable — no Fixes: tag in commit message.

**Step 3.3 — Related file history**

Record:
- `93383b6681074` — "wait for blkcg cleanup before initializing new
  disk" — reduces `-EEXIST` from `radix_tree_insert()` during disk
  rebind, but does not fix the broken error path when insert still
  fails.
- `5e5b7f2ef8549` — UAF fix in `__blkcg_rstat_flush()` (related
  subsystem, separate issue).
- Fix commit on master: `dbbca20764382` (Jul 15, 2026); **not** an
  ancestor of current HEAD (`merge-base` exit 1).

**Step 3.4 — Author context**

Record: Tao Cui; Acked-by Tejun Heo (blk-cgroup/cgroup maintainer). No
other Tao Cui commits in this tree's `block/blk-cgroup.c` history.

**Step 3.5 — Dependencies**

Record: Standalone — no series dependencies, no prerequisite commits
required. Self-contained 4-line change.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

**Step 4.1 — Original discussion**

Record:
- `b4 dig -c dbbca20764382`:
  https://patch.msgid.link/20260715132407.1469777-1-cui.tao@linux.dev
- Series: v4 only (no v1–v3 in b4 results; v4 is the applied version)
- No NAKs found in saved mbox
- No explicit Cc: stable nomination in thread headers

**Step 4.2 — Reviewers**

Record: `b4 dig -w` CC'd: tj@kernel.org, axboe@kernel.dk,
josef@toxicpanda.com, cgroups@vger.kernel.org, linux-
block@vger.kernel.org. Tejun Heo Acked-by.

**Step 4.3 — Bug report**

Record: No external bug report or syzbot link. Bug identified via code
review of percpu_ref lifecycle.

**Step 4.4 — Related patches**

Record: Complementary to `93383b6681074` (reduces trigger frequency) but
independently needed for correct error handling.

**Step 4.5 — Stable list**

Record: No stable@vger.kernel.org discussion found for this specific
fix.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

**Step 5.1 — Key functions**

Record: `blkg_create()` modified; related: `blkg_destroy()`,
`blkg_release()`, `__blkg_release()`, `blkg_free()`.

**Step 5.2 — Callers**

Record: `blkg_create()` called from:
- `blkg_lookup_create()` — I/O hot path via `blkg_tryget_closest()` →
  `bio_assoc_blkcg()` (line 2113)
- `blkg_conf_prep()` — cgroup sysfs configuration (uses
  `radix_tree_preload`)
- `blkcg_init_disk()` — disk initialization (uses `radix_tree_preload`)

`blkg_lookup_create()` does **not** call `radix_tree_preload()`, so
`-ENOMEM` from `radix_tree_insert()` is reachable under memory pressure.

**Step 5.3 — Callees**

Record: On failure path after fix: `percpu_ref_kill()` →
`blkg_release()` → `__blkcg_rstat_flush()` + `call_rcu(__blkg_release)`
→ `css_put()` + `blkg_free()` → `blkg_free_workfn()` releases parent
ref, policy data, queue ref, percpu iostat.

**Step 5.4 — Reachability**

Record: Reachable from block I/O path when `CONFIG_BLK_CGROUP` is
enabled and a new blkg must be created for a cgroup/disk pair. Userspace
cgroup management can also trigger via `blkg_conf_prep()`. Unprivileged
users can trigger via I/O in their cgroup.

**Step 5.5 — Similar patterns**

Record: `blkg_destroy()` at line 568 already uses
`percpu_ref_kill(&blkg->refcnt)` — fix aligns error path with
established pattern. `include/linux/percpu-refcount.h` documents that
`percpu_ref_put()` does not check for zero before `percpu_ref_kill()`.

---

## PHASE 6: CROSS-REFERENCE WITH LOCAL TREE

**Step 6.1 — Buggy code present?**

Record: **YES.** Current tree at lines 436 and 443:

```436:444:block/blk-cgroup.c
        blkg->online = true;
        spin_unlock(&blkcg->lock);

        if (!ret)
                return blkg;

        /* @blkg failed fully initialized, use the usual release path */
        blkg_put(blkg);
        return ERR_PTR(ret);
```

Bug present since at least v6.12 in this repository's history.

**Step 6.2 — Backport complications**

Record: Trivial change; `git apply --check` on upstream patch fails only
because stable has `err_put_css:` label that mainline parent lacks
(context line difference below the hunk). The three actual changed lines
apply without modification. Expected difficulty: **minor context
adjustment, not rework**.

**Step 6.3 — Related fixes already present?**

Record: `93383b6681074` is present (reduces `-EEXIST` trigger). This
specific leak fix is **not** present.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

**Step 7.1 — Subsystem**

Record: **block/blk-cgroup** — CORE/IMPORTANT subsystem. Affects all
systems using cgroup v1/v2 block controller (`CONFIG_BLK_CGROUP`).

**Step 7.2 — Activity**

Record: Active maintenance in 6.18.y — recent fixes include UAF
(`5e5b7f2ef8549`), disk reference leak (`b3e005f16cd98`), blkcg cleanup
wait (`93383b6681074`).

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

**Step 8.1 — Who is affected**

Record: Systems with `CONFIG_BLK_CGROUP` enabled — container hosts
(Kubernetes, Docker, systemd cgroups), cloud VMs, any workload using
block I/O cgroup controller.

**Step 8.2 — Trigger conditions**

Record:
- `radix_tree_insert()` returns error (`-ENOMEM` most likely in
  `blkg_lookup_create()` without preload; `-EEXIST` possible in races
  despite `93383b6681074`)
- Requires blkg creation for a new cgroup/disk pair
- Unprivileged cgroup users can trigger via I/O; cgroup admin via sysfs
- Not every boot — requires memory pressure or specific race — but
  consequences are permanent

**Step 8.3 — Failure mode severity**

Record:
- **Permanent memory/resource leak** (blkg, iostat, policy data)
- **Cgroup css reference leak → cgroup cannot be destroyed** —
  functional breakage for container lifecycle
- **Incorrect online flag** — minor (e.g., `blkcg_print_one_stat()` at
  line 1190 may process a non-inserted blkg)
- Severity: **HIGH** (resource leak with cgroup destruction blocked; not
  a crash but serious operational impact)

**Step 8.4 — Risk-benefit**

Record:
- **Benefit:** HIGH — prevents unrecoverable resource leaks and stuck
  cgroups
- **Risk:** VERY LOW — 4-line change, matches existing `blkg_destroy()`
  pattern, only affects error path
- **Ratio:** Strongly favors backport

---

## PHASE 9: FINAL SYNTHESIS

**Step 9.1 — Evidence summary**

**FOR:**
- Real, verifiable resource leak on error path
- Cgroup css leak prevents cgroup destruction — serious for production
  container workloads
- Small, surgical, maintainer-acked fix
- Buggy code confirmed present in v6.18.44
- Matches documented percpu_ref semantics and existing `blkg_destroy()`
  pattern
- Reachable from common I/O and cgroup configuration paths

**AGAINST:**
- Rare trigger (radix_tree_insert failure)
- No syzbot/user crash report
- Upstream patch needs trivial context adjustment for `err_put_css:`
  label

**UNRESOLVED:**
- No quantitative data on how often `radix_tree_insert()` fails in
  production

**Step 9.2 — Stable rules checklist**

1. Obviously correct and tested? **PASS** — mechanism verified against
   `percpu-refcount.h` and `blkg_destroy()`; Acked-by cgroup maintainer
2. Fixes real bug affecting users? **PASS** — permanent leak + cgroup
   destruction blocked
3. Important issue? **PASS** — HIGH severity resource leak affecting
   cgroup lifecycle
4. Small and contained? **PASS** — 4 lines, one function, one file
5. No new features or APIs? **PASS**
6. Can apply to local tree? **PASS** — trivial manual apply (minor
   context difference only)

**Step 9.3 — Exception categories**

Record: Not applicable (standard bug fix, not device ID/quirk/build/doc
exception).

**Step 9.4 — Decision rationale**

This commit fixes a genuine error-path bug in `blkg_create()` where
failed `radix_tree_insert()` leaves resources permanently leaked because
`blkg_put()` cannot trigger `blkg_release()` while the percpu refcount
is still in percpu mode. The css reference leak prevents cgroup
destruction — a significant operational issue for any system using block
cgroups. The fix is minimal, follows the established `blkg_destroy()`
pattern, is acked by the cgroup maintainer, and the buggy code is
present in this v6.18.44 tree without the fix.

---

## Verification

- [Phase 1] `git show dbbca20764382`: parsed subject, tags, body; Acked-
  by Tejun Heo confirmed
- [Phase 2] Read `block/blk-cgroup.c` lines 371–452, 524–569, 1190; read
  `include/linux/percpu-refcount.h` lines 19–24, 147–150, 326–352
- [Phase 2] Confirmed `blkg_put()` → `percpu_ref_put()` in `block/blk-
  cgroup.h` line 336–338
- [Phase 3] `git describe HEAD`: v6.18.44; `git blame -L 419,452`: buggy
  code from 5d324e5159d9e
- [Phase 3] `git merge-base --is-ancestor dbbca20764382 HEAD`: exit 1 —
  fix NOT in tree
- [Phase 3] `git show v6.12/v6.17:v6.18:block/blk-cgroup.c`: same buggy
  pattern back to v6.12
- [Phase 3] `93383b6681074` present in tree; complementary, not
  substitute
- [Phase 4] `b4 dig -c dbbca20764382`: lore URL found; v4 series
- [Phase 4] `b4 dig -w`: maintainers CC'd; Tejun Heo Acked-by
- [Phase 4] `b4 dig -m /tmp/blkcg_fix.mbox`: no Cc: stable in thread
- [Phase 5] Grep callers: `blkg_create` at lines 510, 936, 1526;
  `blkg_lookup_create` at 2113
- [Phase 5] Confirmed `blkg_lookup_create` does not use
  `radix_tree_preload` (only `blkg_conf_prep` and `blkcg_init_disk` do)
- [Phase 6] Buggy code at lines 436, 443 confirmed in working tree
- [Phase 6] `git apply --check` fails on context below hunk
  (`err_put_css:` vs `err_free_blkg:`); actual 3 changed lines are
  identical — trivial backport
- [Phase 8] Failure mode: permanent leak + cgroup destruction blocked;
  severity HIGH

**YES**

 block/blk-cgroup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 812f60905a708..9aa3b6ae81ca8 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -432,15 +432,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_put_css:
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH AUTOSEL 6.18-6.1] blk-cgroup: protect iterating blkgs with blkcg->lock in blkcg_print_stat()
       [not found] <20260831133314.4125787-1-sashal@kernel.org>
  2026-08-31 13:25 ` [PATCH AUTOSEL 6.18-5.10] blk-cgroup: fix leaks and online flag on radix_tree_insert failure Sasha Levin
@ 2026-08-31 13:28 ` Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-08-31 13:28 UTC (permalink / raw)
  To: patches, stable
  Cc: Yu Kuai, Jens Axboe, Sasha Levin, tj, josef, cgroups, linux-block,
	linux-kernel

From: Yu Kuai <yukuai@fygo.io>

[ Upstream commit 25656304dabd26198ec69460c594a19d086ef099 ]

blkcg_print_one_stat() will be called for each blkg:
- access blkg->iostat, which is freed from rcu callback
  blkg_free_workfn();
- access policy data from pd_stat_fn(), which is freed from
  pd_free_fn(), while pd_free_fn() can be called by removing blkcg or
  deactivating policy;

Take blkcg->lock while iterating so the blkgs stay online and both
blkg->iostat and policy data for activated policies stay valid.  Use
irq-safe locking because blkcg->lock can be nested under q->queue_lock,
which is used from IRQ completion paths.

Prepare to convert protecting blkgs from request_queue with mutex.

Signed-off-by: Yu Kuai <yukuai@fygo.io>
Link: https://patch.msgid.link/05799877e720dcd300e2ddd4625e8e162959d7cc.1780621988.git.yukuai@fygo.io
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Subject Line
**Record:** `[blk-cgroup]` `[protect]` — Protect iteration over blkgs in
`blkcg_print_stat()` using `blkcg->lock`.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Yu Kuai `<yukuai@fygo.io>` (author)
- **Link:** https://patch.msgid.link/05799877e720dcd300e2ddd4625e8e16295
  9d7cc.1780621988.git.yukuai@fygo.io
- **Signed-off-by:** Jens Axboe `<axboe@kernel.dk>` (block maintainer)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Cc:
  stable@vger.kernel.org`

Notable: maintainer sign-off from Jens Axboe; no syzbot/fuzzer report.

### Step 1.3: Body Analysis
**Record:**
- **Bug:** `blkcg_print_one_stat()` reads `blkg->iostat` (freed in
  `blkg_free_workfn()`) and policy data via `pd_stat_fn()` (freed in
  `pd_free_fn()` during cgroup removal or policy deactivation).
- **Symptom:** Use-after-free when reading cgroup I/O stats concurrently
  with teardown/deactivation.
- **Root cause:** Iteration is RCU-protected and per-blkg `queue_lock`
  is held, but neither prevents `pd_free_fn()` or async `blkg` teardown
  from invalidating data being read.
- **Fix:** Hold `blkcg->lock` (IRQ-safe) for the full iteration so blkgs
  stay online and policy/iostat data remain valid.
- **Note:** "Prepare to convert protecting blkgs from request_queue with
  mutex" — future work, not a dependency.

### Step 1.4: Hidden Bug Fix?
**Record:** Yes — explicit UAF/race fix disguised as locking correction.
Not cosmetic cleanup.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- **File:** `block/blk-cgroup.c` (+3 / -6 net)
- **Function:** `blkcg_print_stat()` only
- **Scope:** Single-file, surgical (~10 lines touched)

### Step 2.2: Code Flow Change
**Record:**
- **Before:** `rcu_read_lock()` → `hlist_for_each_entry_rcu()` → per-
  iteration `spin_lock_irq(&blkg->q->queue_lock)` →
  `blkcg_print_one_stat()` → unlock → `rcu_read_unlock()`
- **After:** `guard(spinlock_irq)(&blkcg->lock)` →
  `hlist_for_each_entry()` → `blkcg_print_one_stat()` → auto-unlock
- **Path:** `cgroup` `io.stat` seq_file read (normal monitoring path)

### Step 2.3: Bug Mechanism
**Record:** **Category:** Race condition / use-after-free (reference-
counting and lifetime)

**Mechanism (verified in tree):**
1. `blkcg_deactivate_policy()` holds `queue_lock`, then `blkcg->lock`,
   then calls `pd_free_fn()`:

```1738:1744:block/blk-cgroup.c
                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;
```

2. `blkg_destroy()` requires `blkcg->lock`, unhashes the blkg, and
   eventually frees via `blkg_free_workfn()`:

```529:554:block/blk-cgroup.c
        lockdep_assert_held(&blkg->q->queue_lock);
        lockdep_assert_held(&blkcg->lock);
        // ...
        hlist_del_init_rcu(&blkg->blkcg_node);
```

3. `blkcg_print_stat()` currently does **not** hold `blkcg->lock`, so
   `pd_stat_fn()` and `blkg->iostat` access can race with steps 1–2.

4. The kernel already documents that RCU alone is insufficient:

```177:184:block/blk-cgroup.c
 - A group is RCU protected, but having an rcu lock does not mean that
   one
 - can access all the fields of blkg and assume these are valid.
```

### Step 2.4: Fix Quality
**Record:** Obviously correct. Aligns `blkcg_print_stat()` with
`blkcg_reset_stats()`, which already iterates `blkg_list` under
`spin_lock_irq(&blkcg->lock)`:

```662:669:block/blk-cgroup.c
        spin_lock_irq(&blkcg->lock);
        // ...
        hlist_for_each_entry(blkg, &blkcg->blkg_list, blkcg_node) {
```

**Regression risk:** Low. `blkcg_print_stat()` takes only `blkcg->lock`
(no `queue_lock`), avoiding AB-BA with `blkcg_destroy_blkgs()` (blkcg
lock → queue lock) and `blkcg_deactivate_policy()` (queue lock → blkcg
lock).

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:** Current RCU+`queue_lock` pattern in `blkcg_print_stat()`
from commit `49cb5168a7c6ab` (Aug 2021, "blk-cgroup: refactor
blkcg_print_stat"). Bug window is long; code is present in this tree.

### Step 3.2: Fixes: Tag
**Record:** N/A — no `Fixes:` tag.

### Step 3.3: Related File History
**Record:**
- `5e5b7f2ef8549` — separate UAF fix in `__blkcg_rstat_flush()` (already
  in 6.18.y); same subsystem, different race.
- `5d726c4dbeedd` — Yu Kuai deadlock fix in policy configuration (same
  author/subsystem).
- `810ecfa765f8b` (2013) — historical move from `blkcg->lock` to
  `queue_lock` for `blkcg_print_blkgs()`; this patch partially reverses
  that for `blkcg_print_stat()` where `queue_lock` is insufficient.

### Step 3.4: Author Context
**Record:** Yu Kuai is an active block/cgroup contributor
(`5d726c4dbeedd`, `dc96cefef0d30`, etc.).

### Step 3.5: Dependencies
**Record:** Standalone. `guard(spinlock_irq)` is defined in
`include/linux/spinlock.h` (available in 6.18). No series dependency.

---

## Phase 4: Mailing List and External Research

### Step 4.1–4.5
**Record:**
- `b4 dig -c <commit>`: **N/A** — commit not found in local `FETCH_HEAD`
  master; patch appears not yet merged upstream.
- Lore/patch.msgid.link: **Blocked** (403/Anubis bot protection).
- **UNVERIFIED:** Full review-thread content, stable nominations from
  reviewers, series revisions.

From available metadata: Jens Axboe merged sign-off indicates maintainer
acceptance.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `blkcg_print_stat()`, `blkcg_print_one_stat()` (caller
context unchanged).

### Step 5.2: Callers
**Record:** `blkcg_print_stat` is `.seq_show` for cgroup `stat` file:

```1254:1258:block/blk-cgroup.c
static struct cftype blkcg_files[] = {
        {
                .name = "stat",
                .seq_show = blkcg_print_stat,
```

Triggered via cgroupfs reads (`/sys/fs/cgroup/.../io.stat`).

### Step 5.3: Callees
**Record:** `blkcg_print_one_stat()` reads `blkg->iostat`, calls
`pol->pd_stat_fn()`, uses `blkg_dev_name()`.

### Step 5.4: Reachability
**Record:** Reachable from userspace via cgroup stat reads. Concurrent
with cgroup deletion (`blkcg_destroy_blkgs`) and policy deactivation
(`blkcg_deactivate_policy`) in container/VM environments.

### Step 5.5: Similar Patterns
**Record:** `blkcg_print_blkgs()` still uses RCU+`queue_lock` (lines
718–724) — same class of issue may exist there, but is out of scope for
this commit. `blkcg_reset_stats()` already uses the correct
`blkcg->lock` pattern.

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Buggy Code Present?
**Record:** **Yes.** Tree is **v6.18.44** (`linux-6.18.y` stable). Buggy
code at lines 1244–1250:

```1244:1250:block/blk-cgroup.c
        rcu_read_lock();
        hlist_for_each_entry_rcu(blkg, &blkcg->blkg_list, blkcg_node) {
                spin_lock_irq(&blkg->q->queue_lock);
                blkcg_print_one_stat(blkg, sf);
                spin_unlock_irq(&blkg->q->queue_lock);
        }
        rcu_read_unlock();
```

### Step 6.2: Backport Complications
**Record:** Clean apply expected — hunk matches current file.
`blkcg->lock` exists in `struct blkcg` (`blk-cgroup.h:96`).
`guard(spinlock_irq)` available via `spinlock.h` include chain.

### Step 6.3: Related Fixes Already Present?
**Record:** `5e5b7f2ef8549` (rstat flush UAF) is present; it does
**not** fix this `blkcg_print_stat()` race.

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem / Criticality
**Record:** **block / blk-cgroup** — **IMPORTANT** (cgroup I/O
accounting; widely used with containers/systemd/cgroup v2).

### Step 7.2: Activity
**Record:** Actively maintained; recent stable fixes in same file
(`5e5b7f2ef8549`, `6a01413a4e8fc`).

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Users with `CONFIG_BLK_CGROUP` reading I/O cgroup stats
while cgroups are deleted or policies deactivated — common in
Kubernetes/container teardown with concurrent monitoring.

### Step 8.2: Trigger Conditions
**Record:** Concurrent `io.stat` read + cgroup rmdir or block policy
deactivation/disk removal. Realistic in production; not purely
theoretical given `pd_free_fn()` runs under `blkcg->lock` that
`blkcg_print_stat()` does not take.

### Step 8.3: Failure Mode
**Record:** UAF on policy data and/or iostat → kernel oops, memory
corruption. **Severity: HIGH (potential CRITICAL security impact).**

### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** High — prevents UAF on a userspace-reachable path.
- **Risk:** Very low — ~7-line locking change matching existing
  `blkcg_reset_stats()` pattern.
- **Ratio:** Strongly favors backport.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR:**
- Real UAF race with verified mechanism in local tree
- Userspace-reachable via cgroup `stat` file
- Small, surgical, matches established locking in `blkcg_reset_stats()`
  / `blkcg_destroy_blkgs()`
- Block maintainer (Jens Axboe) sign-off
- Long-standing buggy pattern (since 2021 refactor)
- Related UAF fix already backported to this tree — demonstrates
  subsystem stability priority

**AGAINST:**
- Commit not yet verified as merged to mainline (stable rules require
  upstream merge before submission)
- `blkcg_print_blkgs()` may have a similar latent issue (not fixed here;
  not a reason to reject this fix)
- Mailing list review details unverified due to lore access blocks

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — locking matches sibling
   functions; maintainer SOB; mechanism clear from code.
2. Fixes a real bug affecting users? **PASS** — UAF on cgroup stat read
   path.
3. Important issue? **PASS** — UAF / potential crash or corruption (HIGH
   severity).
4. Small and contained? **PASS** — single function, <20 lines.
5. No new features or APIs? **PASS** — synchronization only.
6. Can apply to local tree? **PASS** — buggy code present; dependencies
   (`blkcg->lock`, `guard(spinlock_irq)`) exist in 6.18.44.

### Step 9.3: Exception Category
**Record:** N/A — standard bug fix, not device-ID/quirk/build/doc
exception.

### Step 9.4: Decision Rationale
This commit fixes a genuine use-after-free race in `blkcg_print_stat()`
when users read block cgroup statistics concurrently with cgroup
teardown or policy deactivation. The buggy code is present in the local
6.18.44 tree, the fix is minimal and mirrors the already-correct
`blkcg_reset_stats()` locking, and the failure mode is a kernel crash or
memory corruption. This is appropriate stable material once merged
upstream.

---

## Verification

- [Phase 1] Parsed subject, tags, body from user-provided commit message
- [Phase 1] Identified as explicit UAF/race fix, not cosmetic cleanup
- [Phase 2] Diff: 1 file, `blkcg_print_stat()` only, ~10 lines
- [Phase 2] Verified `pd_free_fn()` called under `blkcg->lock` in
  `blkcg_deactivate_policy()` (lines 1738–1744)
- [Phase 2] Verified `blkg_destroy()` requires `blkcg->lock` (lines
  529–530, 554)
- [Phase 2] Verified `blkg_free_workfn()` frees policy data and iostat
  (lines 114–142)
- [Phase 2] Verified RCU insufficiency comment (lines 177–184)
- [Phase 2] Verified `blkcg_reset_stats()` uses
  `spin_lock_irq(&blkcg->lock)` for same list iteration (lines 662–669)
- [Phase 3] `git blame`: RCU+queue_lock pattern from `49cb5168a7c6ab`
  (2021)
- [Phase 3] `git log -20 -- block/blk-cgroup.c`: related fixes
  `5e5b7f2ef8549`, `5d726c4dbeedd`
- [Phase 3] `git show 5e5b7f2ef8549`: separate rstat UAF fix, already in
  tree
- [Phase 3] No `Fixes:` tag to follow
- [Phase 3] `guard(spinlock_irq)` defined in `include/linux/spinlock.h`
  lines 571–573
- [Phase 4] `b4 dig`: N/A — commit not in `FETCH_HEAD` master
- [Phase 4] Lore/patch.msgid.link fetch: blocked (403/Anubis)
- [Phase 4] **UNVERIFIED:** Review-thread stable nominations, Tested-by
  reports
- [Phase 5] `blkcg_print_stat` registered as `.seq_show` for `"stat"`
  (lines 1254–1258)
- [Phase 5] `blkcg_print_one_stat` accesses `blkg->iostat` and
  `pd_stat_fn()` (lines 1182–1228)
- [Phase 6] `git describe HEAD`: v6.18.44 on `stable/linux-6.18.y`
- [Phase 6] Buggy code confirmed at lines 1244–1250
- [Phase 6] `struct blkcg` has `spinlock_t lock` in `blk-cgroup.h:96`
- [Phase 6] No equivalent fix already present for `blkcg_print_stat()`
- [Phase 7] Subsystem: block/blk-cgroup, CONFIG_BLK_CGROUP
- [Phase 8] Failure mode: UAF → oops/corruption, HIGH severity
- [Phase 8] Trigger: cgroup stat read during teardown/deactivation

**YES**

 block/blk-cgroup.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index a2347b5795874..812f60905a708 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1241,13 +1241,10 @@ static int blkcg_print_stat(struct seq_file *sf, void *v)
 	else
 		css_rstat_flush(&blkcg->css);
 
-	rcu_read_lock();
-	hlist_for_each_entry_rcu(blkg, &blkcg->blkg_list, blkcg_node) {
-		spin_lock_irq(&blkg->q->queue_lock);
+	guard(spinlock_irq)(&blkcg->lock);
+	hlist_for_each_entry(blkg, &blkcg->blkg_list, blkcg_node)
 		blkcg_print_one_stat(blkg, sf);
-		spin_unlock_irq(&blkg->q->queue_lock);
-	}
-	rcu_read_unlock();
+
 	return 0;
 }
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-31 13:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260831133314.4125787-1-sashal@kernel.org>
2026-08-31 13:25 ` [PATCH AUTOSEL 6.18-5.10] blk-cgroup: fix leaks and online flag on radix_tree_insert failure Sasha Levin
2026-08-31 13:28 ` [PATCH AUTOSEL 6.18-6.1] blk-cgroup: protect iterating blkgs with blkcg->lock in blkcg_print_stat() Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox