* [PATCH AUTOSEL 6.18-6.1] tracing: Disable KCOV instrumentation for trace_irqsoff.o
[not found] <20260831133314.4125787-1-sashal@kernel.org>
@ 2026-08-31 13:22 ` Sasha Levin
2026-08-31 13:24 ` [PATCH AUTOSEL 6.18] ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer Sasha Levin
` (3 subsequent siblings)
4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-08-31 13:22 UTC (permalink / raw)
To: patches, stable
Cc: Karl Mehltretter, Steven Rostedt, Sasha Levin, mhiramat,
linux-kernel, linux-trace-kernel
From: Karl Mehltretter <kmehltretter@gmail.com>
[ Upstream commit 01046072880b654dbadf71be2f645aad4a7b5d87 ]
When KCOV runs its boot selftest with whole-kernel instrumentation
enabled, it sets current->kcov_mode to KCOV_MODE_TRACE_PC without
installing a coverage area. Any instrumented code accepted as task-context
coverage in that window dereferences current->kcov_area and crashes.
On ARMv5 Versatile PB with CONFIG_KCOV_SELFTEST=y,
CONFIG_KCOV_INSTRUMENT_ALL=y and CONFIG_IRQSOFF_TRACER=y, boot hits a
NULL pointer fault during the selftest:
kcov: running self test
Internal error: Oops: 5 [#1] ARM
PC is at __sanitizer_cov_trace_pc+0x4c/0x90
Kernel panic - not syncing: Fatal exception
A diagnostic run showed the unwanted coverage comes from the IRQs-off
tracer callbacks reached from ARM IRQ entry before hardirq context is
visible to KCOV:
__sanitizer_cov_trace_pc from tracer_hardirqs_off+0x18/0x1cc
tracer_hardirqs_off from trace_hardirqs_off+0x34/0x54
trace_hardirqs_off from __irq_svc+0x58/0xb0
__irq_svc from kcov_init+0x7c/0xdc
and similarly through tracer_hardirqs_on().
trace_preemptirq.o is already excluded because this tracing path can run
from early interrupt code and produce coverage unrelated to syscall
inputs. Exclude trace_irqsoff.o as well, instead of requiring users to
turn off CONFIG_KCOV_INSTRUMENT_ALL=y, which is the default whole-kernel
KCOV mode.
With the exclusion in place, the same ARMv5 Versatile PB QEMU test boots
through the KCOV selftest and reaches userspace.
Tested on ARMv5 Versatile PB QEMU with CONFIG_KCOV_SELFTEST=y,
CONFIG_KCOV_INSTRUMENT_ALL=y and CONFIG_IRQSOFF_TRACER=y.
Link: https://patch.msgid.link/20260525170428.67211-1-kmehltretter@gmail.com
Assisted-by: Codex:gpt-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
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:** `[tracing] [disable] Disable KCOV instrumentation for
trace_irqsoff.o` — tracing subsystem; disables KCOV instrumentation on
one object file to prevent a boot crash.
### Step 1.2: Tags
**Record:**
- **Link:**
https://patch.msgid.link/20260525170428.67211-1-kmehltretter@gmail.com
- **Assisted-by:** Codex:gpt-5
- **Signed-off-by:** Karl Mehltretter `<kmehltretter@gmail.com>`
- **Signed-off-by:** Steven Rostedt `<rostedt@goodmis.org>` (tracing
maintainer)
- No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, or Cc:
stable tags
- Notable: maintainer sign-off; tested configuration described in body;
no syzbot report
### Step 1.3: Body analysis
**Record:**
- **Bug:** With `CONFIG_KCOV_SELFTEST=y`,
`CONFIG_KCOV_INSTRUMENT_ALL=y`, and `CONFIG_IRQSOFF_TRACER=y`, KCOV
boot selftest sets `current->kcov_mode = KCOV_MODE_TRACE_PC` without
installing a coverage area. Instrumented IRQ-off tracer code reached
from ARM IRQ entry can call `__sanitizer_cov_trace_pc()`, which
dereferences NULL `current->kcov_area`.
- **Symptom:** NULL pointer fault / kernel panic during boot on ARMv5
Versatile PB QEMU: `PC is at __sanitizer_cov_trace_pc`, panic during
`kcov: running self test`.
- **Root cause:** `trace_irqsoff.o` is still KCOV-instrumented while
`trace_preemptirq.o` was already excluded; `tracer_hardirqs_off()` /
`tracer_hardirqs_on()` live in `trace_irqsoff.c` and run from IRQ
entry before hardirq context is visible to KCOV’s `in_task()`
filtering.
- **Stack trace (from message):** `__sanitizer_cov_trace_pc` ←
`tracer_hardirqs_off` ← `trace_hardirqs_off` ← `__irq_svc` ←
`kcov_init`.
### Step 1.4: Hidden bug fix?
**Record:** Yes. Although framed as instrumentation exclusion, this is a
real boot crash fix for a valid Kconfig combination, completing the same
class of fix already applied to `trace_preemptirq.o` in commit
`bb5eb8f3b3297` (2022).
---
## Phase 2: Diff Analysis
### Step 2.1: Inventory
**Record:**
- **Files:** `kernel/trace/Makefile` only (+3/-2 lines net)
- **Change:** Add `KCOV_INSTRUMENT_trace_irqsoff.o := n`; broaden
comment from “this file” to “these files”
- **Scope:** Single-file, surgical Makefile change
### Step 2.2: Code flow
**Record:**
- **Before:** Only `trace_preemptirq.o` excluded from KCOV;
`trace_irqsoff.o` remains instrumented when
`CONFIG_KCOV_INSTRUMENT_ALL=y`.
- **After:** Both `trace_preemptirq.o` and `trace_irqsoff.o` excluded.
- **Affected path:** IRQ entry/exit →
`trace_hardirqs_off()`/`trace_hardirqs_on()` (uninstrumented wrapper
in `trace_preemptirq.c`) →
`tracer_hardirqs_off()`/`tracer_hardirqs_on()` (instrumented
implementation in `trace_irqsoff.c`) → `__sanitizer_cov_trace_pc()`.
### Step 2.3: Bug mechanism
**Record:** **Category:** NULL pointer dereference / boot crash in debug
instrumentation path.
- `kcov_init()` selftest sets `kcov_mode` without `kcov_area` (verified
in `kernel/kcov.c:1097-1098`).
- `check_kcov_mode()` allows tracing when `in_task()` is true
(`kernel/kcov.c:183-184`); during early ARM IRQ handling,
`preempt_count` may not yet reflect hardirq context
(`include/linux/preempt.h:130`).
- Instrumented `tracer_hardirqs_*()` in `trace_irqsoff.o` calls
`__sanitizer_cov_trace_pc()` which dereferences `t->kcov_area` at line
220 without NULL guard.
### Step 2.4: Fix quality
**Record:** Obviously correct; mirrors existing
`KCOV_INSTRUMENT_trace_preemptirq.o := n` pattern from `bb5eb8f3b3297`.
Minimal, no API changes. Regression risk very low — only disables KCOV
on code that should never contribute syscall-relevant coverage anyway.
---
## Phase 3: Git History Investigation
### Step 3.1: Blame
**Record:** Lines 34-36 in `kernel/trace/Makefile` introduced by
`bb5eb8f3b3297` (Congyu Liu, 2022-05-23) — preemptirq exclusion only.
`trace_irqsoff.c` has existed since `81d68a96a3984` (2008). The gap
(preemptirq excluded, irqsoff not) has been present since the 2022 fix.
### Step 3.2: Fixes: tag
**Record:** Not applicable — no Fixes: tag in commit message.
### Step 3.3: Related file history
**Record:** Recent `trace_irqsoff.c` changes are unrelated tracer fixes
(e.g. `c834a97962c70`). No prerequisite series; standalone one-line
Makefile fix. The candidate commit is not yet in this tree (fix absent
from current `kernel/trace/Makefile`).
### Step 3.4: Author context
**Record:** Steven Rostedt signed off (tracing maintainer). Prior
related fix `bb5eb8f3b3297` was Acked-by Dmitry Vyukov (KCOV author).
Karl Mehltretter appears to be reporting/fixing a gap in the 2022
exclusion.
### Step 3.5: Dependencies
**Record:** No dependencies. Uses standard `KCOV_INSTRUMENT_<stem>.o`
mechanism in `scripts/Makefile.lib:84-90`. Applies standalone.
---
## Phase 4: Mailing List and External Research
### Step 4.1: Original discussion
**Record:** UNVERIFIED — `b4 shazam` could not find the message-id;
lore.kernel.org returns 403/bot protection via WebFetch and curl. Link
from commit message could not be fetched.
### Step 4.2: Reviewers
**Record:** UNVERIFIED via b4 dig -w (could not locate thread). Steven
Rostedt SOB confirms maintainer acceptance.
### Step 4.3: Bug report
**Record:** Reproduced by author on ARMv5 Versatile PB QEMU with
documented Kconfig. No syzbot/bugzilla link. Severity: boot panic during
KCOV selftest.
### Step 4.4: Related patches
**Record:** Direct predecessor: `bb5eb8f3b3297 tracing: Disable kcov on
trace_preemptirq.c` — same rationale, incomplete because tracer
implementation lives in separate `trace_irqsoff.o`.
### Step 4.5: Stable list history
**Record:** UNVERIFIED — could not search lore stable archive due to
access restrictions.
---
## Phase 5: Code Semantic Analysis
### Step 5.1: Key functions
**Record:** `tracer_hardirqs_off()`, `tracer_hardirqs_on()` in
`trace_irqsoff.c`; `trace_hardirqs_off()`, `trace_hardirqs_on()` in
`trace_preemptirq.c`; `__sanitizer_cov_trace_pc()`, `check_kcov_mode()`,
`selftest()`, `kcov_init()` in `kcov.c`.
### Step 5.2: Callers
**Record:** `trace_hardirqs_off`/`on` called from ARM entry code
(`arch/arm/kernel/entry-header.S:208,215,217` via `svc_exit`;
`arch/arm/kernel/entry-armv.S:210`). Exported symbols used broadly
across architectures for IRQ flag tracing. `kcov_init` is
`device_initcall` — runs during boot with interrupts enabled.
### Step 5.3: Callees
**Record:** `tracer_hardirqs_off()` → `start_critical_timing()`; KCOV-
instrumented code in `trace_irqsoff.o` inserts calls to
`__sanitizer_cov_trace_pc()`.
### Step 5.4: Reachability
**Record:** Triggered during boot when KCOV selftest runs
(`device_initcall`) and a timer interrupt fires during the 300ms wait
loop. Reachable with `CONFIG_KCOV=y`, `CONFIG_KCOV_SELFTEST=y`,
`CONFIG_KCOV_INSTRUMENT_ALL=y` (default **y** per
`lib/Kconfig.debug:2149`), and `CONFIG_IRQSOFF_TRACER=y` or
`CONFIG_PREEMPT_TRACER=y` (both build `trace_irqsoff.o`).
### Step 5.5: Similar patterns
**Record:** Extensive KCOV exclusions across tree (mm/, lib/, kernel/,
arch/*/entry/, `kernel/trace/Makefile` for preemptirq). This fix fills
an obvious gap in the same file/pattern.
---
## Phase 6: Cross-Reference Against Local Tree
### Step 6.1: Buggy code present?
**Record:** **Yes.** Local tree is **Linux 6.18.44** (`git describe
HEAD` → `v6.18.44`). `kernel/trace/Makefile` has only:
```34:36:kernel/trace/Makefile
# Functions in this file could be invoked from early interrupt
# code and produce random code coverage.
KCOV_INSTRUMENT_trace_preemptirq.o := n
```
`trace_irqsoff.o` is built (lines 62-63) but not excluded. All
KCOV/selftest infrastructure present (`lib/Kconfig.debug`,
`kernel/kcov.c`).
### Step 6.2: Backport complications
**Record:** Clean apply expected — exact context matches the proposed
diff. No conflicting changes in this area.
### Step 6.3: Related fixes already present?
**Record:** Partial fix `bb5eb8f3b3297` is present (preemptirq only).
This specific irqsoff exclusion is **not** present. No duplicate fix
found.
---
## Phase 7: Subsystem and Maintainer Context
### Step 7.1: Subsystem criticality
**Record:** **tracing + KCOV** — IMPORTANT for kernel developers,
fuzzers (syzkaller per `KCOV_INSTRUMENT_ALL` help text), and CI systems
using KCOV selftest. Not universal production path, but KCOV is
security-relevant infrastructure.
### Step 7.2: Subsystem activity
**Record:** tracing subsystem actively maintained in 6.18.y (recent
irqsoff tracer fixes in 2025).
---
## Phase 8: Impact and Risk Assessment
### Step 8.1: Who is affected
**Record:** Config-specific — users/distributions building with KCOV
enabled, especially `KCOV_SELFTEST` (Kconfig says “Recommended to be
enabled”) and whole-kernel instrumentation (`KCOV_INSTRUMENT_ALL`,
default y). Affects fuzzing/CI/boot-validation workflows, not typical
end-user kernels without KCOV.
### Step 8.2: Trigger conditions
**Record:** Boot-time, with KCOV selftest + instrument-all +
irqsoff/preempt tracer enabled; timer interrupt during selftest wait
loop. Not userspace-triggerable on production kernels without KCOV, but
reliably reproducible in the described test config.
### Step 8.3: Failure mode severity
**Record:** NULL pointer dereference → kernel oops → **panic during
boot**. Severity: **CRITICAL** for affected configurations (system does
not reach userspace).
### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** Prevents deterministic boot panic for valid, documented
Kconfig used in fuzzing/CI; completes incomplete 2022 fix.
- **Risk:** Very low — one Makefile line, established exclusion
mechanism, no behavior change except suppressing inappropriate KCOV
callbacks.
- **Ratio:** Favorable for backport.
---
## Phase 9: Final Synthesis
### Step 9.1: Evidence summary
**FOR backport:**
- Real boot crash (NULL deref in `__sanitizer_cov_trace_pc`)
- Small, surgical, obviously correct fix following existing pattern
- Bug exists in 6.18.44 tree; fix not yet applied
- `KCOV_INSTRUMENT_ALL` defaults to y; `KCOV_SELFTEST` recommended in
Kconfig
- Maintainer (Rostedt) signed off
- Completes gap left by `bb5eb8f3b3297`
**AGAINST backport:**
- Only affects KCOV debug/fuzzing configurations, not typical production
kernels
- No syzbot/CVE report
- Mailing list review details unverified
**Unresolved:** Full lore thread content; whether reviewers explicitly
nominated for stable.
### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — author tested on ARMv5 QEMU;
mechanism verified in code; mirrors prior accepted fix.
2. Fixes a real bug? **PASS** — boot panic with documented Kconfig.
3. Important issue? **PASS** — boot crash (CRITICAL for affected
config); KCOV is fuzzing/security infrastructure.
4. Small and contained? **PASS** — 1 line + comment, single Makefile.
5. No new features/APIs? **PASS** — build-time instrumentation exclusion
only.
6. Can apply to local tree? **PASS** — code present, clean apply
expected.
### Step 9.3: Exception categories
**Record:** Build/instrumentation fix for valid kernel configuration —
analogous to existing KCOV exclusion pattern (similar to build-fix
category for instrumented builds).
### Step 9.4: Decision rationale
For **Linux 6.18.44**, this commit should be backported. The tree has
the incomplete 2022 preemptirq KCOV exclusion but not the irqsoff one,
leaving `tracer_hardirqs_*()` instrumented. Combined with KCOV selftest
(which deliberately enables tracing without a coverage area), this
causes a deterministic boot panic on interrupt during `kcov_init()`. The
fix is minimal, follows an established in-tree pattern, is signed by the
tracing maintainer, and has very low regression risk while preventing a
hard boot failure for KCOV/fuzzing configurations that stable trees are
expected to support.
---
## Verification
- [Phase 1] Parsed subject, tags, body; identified boot panic bug and
maintainer SOB
- [Phase 2] Diff: 1 Makefile line added; verified call chain preemptirq
wrapper → irqsoff tracer → `__sanitizer_cov_trace_pc`
- [Phase 3] `git describe HEAD`: v6.18.44 / 6.18.44
- [Phase 3] `git blame kernel/trace/Makefile:34-36`: `bb5eb8f3b3297`
(2022-05-23)
- [Phase 3] `git show bb5eb8f3b3297`: prior preemptirq-only exclusion
confirmed
- [Phase 3] `trace_irqsoff.c` exists since 2008 (`81d68a96a3984`)
- [Phase 3] Fix commit not in tree; `KCOV_INSTRUMENT_trace_irqsoff.o`
absent
- [Phase 4] UNVERIFIED: lore/b4 thread inaccessible (403/bot protection,
message-id not found)
- [Phase 5] Read `kernel/kcov.c:174-220,1080-1132`: selftest sets mode
without area; `check_kcov_mode` + NULL `kcov_area` deref confirmed
- [Phase 5] Read `kernel/trace/trace_irqsoff.c:643-647`,
`trace_preemptirq.c:102-109`: tracer in irqsoff.o, wrapper in
preemptirq.o
- [Phase 5] Read `arch/arm/kernel/entry-header.S:202-218`:
`trace_hardirqs_off/on` called from `svc_exit` during IRQ handling
- [Phase 5] Read `include/linux/preempt.h:130`: `in_task()` based on
preempt_count hardirq bits
- [Phase 5] Read `scripts/Makefile.lib:84-90`:
`KCOV_INSTRUMENT_<stem>.o` mechanism confirmed
- [Phase 6] Current `kernel/trace/Makefile` missing irqsoff exclusion;
`trace_irqsoff.o` built at lines 62-63
- [Phase 6] `lib/Kconfig.debug:2146-2172`: `KCOV_INSTRUMENT_ALL` default
y; `KCOV_SELFTEST` recommended
- [Phase 8] Failure mode: boot panic, severity CRITICAL for affected
Kconfig
**YES**
kernel/trace/Makefile | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index dcb4e02afc5f4..36b3ed5251f46 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -31,9 +31,10 @@ ifdef CONFIG_GCOV_PROFILE_FTRACE
GCOV_PROFILE := y
endif
-# Functions in this file could be invoked from early interrupt
-# code and produce random code coverage.
+# Functions in these files can run from IRQ entry before hardirq context
+# is visible to KCOV, and produce coverage unrelated to syscall inputs.
KCOV_INSTRUMENT_trace_preemptirq.o := n
+KCOV_INSTRUMENT_trace_irqsoff.o := n
CFLAGS_bpf_trace.o := -I$(src)
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH AUTOSEL 6.18] ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer
[not found] <20260831133314.4125787-1-sashal@kernel.org>
2026-08-31 13:22 ` [PATCH AUTOSEL 6.18-6.1] tracing: Disable KCOV instrumentation for trace_irqsoff.o Sasha Levin
@ 2026-08-31 13:24 ` Sasha Levin
2026-08-31 13:25 ` [PATCH AUTOSEL 6.18-5.10] perf/ftrace: Fix WARNING in __unregister_ftrace_function Sasha Levin
` (2 subsequent siblings)
4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-08-31 13:24 UTC (permalink / raw)
To: patches, stable
Cc: Masami Hiramatsu (Google), Steven Rostedt, Sasha Levin,
linux-kernel, linux-trace-kernel
From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
[ Upstream commit eb3bd277b37cd435d26a44a40d8f7c87ff16feb6 ]
Skip invalid sub-buffers when validating the persistent ring buffer
instead of discarding the entire ring buffer. Only skipped buffers
are invalidated (cleared).
If the cache data in memory fails to be synchronized during a reboot,
the persistent ring buffer may become partially corrupted, but other
sub-buffers may still contain readable event data. Only discard the
subbuffers that are found to be corrupted.
Link: https://lore.kernel.org/all/20260520185018.051228084@kernel.org/
Link: https://patch.msgid.link/20260522171050.914418536@kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
[SDR: Fixed max_loops in rb_iter_peek() as well ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
# Stable Backport Analysis: ring-buffer persistent validation fix
**Local tree:** `v6.18.44` (Linux 6.18.44)
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Subject line
**Record:** `[ring-buffer]` `[skip]` — Skip invalid sub-buffers when
validating persistent ring buffer during boot recovery.
### Step 1.2: Tags
**Record:**
- `Link:`
https://lore.kernel.org/all/20260520185018.051228084@kernel.org/
- `Link:` https://patch.msgid.link/20260522171050.914418536@kernel.org
- `Signed-off-by:` Masami Hiramatsu (Google) <mhiramat@kernel.org>
- `Signed-off-by:` Steven Rostedt <rostedt@goodmis.org> (with note:
"Fixed max_loops in rb_iter_peek() as well")
- No `Fixes:`, `Reported-by:`, `Cc: stable`, `Tested-by:`, or `Reviewed-
by:` tags
- Notable: subsystem maintainer (Rostedt) amended and signed off
### Step 1.3: Body analysis
**Record:**
- **Bug:** After reboot, persistent ring buffer validation treats any
single corrupted sub-buffer as fatal and discards the entire buffer.
- **Symptom:** Valid trace events from previous boot (especially post-
crash traces) are lost when only some sub-buffers are bad.
- **Root cause:** Cache may not fully sync across reboot; partial
corruption is realistic. Current code in `rb_cpu_meta_valid()` and
`rb_meta_validate_events()` rejects the whole buffer on first bad sub-
buffer.
- **Version info:** None in message.
### Step 1.4: Hidden bug fix detection
**Record:** Yes — described as validation improvement, but it fixes real
data-loss and reader-loop failures (`RB_WARN_ON` when >3 empty pages
after recovery).
---
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory
**Record:**
- **File:** `kernel/trace/ring_buffer.c` only (~73 insertions, ~47
deletions)
- **Functions modified:** `rb_cpu_meta_valid()`, `rb_validate_buffer()`,
`rb_meta_validate_events()`, `rb_get_reader_page()`, `rb_iter_peek()`;
relocates `rb_page_size()` earlier
- **Scope:** Single-file surgical fix in persistent ring-buffer recovery
path
### Step 2.2: Code flow changes
**Record:**
| Hunk | Before → After |
|------|----------------|
| `rb_cpu_meta_valid()` | Rejects entire meta if any subbuf `commit >
PAGE_SIZE` → only validates meta array structure; adds `subbuf_size !=
PAGE_SIZE` check |
| `rb_validate_buffer()` | Uses raw `commit` → masks `RB_MISSED_MASK`,
bounds-checks against `meta->subbuf_size` |
| `rb_meta_validate_events()` | `goto invalid` on first bad page → clear
only that sub-buffer, continue; track `discarded` count |
| `rb_get_reader_page()` / `rb_iter_peek()` | `max_loops = 3` hardcoded
→ `max_loops = nr_pages` for persistent buffers |
| `rb_page_size()` | Moved earlier so validation code can use masked
commit size |
### Step 2.3: Bug mechanism
**Record:**
- **Category:** Logic/correctness fix + secondary loop-limit bug
- **Mechanism:** Overly aggressive all-or-nothing validation discards
recoverable trace data; after per-page discard, multiple consecutive
empty pages exceed the hardcoded loop limit of 3, triggering
`RB_WARN_ON` and breaking trace reads
### Step 2.4: Fix quality
**Record:** Fix is minimal, obviously correct, and low regression risk.
Invalidates only proven-bad pages; still falls back to full discard on
structural failures (e.g., commit page not found). Rostedt's `max_loops`
addition addresses a real follow-on failure.
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame
**Record:**
- Aggressive per-subbuf commit check introduced in `c76883f18e59b7`
("ring-buffer: Add test if range of boot buffer is valid", 2024-06-14)
- `goto invalid` on bad buffer page in `rb_meta_validate_events()` dates
to `5f3b6e839f3ceb` (2024-06-12)
- Persistent ring buffer metadata added in `4009cc31e7813` (2025-03-05)
— present in this tree
### Step 3.2: Fixes: tag
**Record:** N/A — no `Fixes:` tag in commit message.
### Step 3.3: Related file history
**Record:** Multiple persistent ring-buffer fixes already in this 6.18.y
tree:
- `ca296d32ece38` — rewind persistent ring buffer on reboot
- `b6925774dd15d` — fix per-subbuf entries
- `2bc60c175568e` — flush/stop on panic
- This fix (`009124508d96f` / upstream `eb3bd277b37cd`) is **not** yet
in HEAD
### Step 3.4: Author context
**Record:** Masami Hiramatsu is the primary persistent ring-buffer
author; Steven Rostedt is trace/ring-buffer maintainer. Both have
multiple related commits in this file.
### Step 3.5: Dependencies
**Record:** Standalone for the validation path. Companion commit
`8a4563881fa3d` ("Skip invalid sub-buffers when rewinding persistent
ring buffer", patch 2/9 in v21 series) addresses a related rewind path
but is separate. This commit includes its own `max_loops` fixes and
applies independently.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
### Step 4.1: Original discussion
**Record:**
- `b4 dig -c 009124508d96f` matched: **[PATCH v21 1/9]** at
https://patch.msgid.link/20260522171050.914418536@kernel.org
- Lore URLs blocked by bot protection (Anubis) — could not read thread
content directly
### Step 4.2: Reviewers
**Record:** `b4 dig -w` CC list includes Steven Rostedt, linux-trace-
kernel, Mathieu Desnoyers, Mark Rutland, Andrew Morton — appropriate
maintainer/reviewer coverage.
### Step 4.3: Bug report
**Record:** No external bug report or syzbot link. Bug rationale is in
commit message (cache sync failure across reboot).
### Step 4.4: Series context
**Record:** Part of v21 9-patch series; this is patch 1/9. Companion
rewinding fix exists separately. Validation fix is self-contained.
### Step 4.5: Stable list history
**Record:** UNVERIFIED — could not search lore stable list due to bot
protection. However, multiple prior persistent ring-buffer fixes are
already present in this 6.18.y tree.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: Key functions
**Record:** `rb_cpu_meta_valid()`, `rb_validate_buffer()`,
`rb_meta_validate_events()`, `rb_get_reader_page()`, `rb_iter_peek()`
### Step 5.2: Callers
**Record:**
- `rb_meta_validate_events()` called from ring buffer CPU init at line
2399 during `range_addr_start` (persistent/reserved-memory) buffer
setup
- `rb_get_reader_page()` → `rb_buffer_peek()` → `ring_buffer_consume()`
/ trace reading paths
- `rb_iter_peek()` → `ring_buffer_iter_peek()` — non-consuming trace
reads
### Step 5.3: Callees
**Record:** `rb_read_data_buffer()`, `local_read/set`,
`rb_page_commit()`, `RB_WARN_ON()`, page list operations
### Step 5.4: Reachability
**Record:** Triggered at boot when persistent tracing instance is
configured via reserved memory / boot parameters (`trace.c` maps boot
instances via `reserve_mem_find_by_name()`). Affects crash/post-mortem
tracing users, not all kernels — but reachable on every boot for
configured systems.
### Step 5.5: Similar patterns
**Record:** Same "skip invalid sub-buffer instead of aborting" pattern
exists in companion rewinding patch (not yet in tree). Consistent with
incremental recovery approach used elsewhere in persistent ring-buffer
series.
---
## PHASE 6: CROSS-REFERENCE AGAINST LOCAL TREE (v6.18.44)
### Step 6.1: Buggy code present?
**Record:** **YES.** Current tree still has:
- `rb_cpu_meta_valid()` commit check at lines 1795–1797 that rejects
entire meta
- `rb_meta_validate_events()` `goto invalid` on first bad page at lines
2012–2016
- `rb_get_reader_page()` / `rb_iter_peek()` hardcoded `nr_loops > 3`
### Step 6.2: Backport complications
**Record:** Expected **clean apply**. `rb_page_size()` already exists at
line 3247; patch relocates it earlier (trivial). Function names match
(`rb_get_reader_page`, not `__rb_get_reader_page`).
### Step 6.3: Related fixes already present?
**Record:** Prior persistent ring-buffer fixes are in tree, but **not**
this validation-granularity fix or the companion rewinding fix.
---
## PHASE 7: SUBSYSTEM CONTEXT
### Step 7.1: Subsystem criticality
**Record:** `kernel/trace/` — IMPORTANT subsystem. Persistent ring
buffer is a debugging/crash-analysis feature, not core VFS/net, but
critical for post-crash trace retention.
### Step 7.2: Activity
**Record:** Actively developed — 10+ persistent ring-buffer commits in
recent `ring_buffer.c` history on this branch.
---
## PHASE 8: IMPACT AND RISK
### Step 8.1: Who is affected
**Record:** Users with persistent/reserved-memory tracing configured
(boot params, DT `reserve_mem`, crash analysis setups). Config-specific,
not universal.
### Step 8.2: Trigger conditions
**Record:** Reboot with partially unsynchronized persistent ring-buffer
memory (unclean shutdown, crash, power loss). Realistic for the
feature's intended use case.
### Step 8.3: Failure severity
**Record:**
- **Without fix:** Total loss of previous-boot trace data when any
single sub-buffer is corrupt; `RB_WARN_ON` / failed reads when
multiple cleared pages exist after recovery
- **Severity:** MEDIUM-HIGH for affected users (defeats purpose of
persistent tracing); LOW for systems without persistent tracing
### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** Recovers partial crash traces; prevents false
`RB_WARN_ON` on read path — high value for persistent tracing users
- **Risk:** Very low — only clears proven-invalid pages; structural
failures still trigger full discard
- **Ratio:** Favorable
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence summary
**FOR backport:**
- Real bug with verified presence in v6.18.44
- Causes loss of recoverable crash-trace data
- Secondary `RB_WARN_ON`/read failure after recovery
- Small, single-file, maintainer-reviewed fix
- Consistent with other persistent ring-buffer fixes already in this
stable line
- Applies cleanly to this tree
**AGAINST backport:**
- Niche feature (persistent tracing only)
- Not a general security/crash issue for all users
- Companion rewinding patch may also be desirable for complete coverage
**Unresolved:** Lore thread content and stable-list discussion (bot-
blocked).
### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — logic is clear; maintainer
signed off; no Tested-by but tracing selftest series exists
2. Fixes real bug affecting users? **PASS** — data loss on boot recovery
for persistent tracing
3. Important issue? **PASS** — trace data loss + `RB_WARN_ON` on read
path for crash-analysis users
4. Small and contained? **PASS** — ~120 lines, one file
5. No new features/APIs? **PASS**
6. Can apply to local tree? **PASS** — code exists, patch should apply
cleanly
### Step 9.3: Exception categories
**Record:** None (not a quirk/DT/build/docs fix — a correctness bug fix
in existing feature).
### Step 9.4: Decision rationale
This commit fixes a real correctness bug in the persistent ring buffer
recovery path present in Linux 6.18.44. When memory is partially
corrupted across reboot (the exact scenario persistent tracing is
designed for), the current code throws away **all** trace data because
of **one** bad sub-buffer. The fix surgically invalidates only corrupted
pages and adjusts reader/iterator loop limits so recovered buffers
remain readable. The change is small, reviewed by the trace maintainer,
and follows the same pattern as other persistent ring-buffer fixes
already accepted into this stable series.
**Note:** Consider also backporting the companion patch `ring-buffer:
Skip invalid sub-buffers when rewinding persistent ring buffer` for the
rewind path, but this validation commit is independently valuable.
---
## Verification
- [Phase 1] `git show 009124508d96f`: parsed subject, body, tags; no
Fixes/Reported-by/Cc:stable
- [Phase 2] `git show 009124508d96f -- kernel/trace/ring_buffer.c`: full
diff analyzed; 1 file, 5 functions
- [Phase 3] `git describe HEAD`: v6.18.44
- [Phase 3] `git merge-base --is-ancestor 009124508d96f HEAD`: exit 1 —
fix NOT in tree
- [Phase 3] `git merge-base --is-ancestor 4009cc31e7813 HEAD`: exit 0 —
persistent ring buffer feature IS in tree
- [Phase 3] `git blame -L 1795,1816,2012,2017
kernel/trace/ring_buffer.c`: buggy code from c76883f18e59b7 /
5f3b6e839f3ceb
- [Phase 3] `git log --oneline -20 -- kernel/trace/ring_buffer.c`:
multiple prior persistent ring-buffer fixes in tree
- [Phase 3] `git merge-base --is-ancestor 8a4563881fa3d HEAD`: exit 1 —
companion rewinding fix not in tree
- [Phase 4] `b4 dig -c 009124508d96f`: found [PATCH v21 1/9] at
patch.msgid.link/20260522171050.914418536@kernel.org
- [Phase 4] `b4 dig -c 009124508d96f -w`: CC list includes Rostedt,
Desnoyers, linux-trace-kernel
- [Phase 4] WebFetch lore/patch.msgid.link URLs: BLOCKED by Anubis bot
protection
- [Phase 5] Read `ring_buffer.c` lines 1754–2057, 2399, 3247, 5421–5765:
confirmed buggy code paths and call sites
- [Phase 5] Grep `rb_meta_validate_events`, `rb_get_reader_page`,
`rb_iter_peek`: traced callers
- [Phase 5] Read `trace.c` lines 11137–11165: persistent boot instance
via `reserve_mem_find_by_name()`
- [Phase 6] Grep `rb_validate_buffer(head_page->page, cpu_buffer->cpu)`
without meta param: confirms pre-fix code in tree
- [Phase 6] Grep `nr_loops > 3` in reader/iter paths: confirms hardcoded
limit in tree
- [Phase 8] Failure mode: total trace data discard + RB_WARN_ON on read;
severity MEDIUM-HIGH for persistent tracing users
**YES****Result:** Backport analysis for `ring-buffer: Skip invalid sub-
buffers when validating persistent ring buffer` on **v6.18.44** —
**YES**.
The fix belongs in this tree: the buggy all-or-nothing validation is
still present, persistent ring-buffer support is in-tree, and the patch
is a small, maintainer-reviewed correctness fix for crash-trace recovery
after reboot.
**Worth noting:** It’s patch **1/9** in the v21 series. The companion
**rewinding** fix (`Skip invalid sub-buffers when rewinding persistent
ring buffer`) isn’t in this tree yet and may be worth evaluating
separately for full coverage.
kernel/trace/ring_buffer.c | 120 ++++++++++++++++++++++---------------
1 file changed, 73 insertions(+), 47 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index fefa16e51e0ef..631d42281f5b3 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -396,6 +396,12 @@ static __always_inline unsigned int rb_page_commit(struct buffer_page *bpage)
return local_read(&bpage->page->commit);
}
+/* Size is determined by what has been committed */
+static __always_inline unsigned int rb_page_size(struct buffer_page *bpage)
+{
+ return rb_page_commit(bpage) & ~RB_MISSED_MASK;
+}
+
static void free_buffer_page(struct buffer_page *bpage)
{
/* Range pages are not to be freed */
@@ -1756,7 +1762,6 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
unsigned long *subbuf_mask)
{
int subbuf_size = PAGE_SIZE;
- struct buffer_data_page *subbuf;
unsigned long buffers_start;
unsigned long buffers_end;
int i;
@@ -1764,6 +1769,11 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
if (!subbuf_mask)
return false;
+ if (meta->subbuf_size != PAGE_SIZE) {
+ pr_info("Ring buffer boot meta [%d] invalid subbuf_size\n", cpu);
+ return false;
+ }
+
buffers_start = meta->first_buffer;
buffers_end = meta->first_buffer + (subbuf_size * meta->nr_subbufs);
@@ -1780,11 +1790,12 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
return false;
}
- subbuf = rb_subbufs_from_meta(meta);
-
bitmap_clear(subbuf_mask, 0, meta->nr_subbufs);
- /* Is the meta buffers and the subbufs themselves have correct data? */
+ /*
+ * Ensure the meta::buffers array has correct data. The data in each subbufs
+ * are checked later in rb_meta_validate_events().
+ */
for (i = 0; i < meta->nr_subbufs; i++) {
if (meta->buffers[i] < 0 ||
meta->buffers[i] >= meta->nr_subbufs) {
@@ -1792,18 +1803,12 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
return false;
}
- if ((unsigned)local_read(&subbuf->commit) > subbuf_size) {
- pr_info("Ring buffer boot meta [%d] buffer invalid commit\n", cpu);
- return false;
- }
-
if (test_bit(meta->buffers[i], subbuf_mask)) {
pr_info("Ring buffer boot meta [%d] array has duplicates\n", cpu);
return false;
}
set_bit(meta->buffers[i], subbuf_mask);
- subbuf = (void *)subbuf + subbuf_size;
}
return true;
@@ -1867,13 +1872,22 @@ static int rb_read_data_buffer(struct buffer_data_page *dpage, int tail, int cpu
return events;
}
-static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu)
+static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu,
+ struct ring_buffer_cpu_meta *meta)
{
unsigned long long ts;
+ unsigned long tail;
u64 delta;
- int tail;
- tail = local_read(&dpage->commit);
+ /*
+ * When a sub-buffer is recovered from a read, the commit value may
+ * have RB_MISSED_* bits set, as these bits are reset on reuse.
+ * Even after clearing these bits, a commit value greater than the
+ * subbuf_size is considered invalid.
+ */
+ tail = local_read(&dpage->commit) & ~RB_MISSED_MASK;
+ if (tail > meta->subbuf_size - BUF_PAGE_HDR_SIZE)
+ return -1;
return rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
}
@@ -1884,6 +1898,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
struct buffer_page *head_page, *orig_head, *orig_reader;
unsigned long entry_bytes = 0;
unsigned long entries = 0;
+ int discarded = 0;
int ret;
u64 ts;
int i;
@@ -1895,14 +1910,19 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
orig_reader = cpu_buffer->reader_page;
/* Do the reader page first */
- ret = rb_validate_buffer(orig_reader->page, cpu_buffer->cpu);
+ ret = rb_validate_buffer(orig_reader->page, cpu_buffer->cpu, meta);
if (ret < 0) {
- pr_info("Ring buffer reader page is invalid\n");
- goto invalid;
+ pr_info("Ring buffer meta [%d] invalid reader page detected\n",
+ cpu_buffer->cpu);
+ discarded++;
+ /* Instead of discard whole ring buffer, discard only this sub-buffer. */
+ local_set(&orig_reader->entries, 0);
+ local_set(&orig_reader->page->commit, 0);
+ } else {
+ entries += ret;
+ entry_bytes += rb_page_size(orig_reader);
+ local_set(&orig_reader->entries, ret);
}
- entries += ret;
- entry_bytes += local_read(&orig_reader->page->commit);
- local_set(&orig_reader->entries, ret);
ts = head_page->page->time_stamp;
@@ -1930,7 +1950,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
break;
/* Stop rewind if the page is invalid. */
- ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu);
+ ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu, meta);
if (ret < 0)
break;
@@ -1939,7 +1959,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
if (ret)
local_inc(&cpu_buffer->pages_touched);
entries += ret;
- entry_bytes += rb_page_commit(head_page);
+ entry_bytes += rb_page_size(head_page);
}
if (i)
pr_info("Ring buffer [%d] rewound %d pages\n", cpu_buffer->cpu, i);
@@ -2009,21 +2029,24 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
if (head_page == orig_reader)
continue;
- ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu);
+ ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu, meta);
if (ret < 0) {
- pr_info("Ring buffer meta [%d] invalid buffer page\n",
- cpu_buffer->cpu);
- goto invalid;
- }
-
- /* If the buffer has content, update pages_touched */
- if (ret)
- local_inc(&cpu_buffer->pages_touched);
-
- entries += ret;
- entry_bytes += local_read(&head_page->page->commit);
- local_set(&head_page->entries, ret);
+ if (!discarded)
+ pr_info("Ring buffer meta [%d] invalid buffer page detected\n",
+ cpu_buffer->cpu);
+ discarded++;
+ /* Instead of discard whole ring buffer, discard only this sub-buffer. */
+ local_set(&head_page->entries, 0);
+ local_set(&head_page->page->commit, 0);
+ } else {
+ /* If the buffer has content, update pages_touched */
+ if (ret)
+ local_inc(&cpu_buffer->pages_touched);
+ entries += ret;
+ entry_bytes += rb_page_size(head_page);
+ local_set(&head_page->entries, ret);
+ }
if (head_page == cpu_buffer->commit_page)
break;
}
@@ -2037,7 +2060,10 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
local_set(&cpu_buffer->entries, entries);
local_set(&cpu_buffer->entries_bytes, entry_bytes);
- pr_info("Ring buffer meta [%d] is from previous boot!\n", cpu_buffer->cpu);
+ pr_info("Ring buffer meta [%d] is from previous boot!", cpu_buffer->cpu);
+ if (discarded)
+ pr_cont(" (%d pages discarded)", discarded);
+ pr_cont("\n");
return;
invalid:
@@ -3243,12 +3269,6 @@ rb_iter_head_event(struct ring_buffer_iter *iter)
return NULL;
}
-/* Size is determined by what has been committed */
-static __always_inline unsigned rb_page_size(struct buffer_page *bpage)
-{
- return rb_page_commit(bpage) & ~RB_MISSED_MASK;
-}
-
static __always_inline unsigned
rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer)
{
@@ -5421,8 +5441,9 @@ rb_update_iter_read_stamp(struct ring_buffer_iter *iter,
static struct buffer_page *
rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
{
- struct buffer_page *reader = NULL;
+ int max_loops = cpu_buffer->ring_meta ? cpu_buffer->nr_pages : 3;
unsigned long bsize = READ_ONCE(cpu_buffer->buffer->subbuf_size);
+ struct buffer_page *reader = NULL;
unsigned long overwrite;
unsigned long flags;
int nr_loops = 0;
@@ -5434,11 +5455,14 @@ rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
again:
/*
* This should normally only loop twice. But because the
- * start of the reader inserts an empty page, it causes
- * a case where we will loop three times. There should be no
- * reason to loop four times (that I know of).
+ * start of the reader inserts an empty page, it causes a
+ * case where we will loop three times. There should be no
+ * reason to loop four times unless the ring buffer is a
+ * recovered persistent ring buffer. For persistent ring buffers,
+ * invalid pages are reset during recovery, so there may be more
+ * than 3 contiguous pages can be empty, but less than nr_pages.
*/
- if (RB_WARN_ON(cpu_buffer, ++nr_loops > 3)) {
+ if (RB_WARN_ON(cpu_buffer, ++nr_loops > max_loops)) {
reader = NULL;
goto out;
}
@@ -5728,12 +5752,14 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts)
struct ring_buffer_per_cpu *cpu_buffer;
struct ring_buffer_event *event;
int nr_loops = 0;
+ int max_loops;
if (ts)
*ts = 0;
cpu_buffer = iter->cpu_buffer;
buffer = cpu_buffer->buffer;
+ max_loops = cpu_buffer->ring_meta ? cpu_buffer->nr_pages : 3;
/*
* Check if someone performed a consuming read to the buffer
@@ -5756,7 +5782,7 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts)
* the ring buffer with an active write as the consumer is.
* Do not warn if the three failures is reached.
*/
- if (++nr_loops > 3)
+ if (++nr_loops > max_loops)
return NULL;
if (rb_per_cpu_empty(cpu_buffer))
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH AUTOSEL 6.18-5.10] perf/ftrace: Fix WARNING in __unregister_ftrace_function
[not found] <20260831133314.4125787-1-sashal@kernel.org>
2026-08-31 13:22 ` [PATCH AUTOSEL 6.18-6.1] tracing: Disable KCOV instrumentation for trace_irqsoff.o Sasha Levin
2026-08-31 13:24 ` [PATCH AUTOSEL 6.18] ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer Sasha Levin
@ 2026-08-31 13:25 ` Sasha Levin
2026-08-31 13:27 ` [PATCH AUTOSEL 6.18-5.10] sparc64: uprobes: add missing break Sasha Levin
2026-08-31 13:30 ` [PATCH AUTOSEL 6.18] ring-buffer: Skip invalid sub-buffers when rewinding persistent ring buffer Sasha Levin
4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-08-31 13:25 UTC (permalink / raw)
To: patches, stable
Cc: Rik van Riel, Steven Rostedt, Sasha Levin, mhiramat, linux-kernel,
linux-trace-kernel
From: Rik van Riel <riel@surriel.com>
[ Upstream commit 9581123304b23049437324038698af9fb56ee663 ]
perf_ftrace_function_unregister() unconditionally calls
unregister_ftrace_function() without checking whether the ftrace_ops
was ever successfully registered. This triggers a WARN_ON in
__unregister_ftrace_function() when the ops doesn't have
FTRACE_OPS_FL_ENABLED set.
This can happen during perf_event_alloc() error cleanup when
perf_trace_destroy() is called via __free_event() on an event whose
ftrace_ops registration failed or was already torn down by
perf_try_init_event()'s err_destroy path.
The call path is:
perf_event_alloc() error cleanup
-> __free_event()
-> event->destroy() [tp_perf_event_destroy]
-> perf_trace_destroy()
-> perf_trace_event_close()
-> TRACE_REG_PERF_CLOSE
-> perf_ftrace_function_unregister()
-> unregister_ftrace_function()
-> __unregister_ftrace_function()
-> WARN_ON(!(ops->flags & FTRACE_OPS_FL_ENABLED))
Fix this by checking FTRACE_OPS_FL_ENABLED before attempting to
unregister. If the ops is not enabled, just free the filter and
return success.
Link: https://patch.msgid.link/20260527111301.2d0d8256@fangorn
Signed-off-by: Rik van Riel <riel@surriel.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The follow-up check confirms the fix (`9581123304b23`) is on `master` —
it's the only commit adding the `FTRACE_OPS_FL_ENABLED` guard in
`perf_ftrace_function_unregister()`. It's not in **6.18.44**, so the
backport recommendation remains **YES**.
kernel/trace/trace_event_perf.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index a6bb7577e8c59..5b272856e5ab1 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -497,7 +497,17 @@ static int perf_ftrace_function_register(struct perf_event *event)
static int perf_ftrace_function_unregister(struct perf_event *event)
{
struct ftrace_ops *ops = &event->ftrace_ops;
- int ret = unregister_ftrace_function(ops);
+ int ret = 0;
+
+ /*
+ * Perf will call this unconditionally even if the ops is not
+ * enabled. The unregister_ftrace_function() will warn if called
+ * when not enabled. Just bypass the unregistering if ops isn't
+ * enabled here.
+ */
+ if (ops->flags & FTRACE_OPS_FL_ENABLED)
+ ret = unregister_ftrace_function(ops);
+
ftrace_free_filter(ops);
return ret;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH AUTOSEL 6.18-5.10] sparc64: uprobes: add missing break
[not found] <20260831133314.4125787-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-08-31 13:25 ` [PATCH AUTOSEL 6.18-5.10] perf/ftrace: Fix WARNING in __unregister_ftrace_function Sasha Levin
@ 2026-08-31 13:27 ` Sasha Levin
2026-08-31 13:30 ` [PATCH AUTOSEL 6.18] ring-buffer: Skip invalid sub-buffers when rewinding persistent ring buffer Sasha Levin
4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-08-31 13:27 UTC (permalink / raw)
To: patches, stable
Cc: Rosen Penev, Masami Hiramatsu (Google), Andreas Larsson,
Sasha Levin, oleg, peterz, davem, linux-kernel,
linux-trace-kernel, sparclinux
From: Rosen Penev <rosenp@gmail.com>
[ Upstream commit 5b0eee4cd812bd6547eea393cb9b5c0322f26c88 ]
Missing fallthrough causes failure with newer compilers:
arch/sparc/kernel/uprobes.c:284:2: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
284 | default:
| ^
arch/sparc/kernel/uprobes.c:284:2: note: insert 'break;' to avoid fall-through
284 | default:
| ^
| break;
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Reviewed-by: Andreas Larsson <andreas@gaisler.com>
Signed-off-by: Andreas Larsson <andreas@gaisler.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit Message Forensics
### Step 1.1: Parse the Subject Line
**Record:** `[sparc64/uprobes]` `[add]` — Add a missing `break`
statement in the sparc64 uprobes die-notifier switch.
### Step 1.2: Parse All Commit Message Tags
**Record:**
- **Signed-off-by:** Rosen Penev `<rosenp@gmail.com>` (author)
- **Reviewed-by:** Masami Hiramatsu (Google) `<mhiramat@kernel.org>`
(uprobes maintainer)
- **Reviewed-by:** Andreas Larsson `<andreas@gaisler.com>` (sparc
maintainer)
- **Signed-off-by:** Andreas Larsson `<andreas@gaisler.com>` (subsystem
maintainer ack)
- No Fixes:, Reported-by:, Link:, Cc: stable@vger.kernel.org, or Tested-
by: tags
- Notable: dual Reviewed-by from uprobes and sparc maintainers; no
syzbot or user bug reports
### Step 1.3: Analyze Commit Body
**Record:**
- **Bug description:** Missing `break` after `case DIE_SSTEP:` causes an
unannotated fall-through warning that becomes a build error under
`-Werror,-Wimplicit-fallthrough`.
- **Symptom:** sparc64 kernel compilation fails on
`arch/sparc/kernel/uprobes.c:284`.
- **Root cause:** `case DIE_SSTEP:` lacks `break;` before `default:`.
- **Version info:** None stated; failure is tied to newer compilers
enforcing `-Wimplicit-fallthrough`.
### Step 1.4: Detect Hidden Bug Fixes
**Record:** Not a hidden runtime bug fix. The fall-through lands on
`default: break;`, which is a no-op. This is a **build-fix / compiler-
warning fix**, not a functional correctness fix. No hidden UAF, race, or
logic error.
---
## Phase 2: Diff Analysis
### Step 2.1: Inventory the Changes
**Record:**
- **Files:** `arch/sparc/kernel/uprobes.c` (+1 line)
- **Function modified:** `arch_uprobe_exception_notify()`
- **Scope:** Single-file, single-line surgical fix
### Step 2.2: Code Flow Change
**Record:**
- **Before:** `case DIE_SSTEP:` could fall through to `default:`
(compiler warning/error).
- **After:** `case DIE_SSTEP:` ends with `break;`, matching `case
DIE_BPT:` and other architectures.
- **Path affected:** Die-notifier callback for uprobes on sparc64;
normal and error paths unchanged at runtime.
### Step 2.3: Bug Mechanism
**Record:**
- **Category:** Build fix (compiler `-Wimplicit-fallthrough` /
`-Werror`)
- **Mechanism:** Unannotated switch fall-through triggers a warning;
with `-Werror` it fails the build. No runtime behavior change because
`default:` only contains `break;`.
### Step 2.4: Fix Quality
**Record:**
- Obviously correct; matches powerpc/mips pattern.
- Minimal, zero regression risk.
- No API, locking, or logic changes.
---
## Phase 3: Git History Investigation
### Step 3.1: Blame Changed Lines
**Record:** Buggy code introduced in `e8f4aa6087fa8` ("sparc64:Support
User Probes for sparc", Oct 2016, Allen Pais). Present in this tree
since uprobes support landed on sparc64.
### Step 3.2: Follow Fixes: Tag
**Record:** No Fixes: tag. N/A.
### Step 3.3: File History
**Record:** Recent `arch/sparc/kernel/uprobes.c` history in 6.18.y:
- `a51b8c83bf274` sparc64: Fix prototype warning for uprobe_trap
- SPDX/treewide cleanups
- Original `e8f4aa6087fa8` uprobes introduction
Commit `5b0eee4cd812b` is on `master` but **not** in
`stable/linux-6.18.y`. Standalone one-patch fix.
### Step 3.4: Author's Other Commits
**Record:** Rosen Penev has no other sparc commits in this tree. Fix
came through sparc maintainer tree (Andreas Larsson).
### Step 3.5: Dependencies
**Record:** No prerequisites. Applies cleanly to current 6.18.y
`uprobes.c`. Self-contained.
---
## Phase 4: Mailing List and External Research
### Step 4.1: Original Patch Discussion
**Record:**
- **b4 dig URL:**
https://patch.msgid.link/20260506031815.779909-1-rosenp@gmail.com
- Single-patch submission, no series revisions (`b4 dig -a` not needed).
- No stable nomination or NAKs found in thread.
- Maintainer reviews present (Hiramatsu, Larsson).
### Step 4.2: Reviewers
**Record:** CC'd to `sparclinux@vger.kernel.org`, `linux-trace-
kernel@vger.kernel.org`, David S. Miller, Oleg Nesterov, Peter Zijlstra
— appropriate uprobes/sparc audience.
### Step 4.3: Bug Report
**Record:** No external bug report. Author demonstrated compiler error
in commit message and patch.
### Step 4.4: Related Patches
**Record:** Standalone. No series dependencies.
### Step 4.5: Stable Mailing List
**Record:** No stable-list discussion found. Not searched exhaustively;
no stable nomination in patch thread.
---
## Phase 5: Code Semantic Analysis
### Step 5.1: Key Functions
**Record:** `arch_uprobe_exception_notify()` — only function modified.
### Step 5.2: Callers
**Record:** Registered as die notifier in `kernel/events/uprobes.c`:
```2901:2913:kernel/events/uprobes.c
static struct notifier_block uprobe_exception_nb = {
.notifier_call = arch_uprobe_exception_notify,
.priority = INT_MAX-1, /* notified after
kprobes, kgdb */
};
void __init uprobes_init(void)
{
...
BUG_ON(register_die_notifier(&uprobe_exception_nb));
}
```
Called from kernel die-notification path on `DIE_BPT` / `DIE_SSTEP` when
uprobes are active.
### Step 5.3: Callees
**Record:** `user_mode()`, `uprobe_pre_sstep_notifier()`,
`uprobe_post_sstep_notifier()`.
### Step 5.4: Call Chain / Reachability
**Record:** Reachable when `CONFIG_UPROBES` is enabled and a userspace
breakpoint/single-step trap occurs. Not a syscall path;
debugging/tracing infrastructure. Build impact is unconditional when
`uprobes.c` is compiled.
### Step 5.5: Similar Patterns
**Record:** powerpc has the correct `break` after `DIE_SSTEP`:
```148:159:arch/powerpc/kernel/uprobes.c
switch (val) {
case DIE_BPT:
if (uprobe_pre_sstep_notifier(regs))
return NOTIFY_STOP;
break;
case DIE_SSTEP:
if (uprobe_post_sstep_notifier(regs))
return NOTIFY_STOP;
break;
default:
break;
}
```
sparc64 was missing the equivalent `break`. Prior sparc precedent:
`f6f8c1c09c224` ("sparc builds with -Werror") addressed similar fall-
through warnings.
---
## Phase 6: Cross-Reference Against Local Tree
### Step 6.1: Does Buggy Code Exist?
**Record:** **Yes.** Local tree is **v6.18.44** (`stable/linux-6.18.y`).
Buggy code at lines 280–285:
```280:286:arch/sparc/kernel/uprobes.c
case DIE_SSTEP:
if (uprobe_post_sstep_notifier(args->regs))
ret = NOTIFY_STOP;
default:
break;
```
Bug present since 2016 uprobes introduction.
### Step 6.2: Backport Complications
**Record:** Clean one-line apply expected. No conflicting changes in
this file on 6.18.y since the uprobes introduction.
### Step 6.3: Related Fixes Already Present?
**Record:** No equivalent fix in 6.18.y. `git merge-base --is-ancestor
5b0eee4cd812b HEAD` → NOT IN TREE.
---
## Phase 7: Subsystem and Maintainer Context
### Step 7.1: Subsystem Criticality
**Record:** **arch/sparc** uprobes — **PERIPHERAL** (sparc64 only,
tracing/debugging). Build impact matters for sparc64 builders.
### Step 7.2: Subsystem Activity
**Record:** Low churn on this file; last functional change was prototype
fix `a51b8c83bf274`.
---
## Phase 8: Impact and Risk Assessment
### Step 8.1: Who Is Affected
**Record:** sparc64 kernel **builders** with `CONFIG_UPROBES` enabled.
`sparc64_defconfig` has `CONFIG_UPROBE_EVENTS=y`, which selects
`UPROBES` via Kconfig. Not universal; sparc64-specific and config-
dependent.
### Step 8.2: Trigger Conditions
**Record:**
- `-Wimplicit-fallthrough` is enabled globally
(`scripts/Makefile.extrawarn:94` via
`CONFIG_CC_IMPLICIT_FALLTHROUGH`).
- Becomes a **build failure** with `-Werror` (`CONFIG_WERROR=y` or `make
W=e`).
- Default `sparc64_defconfig` does not set `CONFIG_WERROR`; typical
distro builds may see a warning only.
- Developers/CI using `-Werror` or `W=e` hit a hard failure.
- **No unprivileged runtime trigger**; no security impact.
### Step 8.3: Failure Mode Severity
**Record:**
- **Build failure** with `-Werror`: **MEDIUM** for
developers/maintainers.
- **Runtime:** **NONE** — fall-through to `default: break;` is
semantically identical.
- **Warning-only** builds: **LOW**.
### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** Unblocks sparc64 builds with WERROR/W=e and UPROBES;
aligns with other arch implementations; matches stable precedent for
`-Wimplicit-fallthrough` fixes (e.g. `3ab659eb696f0`, `98845e7753902`
in this tree).
- **Risk:** Very low — one `break;` with no behavioral change.
- **Ratio:** Favorable for a build-fix backport, though end-user runtime
benefit is zero.
---
## Phase 9: Final Synthesis
### Step 9.1: Evidence Summary
**FOR backport:**
- Valid **build fix** (stable exception category)
- Demonstrated `-Werror` compilation failure
- `uprobes.c` compiled in default sparc64 defconfig (`UPROBE_EVENTS=y` →
`UPROBES`)
- One-line, maintainer-reviewed, obviously correct
- Bug present since 2016; applies cleanly to 6.18.y
- Precedent for similar implicit-fallthrough fixes in stable
**AGAINST backport:**
- **No runtime bug** — purely compiler hygiene
- sparc64 is niche; limited user base
- Default production builds may only warn, not fail
- Low severity for running systems
**Unresolved:** Whether any sparc64 distro enables `CONFIG_WERROR` by
default (not verified; not needed for decision).
### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — trivial fix; reviewed by
uprobes and sparc maintainers.
2. Fixes a real bug? **PASS** — real build failure with `-Werror`.
3. Important issue? **PASS (build-fix category)** — not
crash/corruption, but prevents compilation for sparc64+UPROBES+WERROR
builds.
4. Small and contained? **PASS** — 1 line, 1 file.
5. No new features/APIs? **PASS**.
6. Can apply to local tree? **PASS** — buggy code confirmed in v6.18.44;
patch applies cleanly.
### Step 9.3: Exception Category
**Record:** **Build fix** — prevents compilation failure under
`-Wimplicit-fallthrough` + `-Werror`.
### Step 9.4: Decision Rationale
This commit fixes a sparc64 build break, not a runtime defect. The
missing `break` is harmless at runtime because execution falls through
to `default: break;`, but it violates `-Wimplicit-fallthrough` and fails
builds that use `-Werror` — a configuration kernel developers and CI
commonly use. With `UPROBE_EVENTS=y` in `sparc64_defconfig`, `uprobes.c`
is compiled by default on sparc64. The fix is one line, matches powerpc
and other architectures, is maintainer-reviewed, and fits the stable
exception for build fixes. Risk is negligible.
---
## Verification
- **[Phase 1]** `git show 5b0eee4cd812b`: parsed subject, body, tags
- **[Phase 2]** `git show 5b0eee4cd812b -p`: confirmed +1 `break;` in
`arch_uprobe_exception_notify()`
- **[Phase 2]** Read `arch/sparc/kernel/uprobes.c:274-286`: confirmed
missing `break` in 6.18.y
- **[Phase 2]** Read `arch/powerpc/kernel/uprobes.c:148-159`: confirmed
correct pattern on other arch
- **[Phase 3]** `git blame -L 274,286 arch/sparc/kernel/uprobes.c`: bug
since `e8f4aa6087fa8` (2016)
- **[Phase 3]** `git merge-base --is-ancestor 5b0eee4cd812b HEAD`: NOT
IN TREE
- **[Phase 3]** `git log --oneline -20 -- arch/sparc/kernel/uprobes.c`:
file history verified
- **[Phase 4]** `b4 dig -c 5b0eee4cd812b`: lore URL found
- **[Phase 4]** `b4 dig -c 5b0eee4cd812b -w`: recipients and reviewers
verified
- **[Phase 4]** `b4 dig -m /tmp/uprobes_break.mbox`: thread saved; no
stable nomination found
- **[Phase 5]** `grep arch_uprobe_exception_notify`: callers in
`kernel/events/uprobes.c` verified
- **[Phase 6]** `git describe HEAD`: v6.18.44 on `stable/linux-6.18.y`
- **[Phase 6]** Read `arch/sparc/configs/sparc64_defconfig`:
`CONFIG_UPROBE_EVENTS=y` at line 204
- **[Phase 6]** `kernel/trace/Kconfig:778-783`: `UPROBE_EVENTS` selects
`UPROBES`
- **[Phase 6]** `arch/sparc/Kconfig:150-151`: `ARCH_SUPPORTS_UPROBES`
for SPARC64
- **[Phase 6]** `init/Kconfig:939-942`: `CC_IMPLICIT_FALLTHROUGH`
enabled by default
- **[Phase 6]** `scripts/Makefile.extrawarn:94,217-219`: global
`-Wimplicit-fallthrough`; `-Werror` with `CONFIG_WERROR` or `W=e`
- **[Phase 6]** `init/Kconfig:227-229`: `CONFIG_WERROR` defaults to
`COMPILE_TEST` (typically off in defconfig)
- **[Phase 7]** `git show f6f8c1c09c224`: prior sparc
`-Werror`/fallthrough precedent
- **[Phase 8]** Runtime analysis: fall-through to `default:break` is a
no-op — **no runtime bug**
- **[Phase 8]** `git log --oneline --grep='missing break'`: similar
implicit-fallthrough fixes exist in tree history
**YES**
arch/sparc/kernel/uprobes.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/sparc/kernel/uprobes.c b/arch/sparc/kernel/uprobes.c
index 305017bec164f..c8cac64e9988a 100644
--- a/arch/sparc/kernel/uprobes.c
+++ b/arch/sparc/kernel/uprobes.c
@@ -280,6 +280,7 @@ int arch_uprobe_exception_notify(struct notifier_block *self,
case DIE_SSTEP:
if (uprobe_post_sstep_notifier(args->regs))
ret = NOTIFY_STOP;
+ break;
default:
break;
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH AUTOSEL 6.18] ring-buffer: Skip invalid sub-buffers when rewinding persistent ring buffer
[not found] <20260831133314.4125787-1-sashal@kernel.org>
` (3 preceding siblings ...)
2026-08-31 13:27 ` [PATCH AUTOSEL 6.18-5.10] sparc64: uprobes: add missing break Sasha Levin
@ 2026-08-31 13:30 ` Sasha Levin
4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-08-31 13:30 UTC (permalink / raw)
To: patches, stable
Cc: Masami Hiramatsu (Google), Steven Rostedt, Sasha Levin,
linux-kernel, linux-trace-kernel
From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
[ Upstream commit c8a7d4b4723a21e7464efe86dcf80627e0b4df33 ]
Skip invalid sub-buffers when rewinding the persistent ring buffer
instead of stopping the rewinding the ring buffer. The skipped
buffers are cleared.
To ensure the rewinding stops at the unused page, this also clears
buffer_data_page::time_stamp when tracing resets the buffer. This
allows us to identify unused pages and empty pages.
Link: https://patch.msgid.link/20260522171051.091265852@kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
[ SDR: Have reader_page still get evaluated if header_page fails ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
# Stable Backport Analysis: `ring-buffer: Skip invalid sub-buffers when
rewinding persistent ring buffer`
**Local tree:** `v6.18.44` (Linux 6.18.44, `stable/linux-6.18.y`)
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Subject line
**Record:** `[ring-buffer]` `[skip]` — When rewinding a persistent ring
buffer across reboot, skip (and clear) invalid sub-buffers instead of
stopping the rewind early.
### Step 1.2: Tags
**Record:**
- **Link:** https://patch.msgid.link/20260522171051.091265852@kernel.org
- **Signed-off-by:** Masami Hiramatsu (Google), Steven Rostedt
- **No** Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, or
Cc: stable in the commit message
- **Notable:** Patch 2/9 in the v21 series “Making persistent ring
buffers robust”; Steven Rostedt note: “Have reader_page still get
evaluated if header_page fails”
### Step 1.3: Body analysis
**Record:**
- **Bug:** During persistent ring-buffer rewind after reboot, hitting an
invalid sub-buffer stops the rewind loop.
- **Symptom:** Valid older trace data in earlier sub-buffers is not
recovered; only data up to the first invalid page is kept.
- **Root cause (author):** Rewind treated invalid pages as a hard stop;
unused pages were not reliably identifiable because `time_stamp` was
not cleared on buffer reset.
- **Fix approach:** Continue rewinding past invalid pages (clearing
them), use timestamp boundaries for validation, and clear
`buffer_data_page::time_stamp` in `rb_init_page()`.
### Step 1.4: Hidden bug fix?
**Record:** Yes — described as robustness, but it fixes incorrect
recovery behavior: premature rewind termination and loss of readable
trace events after partial buffer corruption (e.g. unsynchronized cache
across reboot).
---
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory
**Record:**
- **File:** `kernel/trace/ring_buffer.c` only (~69 insertions, ~38
deletions)
- **Functions modified:** `rb_init_page()`, `rb_validate_buffer()`,
`rb_meta_validate_events()`
- **Scope:** Single-file, focused change in persistent-buffer
validation/rewind path
### Step 2.2: Code flow changes
**Record:**
| Hunk | Before | After |
|------|--------|-------|
| `rb_init_page()` | Clears only `commit` | Also clears `time_stamp` |
| `rb_validate_buffer()` | Validates commit size + event walk | Adds
`prev_ts`/`next_ts` bounds; clears invalid pages; sets `entries` |
| Rewind loop | `break` on invalid page | Skip invalid pages, increment
`discarded`, continue |
| Head page handling | Reader validated first | Head validated first;
rewind skipped only if head invalid |
| Unused-page detection | Timestamp/commit heuristics | Stop when
`!time_stamp && commit == 0` |
| Invalid path cleanup | Manual commit clears | Uses `rb_init_page()` |
### Step 2.3: Bug mechanism
**Record:** **Logic/correctness fix** in persistent ring-buffer
recovery. Invalid sub-buffer during rewind caused early loop exit
(`break`), leaving older valid pages unrecovered. Timestamp clearing and
boundary checks improve detection of unused/corrupt pages.
### Step 2.4: Fix quality
**Record:** Fix is logically sound and minimal for its scope. Low
regression risk: only affects persistent ring-buffer recovery at boot.
Timestamp clearing on reset is consistent with identifying unused pages.
**Caveat:** Builds on refactored `rb_validate_buffer()` from
prerequisite commit `eb3bd277b37cd`.
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame
**Record:** Rewind loop with `break` on invalid page introduced by
`ca296d32ece38` (“tracing: ring_buffer: Rewind persistent ring buffer on
reboot”, 2025-06-04). That commit **is in this tree** (6.18.y). Base
validation infrastructure from `5f3b6e839f3ce` (“Validate boot range
memory events”, 2024-06-12), also in tree.
### Step 3.2: Fixes: tag
**Record:** N/A — no Fixes: tag.
### Step 3.3: Related file history
**Record:** Stable 6.18.y already has persistent ring-buffer fixes:
- `ca296d32ece38` — rewind on reboot (introduces bug)
- `b6925774dd15d` — per-subbuf entries fix (backported)
- `2bc60c175568e` — flush on panic (backported)
- `6b4bf6519e507` — reader_page double-count fix (backported)
**Missing from stable (on master):**
- `eb3bd277b37cd` — skip invalid sub-buffers when **validating** (patch
1/9, prerequisite)
- `c8a7d4b4723a2` — this commit (patch 2/9)
Part of v21 series “Making persistent ring buffers robust”; not
standalone.
### Step 3.4: Author context
**Record:** Masami Hiramatsu and Steven Rostedt are tracing/ring-buffer
maintainers. Multiple related persistent ring-buffer commits in the same
timeframe.
### Step 3.5: Dependencies
**Record:**
- **Requires** `eb3bd277b37cd` (changes `rb_validate_buffer()` signature
and per-sub-buffer discard logic).
- `c8a7d4b4723a2` alone: **merge conflict** on stable.
- `eb3bd277b37cd` then `c8a7d4b4723a2`: **both apply cleanly** (verified
via cherry-pick).
- Does not require later series patches (tests, display, cleanup) for
core fix.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
### Step 4.1: Original discussion
**Record:**
- **b4 dig URL:**
https://patch.msgid.link/20260522171051.091265852@kernel.org
- **Series:** v19 → v20 → v21; committed version matches v21 patch 2/9
- **Cover letter (v21 0/9):** “make the persistent ring buffer more
robust when sub-buffers are detected to be corrupted. Instead of
invalidating the entire buffer, just invalidate the individual sub-
buffers.”
### Step 4.2: Reviewers
**Record:** CC’d: linux-kernel, linux-trace-kernel, Masami Hiramatsu,
Mark Rutland, Mathieu Desnoyers, Andrew Morton, Ian Rogers. Reviewed-by
Masami Hiramatsu on a related thread message in mbox.
### Step 4.3: Bug report
**Record:** No syzbot/user bug report. Issue inferred from persistent-
buffer corruption scenario in companion patch `eb3bd277b37cd`: “cache
data in memory fails to be synchronized during a reboot.”
### Step 4.4: Related patches
**Record:** Same series includes validation skip (`eb3bd`), inject test,
dropped-events display, validation cleanup. Validation + rewind skip are
the core functional pair.
### Step 4.5: Stable list history
**Record:** No Cc: stable or stable-list discussion found in retrieved
mbox thread.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: Key functions
**Record:** `rb_init_page()`, `rb_validate_buffer()`,
`rb_meta_validate_events()`
### Step 5.2: Callers
**Record:** `rb_meta_validate_events()` called from
`rb_allocate_cpu_buffer()` at line 2399 during CPU ring-buffer
allocation when persistent/range-mapped meta is present.
### Step 5.3: Callees
**Record:** `rb_read_data_buffer()`, `rb_page_commit()`,
`rb_page_size()`, `rb_dec_page()`/`rb_inc_page()`, `local_set()`.
### Step 5.4: Reachability
**Record:** Triggered at **boot** during ring-buffer init for
**reserve_mem / range-mapped persistent** trace buffers with valid meta
from a previous boot. Not a syscall hot path; requires tracing admin
setup. Not unprivileged.
### Step 5.5: Similar patterns
**Record:** Stable tree still has related bugs:
- Rewind: `break` on invalid page (lines 1932–1935)
- Validation: `goto invalid` resets entire buffer on any invalid page
(lines 1899–1901, 2013–2016)
`eb3bd277b37cd` addresses validation; this commit addresses rewind.
---
## PHASE 6: CROSS-REFERENCE WITH LOCAL TREE (6.18.44)
### Step 6.1: Buggy code present?
**Record:** **Yes.** Current stable code at lines 1932–1935:
```1932:1935:kernel/trace/ring_buffer.c
/* Stop rewind if the page is invalid. */
ret = rb_validate_buffer(head_page->page,
cpu_buffer->cpu);
if (ret < 0)
break;
```
Bug introduced with rewind feature in `ca296d32ece38`, which is in
6.18.y.
### Step 6.2: Backport complications
**Record:** This commit alone does **not** apply cleanly (content
conflict). With prerequisite `eb3bd277b37cd` first, both apply cleanly.
Expect minor context differences vs mainline but no structural blocker.
### Step 6.3: Related fixes already present?
**Record:** No equivalent skip-on-invalid rewind or per-sub-buffer
validation discard logic in 6.18.44.
---
## PHASE 7: SUBSYSTEM CONTEXT
### Step 7.1: Subsystem criticality
**Record:** **kernel/trace** (ring buffer) — **IMPORTANT** for
tracing/debugging; not core MM/net, but persistent buffers target
crash/post-mortem analysis.
### Step 7.2: Activity
**Record:** Actively maintained; several persistent ring-buffer fixes
already backported to 6.18.y.
---
## PHASE 8: IMPACT AND RISK
### Step 8.1: Who is affected
**Record:** Users of **persistent/reserve_mem ring buffers** who reboot
after unclean shutdown or partial memory corruption. Config-specific,
admin-configured.
### Step 8.2: Trigger conditions
**Record:** Boot with persistent trace buffer meta from previous boot +
at least one invalid sub-buffer during rewind. Most likely after
crash/unclean reboot with unsynced buffer memory. Uncommon but realistic
for the feature’s purpose.
### Step 8.3: Failure severity
**Record:** **Loss of recoverable trace events** (diagnostic data), not
kernel crash/panic/deadlock. Severity: **MEDIUM** for persistent-tracing
users; **LOW** for others.
### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** MEDIUM — restores trace data that would otherwise be
discarded during rewind
- **Risk:** LOW — boot-time recovery path only; small, maintainer-
authored change
- **Ratio:** Favorable when paired with `eb3bd277b37cd`
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence summary
**FOR backport:**
- Real bug in shipped 6.18.y feature (persistent ring-buffer rewind)
- Loses valid trace data on partial corruption — the scenario persistent
buffers are meant to survive
- Small, surgical, maintainer-reviewed
- Applies cleanly with prerequisite `eb3bd277b37cd` (also clean)
- Related persistent ring-buffer fixes already in 6.18.y
- Part of intentional robustness series
**AGAINST backport:**
- Not a kernel crash/security/deadlock
- Niche feature (reserve_mem persistent tracing)
- Requires prerequisite commit not yet in stable
- No explicit Cc: stable or user/syzbot report
- Patch 2/9 of a series
**Unresolved:** No runtime test results in commit message; no explicit
stable nomination.
### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** (logic clear; maintainer
authorship; no test tag)
2. Fixes real bug affecting users? **PASS** (persistent tracing users
lose recoverable data)
3. Important issue? **PASS (borderline)** — diagnostic data loss in
crash-recovery path, not kernel instability
4. Small and contained? **PASS** (~107 lines, one file)
5. No new features/APIs? **PASS** (fixes recovery behavior)
6. Can apply to local tree? **PASS with dependency** — needs
`eb3bd277b37cd` first; then applies cleanly
### Step 9.3: Exception categories
**Record:** None (not device ID, quirk, DT, build, or docs).
### Step 9.4: Decision rationale
This commit fixes a real correctness bug in persistent ring-buffer
rewind that exists in 6.18.44. When an invalid sub-buffer is encountered
during rewind after reboot, the current code stops early and fails to
recover older valid trace pages. That undermines the purpose of
persistent tracing across reboots, especially after crashes with partial
memory corruption.
The fix is small, maintainer-authored, and backports cleanly together
with its immediate prerequisite `eb3bd277b37cd` (which fixes the
analogous “discard everything” problem in the validation path). Stable
6.18.y already carries the rewind feature and other persistent ring-
buffer fixes; this pair completes that robustness work.
**Backport recommendation:** Apply `eb3bd277b37cd` first, then
`c8a7d4b4723a2`.
---
## Verification
- [Phase 1] `git show c8a7d4b4723a2 -s`: parsed subject, body, Link,
Signed-off-by tags
- [Phase 1] No Fixes:/Reported-by:/Cc: stable in commit message
- [Phase 2] `git show c8a7d4b4723a2 --stat`: 1 file, +69/-38 lines
- [Phase 3] `git describe HEAD`: v6.18.44
- [Phase 3] `git blame -L 1930,1936 kernel/trace/ring_buffer.c`: rewind
logic from `ca296d32ece38`
- [Phase 3] `git merge-base --is-ancestor ca296d32ece38 HEAD`: rewind
commit in tree
- [Phase 3] `git log stable/linux-6.18.y --grep='Skip invalid sub-
buffers'`: no matches — fix not in stable
- [Phase 3] `git log stable/linux-6.18.y..master --
kernel/trace/ring_buffer.c | grep -iE 'invalid|rewind'`: both commits
missing from stable
- [Phase 4] `b4 dig -c c8a7d4b4723a2`: lore URL found
- [Phase 4] `b4 dig -c c8a7d4b4723a2 -a`: v19/v20/v21 series revisions
- [Phase 4] `b4 dig -c c8a7d4b4723a2 -w`: maintainers CC’d
- [Phase 4] `grep` on `/tmp/ringbuf_thread.mbox`: cover letter describes
corruption robustness; no Cc: stable
- [Phase 5] `grep rb_meta_validate_events`: caller at line 2399 in
`rb_allocate_cpu_buffer()`
- [Phase 6] Read `kernel/trace/ring_buffer.c` lines 1870–2057: buggy
rewind `break` confirmed
- [Phase 6] `git cherry-pick --no-commit c8a7d4b4723a2`: **CONFLICT**
- [Phase 6] `git cherry-pick --no-commit eb3bd277b37cd && git cherry-
pick --no-commit c8a7d4b4723a2`: **both succeeded** (exit 0)
- [Phase 6] `git reset --hard HEAD`: tree restored to v6.18.44
- [Phase 7] `git log --oneline -10 -- kernel/trace/ring_buffer.c`:
active persistent ring-buffer maintenance
- [Phase 8] Failure mode verified from code: premature rewind stop →
trace data not recovered; severity MEDIUM
**YES**
kernel/trace/ring_buffer.c | 107 ++++++++++++++++++++++++-------------
1 file changed, 69 insertions(+), 38 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 631d42281f5b3..cb35aef4c47bd 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -389,6 +389,7 @@ struct buffer_page {
static void rb_init_page(struct buffer_data_page *bpage)
{
local_set(&bpage->commit, 0);
+ bpage->time_stamp = 0;
}
static __always_inline unsigned int rb_page_commit(struct buffer_page *bpage)
@@ -1872,12 +1873,14 @@ static int rb_read_data_buffer(struct buffer_data_page *dpage, int tail, int cpu
return events;
}
-static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu,
- struct ring_buffer_cpu_meta *meta)
+static int rb_validate_buffer(struct buffer_page *bpage, int cpu,
+ struct ring_buffer_cpu_meta *meta, u64 prev_ts, u64 next_ts)
{
+ struct buffer_data_page *dpage = bpage->page;
unsigned long long ts;
unsigned long tail;
u64 delta;
+ int ret;
/*
* When a sub-buffer is recovered from a read, the commit value may
@@ -1886,9 +1889,27 @@ static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu,
* subbuf_size is considered invalid.
*/
tail = local_read(&dpage->commit) & ~RB_MISSED_MASK;
- if (tail > meta->subbuf_size - BUF_PAGE_HDR_SIZE)
- return -1;
- return rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
+ if (tail <= meta->subbuf_size - BUF_PAGE_HDR_SIZE)
+ ret = rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
+ else
+ ret = -1;
+
+ /*
+ * The timestamp must be greater than @prev_ts and smaller than @next_ts.
+ * Since this function works in both forward (verify) and reverse (unwind)
+ * loop, we don't know both @prev_ts and @next_ts at the same time.
+ * So use the known boundary as the boundary.
+ */
+ if (ret < 0 || (prev_ts && prev_ts > ts) || (next_ts && ts > next_ts)) {
+ local_set(&bpage->entries, 0);
+ local_set(&dpage->commit, 0);
+ dpage->time_stamp = prev_ts ? prev_ts : next_ts;
+ ret = -1;
+ } else {
+ local_set(&bpage->entries, ret);
+ }
+
+ return ret;
}
/* If the meta data has been validated, now validate the events */
@@ -1899,6 +1920,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
unsigned long entry_bytes = 0;
unsigned long entries = 0;
int discarded = 0;
+ bool skip = false;
int ret;
u64 ts;
int i;
@@ -1909,25 +1931,35 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
orig_head = head_page = cpu_buffer->head_page;
orig_reader = cpu_buffer->reader_page;
- /* Do the reader page first */
- ret = rb_validate_buffer(orig_reader->page, cpu_buffer->cpu, meta);
+ /* Do the head page first */
+ ret = rb_validate_buffer(head_page, cpu_buffer->cpu, meta, 0, 0);
+ if (ret < 0) {
+ pr_info("Ring buffer meta [%d] invalid head page detected\n",
+ cpu_buffer->cpu);
+ /* Don't bother rewinding */
+ skip = true;
+ ts = 0;
+ } else {
+ ts = head_page->page->time_stamp;
+ }
+
+ /* Do the reader page - reader must be previous to head. */
+ ret = rb_validate_buffer(orig_reader, cpu_buffer->cpu, meta, 0, ts);
if (ret < 0) {
pr_info("Ring buffer meta [%d] invalid reader page detected\n",
cpu_buffer->cpu);
discarded++;
- /* Instead of discard whole ring buffer, discard only this sub-buffer. */
- local_set(&orig_reader->entries, 0);
- local_set(&orig_reader->page->commit, 0);
} else {
entries += ret;
entry_bytes += rb_page_size(orig_reader);
- local_set(&orig_reader->entries, ret);
+ ts = orig_reader->page->time_stamp;
}
- ts = head_page->page->time_stamp;
+ if (skip)
+ goto skip_rewind;
/*
- * Try to rewind the head so that we can read the pages which already
+ * Try to rewind the head so that we can read the pages which are already
* read in the previous boot.
*/
if (head_page == cpu_buffer->tail_page)
@@ -1940,26 +1972,27 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
if (head_page == cpu_buffer->tail_page)
break;
- /* Ensure the page has older data than head. */
- if (ts < head_page->page->time_stamp)
+ /* Rewind until unused page (no timestamp, no commit). */
+ if (!head_page->page->time_stamp && rb_page_commit(head_page) == 0)
break;
- ts = head_page->page->time_stamp;
- /* Ensure the page has correct timestamp and some data. */
- if (!ts || rb_page_commit(head_page) == 0)
- break;
-
- /* Stop rewind if the page is invalid. */
- ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu, meta);
- if (ret < 0)
- break;
-
- /* Recover the number of entries and update stats. */
- local_set(&head_page->entries, ret);
- if (ret)
- local_inc(&cpu_buffer->pages_touched);
- entries += ret;
- entry_bytes += rb_page_size(head_page);
+ /*
+ * Skip if the page is invalid, or its timestamp is newer than the
+ * previous valid page.
+ */
+ ret = rb_validate_buffer(head_page, cpu_buffer->cpu, meta, 0, ts);
+ if (ret < 0) {
+ if (!discarded)
+ pr_info("Ring buffer meta [%d] invalid buffer page detected\n",
+ cpu_buffer->cpu);
+ discarded++;
+ } else {
+ entries += ret;
+ entry_bytes += rb_page_size(head_page);
+ if (ret > 0)
+ local_inc(&cpu_buffer->pages_touched);
+ ts = head_page->page->time_stamp;
+ }
}
if (i)
pr_info("Ring buffer [%d] rewound %d pages\n", cpu_buffer->cpu, i);
@@ -2021,6 +2054,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
/* Nothing more to do, the only page is the reader page */
goto done;
}
+ ts = head_page->page->time_stamp;
/* Iterate until finding the commit page */
for (i = 0; i < meta->nr_subbufs + 1; i++, rb_inc_page(&head_page)) {
@@ -2029,15 +2063,12 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
if (head_page == orig_reader)
continue;
- ret = rb_validate_buffer(head_page->page, cpu_buffer->cpu, meta);
+ ret = rb_validate_buffer(head_page, cpu_buffer->cpu, meta, ts, 0);
if (ret < 0) {
if (!discarded)
pr_info("Ring buffer meta [%d] invalid buffer page detected\n",
cpu_buffer->cpu);
discarded++;
- /* Instead of discard whole ring buffer, discard only this sub-buffer. */
- local_set(&head_page->entries, 0);
- local_set(&head_page->page->commit, 0);
} else {
/* If the buffer has content, update pages_touched */
if (ret)
@@ -2045,7 +2076,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
entries += ret;
entry_bytes += rb_page_size(head_page);
- local_set(&head_page->entries, ret);
+ ts = head_page->page->time_stamp;
}
if (head_page == cpu_buffer->commit_page)
break;
@@ -2073,12 +2104,12 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
/* Reset the reader page */
local_set(&cpu_buffer->reader_page->entries, 0);
- local_set(&cpu_buffer->reader_page->page->commit, 0);
+ rb_init_page(cpu_buffer->reader_page->page);
/* Reset all the subbuffers */
for (i = 0; i < meta->nr_subbufs - 1; i++, rb_inc_page(&head_page)) {
local_set(&head_page->entries, 0);
- local_set(&head_page->page->commit, 0);
+ rb_init_page(head_page->page);
}
}
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread