public inbox for linux-sound@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.18] ALSA: hda/realtek: add quirk for Framework F111:000F
       [not found] <20260420131539.986432-1-sashal@kernel.org>
@ 2026-04-20 13:08 ` Sasha Levin
  2026-04-20 13:08 ` [PATCH AUTOSEL 7.0-5.15] ALSA: pcm: Serialize snd_pcm_suspend_all() with open_mutex Sasha Levin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
  To: patches, stable
  Cc: Dustin L. Howett, Takashi Iwai, Sasha Levin, perex, tiwai,
	linux-sound, linux-kernel

From: "Dustin L. Howett" <dustin@howett.net>

[ Upstream commit bac1e57adf08c9ee33e95fb09cd032f330294e70 ]

Similar to commit 7b509910b3ad ("ALSA hda/realtek: Add quirk for
Framework F111:000C") and previous quirks for Framework systems with
Realtek codecs.

000F is another new platform with an ALC285 which needs the same quirk.

Signed-off-by: Dustin L. Howett <dustin@howett.net>
Link: https://patch.msgid.link/20260327-framework-alsa-000f-v1-1-74013aba1c00@howett.net
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Error: Failed to generate final synthesis

 sound/hda/codecs/realtek/alc269.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/hda/codecs/realtek/alc269.c b/sound/hda/codecs/realtek/alc269.c
index 0c975005793e7..e7f7b148b40e5 100644
--- a/sound/hda/codecs/realtek/alc269.c
+++ b/sound/hda/codecs/realtek/alc269.c
@@ -7555,6 +7555,7 @@ static const struct hda_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0xf111, 0x0009, "Framework Laptop", ALC295_FIXUP_FRAMEWORK_LAPTOP_MIC_NO_PRESENCE),
 	SND_PCI_QUIRK(0xf111, 0x000b, "Framework Laptop", ALC295_FIXUP_FRAMEWORK_LAPTOP_MIC_NO_PRESENCE),
 	SND_PCI_QUIRK(0xf111, 0x000c, "Framework Laptop", ALC295_FIXUP_FRAMEWORK_LAPTOP_MIC_NO_PRESENCE),
+	SND_PCI_QUIRK(0xf111, 0x000f, "Framework Laptop", ALC295_FIXUP_FRAMEWORK_LAPTOP_MIC_NO_PRESENCE),
 
 #if 0
 	/* Below is a quirk table taken from the old code.
-- 
2.53.0


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

* [PATCH AUTOSEL 7.0-5.15] ALSA: pcm: Serialize snd_pcm_suspend_all() with open_mutex
       [not found] <20260420131539.986432-1-sashal@kernel.org>
  2026-04-20 13:08 ` [PATCH AUTOSEL 6.18] ALSA: hda/realtek: add quirk for Framework F111:000F Sasha Levin
@ 2026-04-20 13:08 ` Sasha Levin
  2026-04-20 13:08 ` [PATCH AUTOSEL 7.0-6.18] ALSA: hda/realtek: Add quirk for CSL Unity BF24B Sasha Levin
  2026-04-20 13:08 ` [PATCH AUTOSEL 7.0-5.10] ALSA: aoa/onyx: Fix OF node leak on probe failure Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
  To: patches, stable
  Cc: Cássio Gabriel, Takashi Iwai, Takashi Iwai, Sasha Levin,
	perex, linux-sound, linux-kernel

From: Cássio Gabriel <cassiogabrielcontato@gmail.com>

[ Upstream commit 1a56641b7ae4f19216774a59d68024be3e6197d0 ]

snd_pcm_suspend_all() walks all PCM substreams and uses a lockless
runtime check to skip closed streams. It then calls snd_pcm_suspend()
for each remaining substream and finally runs snd_pcm_sync_stop() in a
second pass.

The runtime lifetime is still controlled by pcm->open_mutex in the
open/release path. That means a concurrent close can clear or free
substream->runtime after the initial check in snd_pcm_suspend_all(),
leaving the later suspend or sync-stop path to dereference a stale or
NULL runtime pointer.

Serialize snd_pcm_suspend_all() with pcm->open_mutex so the runtime
pointer stays stable across both loops. This matches the existing PCM
runtime lifetime rule already used by other core paths that access
substream->runtime outside the stream lock.

Suggested-by: Takashi Iwai <tiwai@suse.com>
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
Link: https://patch.msgid.link/20260327-alsa-pcm-suspend-open-close-lock-v2-1-cc4baca4dcd6@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Here is my complete analysis:

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject Line
- **Subsystem**: `ALSA: pcm:` (ALSA PCM core)
- **Action verb**: "Serialize" — implies fixing a synchronization issue
  (race condition)
- **Summary**: Serialize `snd_pcm_suspend_all()` with `open_mutex` to
  prevent concurrent access

Record: [ALSA PCM core] [Serialize] [Add mutex to prevent race between
suspend and open/close]

### Step 1.2: Tags
- **Suggested-by: Takashi Iwai `<tiwai@suse.com>`** — Takashi Iwai is
  the ALSA subsystem maintainer. He suggested this fix, which carries
  enormous weight.
- **Signed-off-by: Takashi Iwai `<tiwai@suse.de>`** — Maintainer also
  signed off (committed it).
- **Signed-off-by: Cássio Gabriel** — Author of the patch.
- **Link:** to patch.msgid.link (v2 of the patch).
- No Fixes: tag, no Reported-by, no Cc: stable — expected for autosel
  candidates.

Record: Fix suggested and committed by the subsystem maintainer (Takashi
Iwai). This is the highest trust signal possible.

### Step 1.3: Commit Body Analysis
The commit describes:
- **Bug**: `snd_pcm_suspend_all()` walks PCM substreams with a lockless
  runtime check, then calls `snd_pcm_suspend()` and
  `snd_pcm_sync_stop()`. A concurrent close can clear/free
  `substream->runtime` via the open/close path (which uses
  `pcm->open_mutex`).
- **Failure mode**: Stale or NULL runtime pointer dereference —
  crash/UAF.
- **Fix approach**: Take `pcm->open_mutex` in `snd_pcm_suspend_all()`,
  matching the existing PCM runtime lifetime rule.

Record: UAF/NULL deref race during system suspend. Author explains clear
mechanism.

### Step 1.4: Hidden Bug Fix Detection
This is NOT hidden — it's an explicit race condition fix. The code
itself had a `/* FIXME: the open/close code should lock this as well */`
comment acknowledging the bug.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
- **Files changed**: 1 (`sound/core/pcm_native.c`)
- **Lines**: +5 (including comment update), -1 (removing FIXME comment).
  Net functional change: +1 line (`guard(mutex)(&pcm->open_mutex)`)
- **Functions modified**: `snd_pcm_suspend_all()`
- **Scope**: Single-file, single-function, surgical fix.

### Step 2.2: Code Flow Change
**Before**: `snd_pcm_suspend_all()` iterates substreams without any
lock, checking `substream->runtime` and calling `snd_pcm_suspend()` /
`snd_pcm_sync_stop()` unsynchronized with the open/close path.

**After**: `guard(mutex)(&pcm->open_mutex)` is taken at function entry,
serializing the entire function with the release path
(`snd_pcm_release()` at line 2941 holds `open_mutex` around
`snd_pcm_release_substream()`).

### Step 2.3: Bug Mechanism
**Category**: Race condition / Use-After-Free / NULL pointer dereference

The race:
1. Thread A (`snd_pcm_suspend_all`): checks `!substream->runtime` → sees
   non-NULL
2. Thread B (`snd_pcm_release`): under `open_mutex`, calls
   `snd_pcm_detach_substream()` → sets `substream->runtime = NULL` →
   `kfree(runtime)` (confirmed in `sound/core/pcm.c` lines 980-1003)
3. Thread A: calls `snd_pcm_suspend(substream)` → dereferences
   freed/NULL runtime → **CRASH**

Second race window: the second loop calls `snd_pcm_sync_stop()` on ALL
substreams, which accesses `substream->runtime` (line 641).

### Step 2.4: Fix Quality
- Obviously correct: takes the same mutex already used by open/close
  paths.
- Minimal: one functional line added.
- Regression risk: Very low. The `open_mutex` is not taken in interrupt
  context; `snd_pcm_suspend()` internally uses
  `guard(pcm_stream_lock_irqsave)` which is a different lock, so no
  deadlock risk. The mutex merely serializes with open/close.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
The FIXME comment (`/* FIXME: the open/close code should lock this as
well */`) was introduced in commit `8d19b4e0b377e2` (2021-02-06, "ALSA:
pcm: Use for_each_pcm_substream() macro") by Takashi Iwai himself. The
core `snd_pcm_suspend_all` function dates back to `1da177e4c3f41` (Linus
Torvalds, 2005-04-16, the initial Linux tree).

Record: The race has existed since v2.6.12 (original tree). It was made
worse in v5.12 when `snd_pcm_sync_stop` loop was added (commit
`2c87c1a49c9d`). The FIXME was planted by the maintainer acknowledging
the bug.

### Step 3.2: Fixes tag
No Fixes: tag present. The race effectively has existed since the
beginning.

### Step 3.3: File History
Recent changes to `pcm_native.c` include `guard()`-based locking
conversions and race fixes (e.g., `93a81ca065775` "Fix race of buffer
access at PCM OSS layer"). This shows the subsystem is actively being
hardened for concurrency.

### Step 3.4: Author
Cássio Gabriel has one other commit in the sound subsystem. However, the
fix was **suggested by Takashi Iwai** (ALSA maintainer) and **committed
by Takashi Iwai**, giving it the highest credibility.

### Step 3.5: Dependencies
The fix uses `guard(mutex)` which requires the cleanup.h infrastructure
(available since v6.5) and the guard conversions in pcm_native.c
(`dd0da75b9a276`, available since v6.12). For older stable trees, a
trivial adaptation to `mutex_lock`/`mutex_unlock` would be needed.

---

## PHASE 4: MAILING LIST RESEARCH

### Step 4.1-4.2: Patch Discussion
b4 dig did not find the patch by commit hash (likely because this is on
a v7.0 tree). The Link: tag points to `patch.msgid.link`, which
redirected to lore.kernel.org but was blocked by anti-bot protection.
However, the commit subject says "v2", indicating the patch went through
at least two revisions. The maintainer (Takashi Iwai) suggested and
committed the fix — the highest form of endorsement.

### Step 4.3-4.5: Bug Report and Stable History
No explicit bug report (no Reported-by:). This is a proactive fix based
on code analysis (the FIXME comment). No prior stable discussion found.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1-5.2: Callers of `snd_pcm_suspend_all`
- `sound/core/pcm.c:do_pcm_suspend()` — PM callback, called during
  **system suspend**
- `sound/soc/soc-core.c` — ASoC suspend
- `sound/hda/common/codec.c` — HDA codec suspend
- `sound/usb/usx2y/us144mkii.c` — USB audio suspend

This is called on **every system suspend** for every PCM device. This
means every laptop, desktop, or embedded system with audio is affected.

### Step 5.3-5.4: Call Chain
System suspend → `do_pcm_suspend()` → `snd_pcm_suspend_all()` → race
with user-space closing audio fd → `snd_pcm_release()` → UAF/NULL deref.

Trigger: Closing a laptop lid while an audio application is running.
This is an extremely common scenario.

---

## PHASE 6: STABLE TREE ANALYSIS

### Step 6.1: Buggy Code in Stable
The buggy code (unsynchronized runtime access in `snd_pcm_suspend_all`)
has existed since v2.6.12. The `snd_pcm_sync_stop` second loop was added
around v5.12. All active stable trees contain the buggy code.

### Step 6.2: Backport Complications
For v7.0 and v6.12+: should apply cleanly (same `guard()`
infrastructure).
For v6.6 and older: would need trivial adaptation from `guard(mutex)` to
`mutex_lock`/`mutex_unlock`.

---

## PHASE 7: SUBSYSTEM CONTEXT

### Step 7.1: Subsystem Criticality
- **Subsystem**: ALSA PCM core (`sound/core/`)
- **Criticality**: IMPORTANT — affects every system with audio hardware
  (virtually all Linux systems)

### Step 7.2: Activity
Very actively maintained by Takashi Iwai. Recent commits show ongoing
concurrency hardening.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who Is Affected
Every Linux system with audio hardware (virtually universal).

### Step 8.2: Trigger Conditions
System suspend while an audio stream is being closed. This is a common
scenario on laptops (lid close while music playing). The race window is
between checking `substream->runtime` and using it — a narrow but real
window.

### Step 8.3: Failure Mode Severity
NULL pointer dereference or use-after-free → **kernel oops/crash during
suspend**. Severity: **CRITICAL**.

### Step 8.4: Risk-Benefit
- **Benefit**: HIGH — prevents kernel crash during suspend for all
  audio-capable systems
- **Risk**: VERY LOW — one functional line, uses existing mutex already
  employed by the open/close paths, no deadlock risk, suggested and
  committed by the subsystem maintainer
- **Ratio**: Strongly favorable

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence Summary

**FOR backporting:**
- Fixes a real race condition (UAF / NULL deref) acknowledged by a FIXME
  comment from the subsystem maintainer
- Fix was suggested and committed by Takashi Iwai (ALSA maintainer)
- Minimal change: 1 functional line added
- Affects every system with audio during system suspend
- Failure mode is kernel crash (CRITICAL severity)
- Uses existing locking infrastructure already in use by the same code
  paths
- Second revision (v2), showing review iteration

**AGAINST backporting:**
- No user-reported crash (proactive fix based on code analysis)
- May need minor adaptation for older stable trees (guard →
  mutex_lock/unlock)

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** — suggested by maintainer, uses
   existing mutex pattern
2. Fixes a real bug? **YES** — race between suspend and release, FIXME
   acknowledged the issue
3. Important issue? **YES** — UAF/NULL deref → kernel crash during
   suspend
4. Small and contained? **YES** — 1 functional line in 1 function in 1
   file
5. No new features or APIs? **YES** — pure synchronization fix
6. Can apply to stable? **YES** — clean for 6.12+, trivial adaptation
   for older

### Step 9.3: Exception Categories
Not an exception — this is a standard race condition fix, a core stable
candidate.

### Step 9.4: Decision
This is a textbook stable fix: a single-line mutex acquisition that
prevents a UAF/NULL deref race during system suspend, suggested and
committed by the subsystem maintainer, fixing a longstanding FIXME.

---

## Verification

- [Phase 1] Parsed tags: Suggested-by and Signed-off-by from Takashi
  Iwai (ALSA maintainer)
- [Phase 2] Diff analysis: +1 functional line
  (`guard(mutex)(&pcm->open_mutex)`), removes 1-line FIXME comment
- [Phase 2] Verified `snd_pcm_sync_stop()` accesses `substream->runtime`
  at line 641 without lock
- [Phase 2] Verified `snd_pcm_detach_substream()` sets
  `substream->runtime = NULL` and `kfree(runtime)` at lines 997-1003 of
  `sound/core/pcm.c`
- [Phase 2] Verified `snd_pcm_release()` holds `open_mutex` around
  `snd_pcm_release_substream()` at line 2941
- [Phase 3] git blame: FIXME comment introduced by Takashi Iwai in
  `8d19b4e0b377e2` (2021-02-06), core function from `1da177e4c3f41`
  (2005)
- [Phase 3] `snd_pcm_sync_stop` loop added in `2c87c1a49c9d11` ("Assure
  sync with pending stop operation at suspend", 2021)
- [Phase 3] `guard()` infrastructure for pcm_native.c in `dd0da75b9a276`
  (v6.12+)
- [Phase 4] b4 dig: did not find match (this is a v7.0 tree). Link: in
  commit points to v2 of the patch.
- [Phase 5] Callers: `do_pcm_suspend()` (PM callback), soc-core, HDA
  codec, USB audio — called on every system suspend
- [Phase 6] Buggy code present in all active stable trees (since
  v2.6.12)
- [Phase 7] ALSA PCM core — affects virtually all Linux systems with
  audio
- [Phase 8] Failure mode: NULL deref or UAF during system suspend →
  kernel crash (CRITICAL)

**YES**

 sound/core/pcm_native.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 5a64453da7283..1ccc482a8dfd3 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1761,6 +1761,9 @@ static int snd_pcm_suspend(struct snd_pcm_substream *substream)
  * snd_pcm_suspend_all - trigger SUSPEND to all substreams in the given pcm
  * @pcm: the PCM instance
  *
+ * Takes and releases pcm->open_mutex to serialize against
+ * concurrent open/close while walking the substreams.
+ *
  * After this call, all streams are changed to SUSPENDED state.
  *
  * Return: Zero if successful (or @pcm is %NULL), or a negative error code.
@@ -1773,8 +1776,9 @@ int snd_pcm_suspend_all(struct snd_pcm *pcm)
 	if (! pcm)
 		return 0;
 
+	guard(mutex)(&pcm->open_mutex);
+
 	for_each_pcm_substream(pcm, stream, substream) {
-		/* FIXME: the open/close code should lock this as well */
 		if (!substream->runtime)
 			continue;
 
-- 
2.53.0


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

* [PATCH AUTOSEL 7.0-6.18] ALSA: hda/realtek: Add quirk for CSL Unity BF24B
       [not found] <20260420131539.986432-1-sashal@kernel.org>
  2026-04-20 13:08 ` [PATCH AUTOSEL 6.18] ALSA: hda/realtek: add quirk for Framework F111:000F Sasha Levin
  2026-04-20 13:08 ` [PATCH AUTOSEL 7.0-5.15] ALSA: pcm: Serialize snd_pcm_suspend_all() with open_mutex Sasha Levin
@ 2026-04-20 13:08 ` Sasha Levin
  2026-04-20 13:08 ` [PATCH AUTOSEL 7.0-5.10] ALSA: aoa/onyx: Fix OF node leak on probe failure Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
  To: patches, stable
  Cc: Zhang Heng, Takashi Iwai, Sasha Levin, perex, tiwai, linux-sound,
	linux-kernel

From: Zhang Heng <zhangheng@kylinos.cn>

[ Upstream commit de65275fc94e2e0acc79bd016d60889bf251ccd9 ]

The CSL Unity BF24B all-in-one PC uses a Realtek ALC662 rev3 audio
codec and requires the correct GPIO configuration to enable sound
output from both the speakers and the headphone.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=221258
Signed-off-by: Zhang Heng <zhangheng@kylinos.cn>
Link: https://patch.msgid.link/20260409024028.1297587-1-zhangheng@kylinos.cn
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information needed for a comprehensive analysis. Let
me compile the findings.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

**Step 1.1: Subject Line**
- Subsystem: `ALSA: hda/realtek`
- Action verb: "Add quirk" — this is adding a hardware-specific
  workaround
- Summary: Adding GPIO quirk for CSL Unity BF24B all-in-one PC to enable
  audio output
- Record: [ALSA/HDA/Realtek] [Add quirk] [GPIO config to enable
  speakers/headphone on CSL Unity BF24B]

**Step 1.2: Tags**
- Link: `https://bugzilla.kernel.org/show_bug.cgi?id=221258` — user-
  filed bug report
- Signed-off-by: Zhang Heng (author, regular Realtek quirk contributor)
- Link: patch.msgid.link (for tracking the patch)
- Signed-off-by: Takashi Iwai (ALSA subsystem maintainer — applied the
  patch)
- No Fixes: tag (expected — this is a quirk addition, not fixing a code
  defect)
- No Cc: stable (expected — this is why we're reviewing it)
- Record: Bugzilla report from real user. Signed off by the ALSA
  maintainer.

**Step 1.3: Commit Body**
- Bug: CSL Unity BF24B all-in-one PC has no sound output from speakers
  or headphones
- Codec: Realtek ALC662 rev3
- Root cause: GPIO configuration needed to enable the amplifier
- Record: Complete audio failure on specific hardware without GPIO
  toggle workaround.

**Step 1.4: Hidden Bug Fix Detection**
- This is not a hidden bug fix — it's an explicit hardware quirk
  addition. The device literally has no audio without it.

## PHASE 2: DIFF ANALYSIS

**Step 2.1: Inventory**
- Files changed: 1 (`sound/hda/codecs/realtek/alc662.c`)
- Lines added: ~25 (new function + enum + fixup entry + quirk entry)
- Functions modified: none existing; one new function added
  (`alc662_fixup_csl_amp`)
- Scope: single-file, surgical, self-contained

**Step 2.2: Code Flow Change**
1. New function `alc662_fixup_csl_amp`:
   - `HDA_FIXUP_ACT_PRE_PROBE`: Sets GPIO mask and direction for bits
     0+1
   - `HDA_FIXUP_ACT_INIT`: Toggles GPIO 0+1 high, waits 100ms, then low
     — to enable the amplifier
2. New enum `ALC662_FIXUP_CSL_GPIO` added at end of existing enum
3. New fixup table entry linking enum to function
4. New quirk table entry: `SND_PCI_QUIRK(0x1022, 0xc950, "CSL Unity
   BF24B", ALC662_FIXUP_CSL_GPIO)`

**Step 2.3: Bug Mechanism**
- Category: Hardware workaround (h)
- The CSL Unity BF24B's amplifier requires a GPIO toggle pulse to
  enable. Without it, the amp stays off and no sound comes out.

**Step 2.4: Fix Quality**
- The pattern is nearly identical to `alc245_fixup_hp_x360_amp` in
  alc269.c (lines 1448-1465), which toggles GPIO0 similarly. This new
  one toggles GPIO0+GPIO1 (mask 0x03).
- Obviously correct — follows well-established patterns used dozens of
  times in this driver.
- Minimal regression risk — only affects devices with PCI SSID
  0x1022:0xc950.
- Takashi Iwai noted a minor inefficiency (extra GPIO write from
  `alc_auto_init_amp`) but accepted the patch as-is, planning cleanup
  later.

## PHASE 3: GIT HISTORY INVESTIGATION

**Step 3.1: Blame**
- The ALC662 quirk infrastructure has been in the kernel for many years.
  The file was recently split from `sound/pci/hda/patch_realtek.c` in
  commit `aeeb85f26c3bb` (July 2025, kernel 7.0-rc cycle), which was
  itself moved from `sound/pci/hda/` to `sound/hda/codecs/` in
  `6014e9021b28e`.
- The ALC662 fixup enum and table existed in `patch_realtek.c` long
  before the split.

**Step 3.2: No Fixes: tag** — expected for a quirk addition.

**Step 3.3: File History**
- Recent changes are all quirk additions — this is a very common pattern
  for this file.
- The author (Zhang Heng) has contributed many similar quirks: Acer
  Swift, HP Laptop, ASUS ROG, Lenovo Yoga, etc.

**Step 3.4: Author**
- Zhang Heng is a prolific Realtek quirk contributor with 10+ similar
  commits in the tree.

**Step 3.5: Dependencies**
- This is fully standalone. Uses only existing APIs
  (`alc_update_gpio_data`, `alc_spec`).
- For stable trees (6.x), the patch would need to target
  `sound/pci/hda/patch_realtek.c` instead. The functions and structures
  are identical there (verified: `alc_update_gpio_data` appears 14 times
  in pre-split `patch_realtek.c`).

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

**Step 4.1: Patch Discussion**
- Found full discussion on patchew.org. Takashi Iwai reviewed the patch
  and asked about the necessity of setting `gpio_mask`/`gpio_dir` in
  `PRE_PROBE`. He proposed a cleaner GPIO helper approach.
- Despite reservations about minor inefficiency, Iwai explicitly stated:
  *"I'm going to take your patch for now, but we might need to
  reconsider the implementation"*.
- The patch was accepted as v1, no subsequent versions.

**Step 4.2: Reviewers**
- Takashi Iwai (ALSA subsystem maintainer) directly reviewed and applied
  it.

**Step 4.3: Bug Report**
- Bugzilla #221258 — user tested with `hda-verb` commands setting GPIO
  mask, direction, and data to confirm the workaround works. The user's
  manual commands were:
  - SET_GPIO_MASK 0x03, SET_GPIO_DIRECTION 0x03, SET_GPIO_DATA 0x03,
    then toggle

**Step 4.4: Series Context**
- Single standalone patch, not part of a series.

**Step 4.5: Stable Discussion**
- No explicit stable nomination found, but audio quirks are routinely
  backported.

## PHASE 5: CODE SEMANTIC ANALYSIS

**Step 5.1: Functions**
- New: `alc662_fixup_csl_amp` — only called via the fixup framework for
  matching PCI SSID
- Uses: `alc_update_gpio_data` (well-tested existing function, used 14+
  times across the driver)

**Step 5.2: Callers**
- Called only by the HDA fixup framework when a device with PCI SSID
  0x1022:0xc950 is detected.

**Step 5.3-5.5**: The function follows an identical pattern to
`alc245_fixup_hp_x360_amp`, which is well-established and proven.

## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS

**Step 6.1: Buggy Code in Stable**
- The ALC662 codec support exists in all stable trees. The
  `alc662_fixups[]` table and `alc662_fixup_tbl[]` quirk table have been
  present since early kernel versions.
- The functions used (`alc_update_gpio_data`, `msleep`, etc.) exist in
  all active stable trees.

**Step 6.2: Backport Complications**
- **Key issue**: In 7.0, the driver was split into
  `sound/hda/codecs/realtek/alc662.c`. In stable 6.x trees, the
  equivalent code lives in `sound/pci/hda/patch_realtek.c`. The patch
  will need path adjustment but the code structures are identical.
- Record: Minor mechanical adjustment needed for file path; code applies
  cleanly to older structure.

**Step 6.3: No related fixes already in stable.**

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

**Step 7.1**: Subsystem is ALSA/HDA (audio). Criticality: IMPORTANT —
audio is essential for desktop/laptop users.

**Step 7.2**: Very actively developed — constant quirk additions for new
hardware.

## PHASE 8: IMPACT AND RISK ASSESSMENT

**Step 8.1: Who is Affected**
- Users of CSL Unity BF24B all-in-one PCs. Without this quirk, they have
  **zero audio output**.

**Step 8.2: Trigger Conditions**
- Every boot, every resume from suspend. Audio is completely non-
  functional without the quirk.

**Step 8.3: Failure Mode Severity**
- Complete loss of audio functionality on the device. Severity: HIGH for
  affected users.

**Step 8.4: Risk-Benefit Ratio**
- BENEFIT: High — enables audio on a specific PC model that has zero
  sound without it
- RISK: Very low — 25 lines, only affects one PCI SSID, follows proven
  patterns, reviewed by subsystem maintainer
- Ratio: Strongly favorable

## PHASE 9: FINAL SYNTHESIS

**Step 9.1: Evidence Summary**
- FOR: Hardware quirk for broken audio; user-filed bugzilla; subsystem
  maintainer reviewed and applied; follows identical pattern to existing
  quirks; small and surgical; standalone
- AGAINST: File path needs adjustment for stable (mechanical); Takashi
  noted minor inefficiency (but accepted anyway)
- UNRESOLVED: Could not access bugzilla directly (anti-bot protection),
  but bug context confirmed through mailing list discussion

**Step 9.2: Stable Rules Checklist**
1. Obviously correct and tested? **YES** — follows proven pattern,
   reviewed by maintainer, tested by user with hda-verb
2. Fixes a real bug? **YES** — zero audio output on specific hardware
3. Important issue? **YES** — complete loss of audio functionality
4. Small and contained? **YES** — 25 lines in one file
5. No new features or APIs? **YES** — only a device-specific workaround
6. Can apply to stable? **YES** — needs file path adjustment but code is
   compatible

**Step 9.3: Exception Category**
- This is a textbook **HARDWARE QUIRK/WORKAROUND** — explicitly listed
  as a YES exception for stable.

## Verification

- [Phase 1] Parsed tags: Link to bugzilla.kernel.org #221258, signed off
  by Takashi Iwai (ALSA maintainer)
- [Phase 2] Diff analysis: 25 lines added — new fixup function, enum
  entry, fixup table entry, quirk table entry
- [Phase 2] Pattern match: `alc662_fixup_csl_amp` is nearly identical to
  `alc245_fixup_hp_x360_amp` at alc269.c:1448-1465
- [Phase 3] git log: file split from `patch_realtek.c` in aeeb85f26c3bb
  (Jul 2025), previously moved in 6014e9021b28e
- [Phase 3] git log --author: Zhang Heng has 10+ similar quirk commits
  in this tree
- [Phase 3] Verified `alc_update_gpio_data` exists 14 times in pre-split
  `patch_realtek.c`
- [Phase 4] Patchew.org: full discussion retrieved; Takashi reviewed,
  questioned PRE_PROBE gpio_mask but accepted patch
- [Phase 4] Takashi Iwai explicitly stated: "I'm going to take your
  patch for now"
- [Phase 4] User tested with hda-verb commands confirming GPIO toggle
  enables audio
- [Phase 5] `alc_update_gpio_data` is a well-tested exported function
  used across the driver
- [Phase 6] ALC662 support and fixup infrastructure exist in all active
  stable trees
- [Phase 6] Backport needs path adjustment:
  `sound/hda/codecs/realtek/alc662.c` → `sound/pci/hda/patch_realtek.c`
- [Phase 8] Failure mode: complete audio loss on affected hardware,
  severity HIGH
- UNVERIFIED: Could not directly access bugzilla #221258 (anti-bot
  protection), but context confirmed via mailing list

This is a textbook hardware quirk addition — small, self-contained,
fixes complete audio failure on a specific device, follows proven
patterns, reviewed and signed off by the subsystem maintainer. It falls
directly into the "quirks and workarounds" exception category that is
automatically appropriate for stable.

**YES**

 sound/hda/codecs/realtek/alc662.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/sound/hda/codecs/realtek/alc662.c b/sound/hda/codecs/realtek/alc662.c
index 5073165d1f3cf..3abe41c7315c4 100644
--- a/sound/hda/codecs/realtek/alc662.c
+++ b/sound/hda/codecs/realtek/alc662.c
@@ -255,6 +255,25 @@ static void alc_fixup_headset_mode_alc668(struct hda_codec *codec,
 	alc_fixup_headset_mode(codec, fix, action);
 }
 
+static void alc662_fixup_csl_amp(struct hda_codec *codec,
+				 const struct hda_fixup *fix, int action)
+{
+	struct alc_spec *spec = codec->spec;
+
+	switch (action) {
+	case HDA_FIXUP_ACT_PRE_PROBE:
+		spec->gpio_mask |= 0x03;
+		spec->gpio_dir |= 0x03;
+		break;
+	case HDA_FIXUP_ACT_INIT:
+		/* need to toggle GPIO to enable the amp */
+		alc_update_gpio_data(codec, 0x03, true);
+		msleep(100);
+		alc_update_gpio_data(codec, 0x03, false);
+		break;
+	}
+}
+
 enum {
 	ALC662_FIXUP_ASPIRE,
 	ALC662_FIXUP_LED_GPIO1,
@@ -313,6 +332,7 @@ enum {
 	ALC897_FIXUP_HEADSET_MIC_PIN2,
 	ALC897_FIXUP_UNIS_H3C_X500S,
 	ALC897_FIXUP_HEADSET_MIC_PIN3,
+	ALC662_FIXUP_CSL_GPIO,
 };
 
 static const struct hda_fixup alc662_fixups[] = {
@@ -766,11 +786,16 @@ static const struct hda_fixup alc662_fixups[] = {
 			{ }
 		},
 	},
+	[ALC662_FIXUP_CSL_GPIO] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc662_fixup_csl_amp,
+	},
 };
 
 static const struct hda_quirk alc662_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x1019, 0x9087, "ECS", ALC662_FIXUP_ASUS_MODE2),
 	SND_PCI_QUIRK(0x1019, 0x9859, "JP-IK LEAP W502", ALC897_FIXUP_HEADSET_MIC_PIN3),
+	SND_PCI_QUIRK(0x1022, 0xc950, "CSL Unity BF24B", ALC662_FIXUP_CSL_GPIO),
 	SND_PCI_QUIRK(0x1025, 0x022f, "Acer Aspire One", ALC662_FIXUP_INV_DMIC),
 	SND_PCI_QUIRK(0x1025, 0x0241, "Packard Bell DOTS", ALC662_FIXUP_INV_DMIC),
 	SND_PCI_QUIRK(0x1025, 0x0308, "Acer Aspire 8942G", ALC662_FIXUP_ASPIRE),
-- 
2.53.0


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

* [PATCH AUTOSEL 7.0-5.10] ALSA: aoa/onyx: Fix OF node leak on probe failure
       [not found] <20260420131539.986432-1-sashal@kernel.org>
                   ` (2 preceding siblings ...)
  2026-04-20 13:08 ` [PATCH AUTOSEL 7.0-6.18] ALSA: hda/realtek: Add quirk for CSL Unity BF24B Sasha Levin
@ 2026-04-20 13:08 ` Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
  To: patches, stable
  Cc: wangdicheng, Takashi Iwai, Sasha Levin, johannes, perex, tiwai,
	linuxppc-dev, linux-sound, linux-kernel

From: wangdicheng <wangdicheng@kylinos.cn>

[ Upstream commit e5d5aef802a5f41283084f7d443ef4fd4b65d86d ]

Add missing of_node_put() in the error path.

Signed-off-by: wangdicheng <wangdicheng@kylinos.cn>
Link: https://patch.msgid.link/20260403014736.33014-1-wangdich9700@163.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information needed. Let me compile the full analysis.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Parse the Subject Line
- **Subsystem**: ALSA: aoa/onyx (Apple Onboard Audio, onyx codec)
- **Action verb**: "Fix" — direct bug fix
- **Summary**: Fixes an OF (Open Firmware) device tree node reference
  leak when the probe function fails

### Step 1.2: Parse All Commit Message Tags
- **Signed-off-by**: wangdicheng <wangdicheng@kylinos.cn> (author)
- **Link**:
  https://patch.msgid.link/20260403014736.33014-1-wangdich9700@163.com
  (patch discussion)
- **Signed-off-by**: Takashi Iwai <tiwai@suse.de> (ALSA subsystem
  maintainer; this means it was reviewed/applied by the maintainer)
- No Fixes: tag (expected — that's why we're reviewing)
- No Cc: stable (expected — that's why we're reviewing)
- No Reported-by (static analysis / code review finding)

### Step 1.3: Analyze the Commit Body Text
The commit body is concise: "Add missing of_node_put() in the error
path." This describes a classic OF node reference leak fix. When
`of_node_get()` acquires a reference but the error path doesn't call
`of_node_put()`, the node's reference count is leaked.

### Step 1.4: Detect Hidden Bug Fixes
This is an explicit bug fix, not disguised. The subject says "Fix" and
the body confirms a missing cleanup call.

---

## PHASE 2: DIFF ANALYSIS - LINE BY LINE

### Step 2.1: Inventory the Changes
- **File**: `sound/aoa/codecs/onyx.c`
- **Lines**: +3, -1 (net +2 lines)
- **Function**: `onyx_i2c_probe()`
- **Scope**: Single-file surgical fix to one error path

### Step 2.2: Understand the Code Flow Change
The key flow in `onyx_i2c_probe()`:

```978:990:sound/aoa/codecs/onyx.c
        onyx->codec.init = onyx_init_codec;
        onyx->codec.exit = onyx_exit_codec;
        onyx->codec.node = of_node_get(node);  // acquires OF node
reference

        if (aoa_codec_register(&onyx->codec)) {
                goto fail;  // BEFORE: leaks OF node reference
        }
        ...
 fail:
        kfree(onyx);  // frees memory but doesn't release OF node ref
        return -ENODEV;
```

**After the fix**: `goto fail` becomes `goto fail_put`, which first
calls `of_node_put(onyx->codec.node)` then falls through to `fail`.

### Step 2.3: Identify the Bug Mechanism
Category: **Reference counting fix / resource leak**. `of_node_get()`
increments the device tree node refcount. If `aoa_codec_register()`
fails, the refcount is never decremented, leaking the OF node.

### Step 2.4: Assess the Fix Quality
- **Obviously correct**: Yes — `of_node_get()` at line 980 must be
  balanced by `of_node_put()` on error. The remove path at line 997
  already correctly calls `of_node_put()`.
- **Minimal/surgical**: Yes — only 3 lines added, 1 changed.
- **Regression risk**: Essentially zero. The new label only executes on
  error paths and merely balances a reference count.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame the Changed Lines
The buggy code was introduced by commit `f3d9478b2ce468` ("[ALSA] snd-
aoa: add snd-aoa") by Johannes Berg, dated **2006-06-21**. This is the
original addition of the entire AOA subsystem. The bug has been present
for ~20 years, meaning it exists in **every supported stable tree**.

### Step 3.2: Follow the Fixes Tag
No Fixes: tag present, but the implicit fix target is `f3d9478b2ce468`
(2006). The same commit was referenced by the related fix
`222bce5eb88d1` ("ALSA: snd-aoa: add of_node_put() in error path") which
fixed a *similar* OF node leak in `sound/aoa/core/gpio-feature.c` but
did NOT touch `onyx.c`.

### Step 3.3: Related Changes
- `222bce5eb88d1` — Fixed same bug class in `gpio-feature.c` (2018),
  never touched onyx.c
- The sibling codec `tas.c` has the **exact same bug** at lines 864-875
  (acquires `of_node_get(node)` but `goto fail` doesn't release it)

### Step 3.4: Author's Other Commits
wangdicheng appears to be contributing fixes across multiple subsystems
(KylinOS developer). Takashi Iwai, who applied the patch, is the ALSA
subsystem maintainer — adding significant confidence.

### Step 3.5: Dependent/Prerequisite Commits
None. The fix is fully standalone. The code structure (`fail` label,
`kfree(onyx)`) has been unchanged since 2006.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1-4.5: Patch Discussion
Lore.kernel.org is behind a proof-of-work challenge, and b4 dig couldn't
match this commit by patch-id. However, the patch was applied by ALSA
maintainer Takashi Iwai directly, which confirms it passed maintainer
review. The Link tag provides the message-id for the submission.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key Functions
- `onyx_i2c_probe()` — the I2C driver probe function, called when the
  device is matched

### Step 5.2: Callers
`onyx_i2c_probe()` is the `.probe` callback in `onyx_driver` (i2c_driver
struct at line 1012). It is called by the I2C core during device
enumeration/binding. This is a standard device probe path.

### Step 5.3-5.4: Reachability
The probe function is called whenever the kernel attempts to bind the
onyx I2C codec to its driver. On Apple Mac hardware with this audio
codec, this happens during boot. If `aoa_codec_register()` fails (which
can happen — e.g. if the fabric isn't registered yet), this leak
triggers.

### Step 5.5: Similar Patterns
As verified, `sound/aoa/codecs/tas.c` has the **identical bug** (lines
864-875): `of_node_get(node)` followed by `goto fail` that doesn't call
`of_node_put()`.

---

## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS

### Step 6.1: Buggy Code Existence in Stable Trees
The buggy code exists in **all** stable trees. It was introduced in 2006
(`f3d9478b2ce468`) and has never been fixed.

### Step 6.2: Backport Complications
The file has had minor changes (guard() conversions, alloc_obj macros)
but the probe function structure around the error path is essentially
unchanged since 2006. The patch should apply **cleanly** to all stable
trees, possibly with trivial context adjustments (e.g. `kzalloc_obj` vs
`kzalloc`).

### Step 6.3: Related Fixes Already in Stable
No. The related `222bce5eb88d1` fix was for `gpio-feature.c`, not
`onyx.c`.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: Subsystem Criticality
- **Subsystem**: ALSA / AOA (Apple Onboard Audio) — driver-specific
- **Criticality**: PERIPHERAL — affects Apple PowerPC/Mac hardware with
  onyx codecs
- **Maintainer review**: Applied by Takashi Iwai (ALSA maintainer)
  directly

### Step 7.2: Subsystem Activity
Low activity (last substantive change was treewide refactoring). This is
a mature, stable driver.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who Is Affected
Users of Apple Mac hardware with the pcm3052 (onyx) audio codec. This is
primarily older PowerPC-based Macs.

### Step 8.2: Trigger Conditions
The leak triggers when `aoa_codec_register()` fails during probe. This
can happen if the AOA fabric isn't registered, or if
`attach_codec_to_fabric()` returns an error. While not extremely common,
repeated probe failures (e.g. during deferred probing or error
injection) would accumulate leaked OF nodes.

### Step 8.3: Failure Mode Severity
- **Failure mode**: OF node reference count leak (resource leak)
- **Severity**: LOW-MEDIUM — repeated leaks consume memory, and the
  leaked OF node can never be freed, but this is a one-time probe path,
  not a hot path

### Step 8.4: Risk-Benefit Ratio
- **Benefit**: Fixes a real resource leak that has existed for 20 years.
  Trivial to understand and verify.
- **Risk**: Essentially zero — 3 lines added to an error path, one
  `goto` target changed
- **Ratio**: Very favorable — minimal risk for a correct bug fix

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence Compiled

**FOR backporting:**
- Fixes a real bug: missing `of_node_put()` causing OF node reference
  leak
- Extremely small and surgical fix (3 lines added, 1 changed)
- Obviously correct — verified by reading the code and comparing with
  the remove path
- Applied by ALSA subsystem maintainer (Takashi Iwai)
- Bug exists in all stable trees (introduced in 2006)
- Zero regression risk
- Consistent with similar fixes applied to the same subsystem
  (222bce5eb88d1)

**AGAINST backporting:**
- Low-severity bug (resource leak, not crash/security/corruption)
- Affects niche hardware (Apple PowerPC Macs)
- Only triggers on probe failure (not common under normal conditions)

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** — trivial fix, applied by
   maintainer
2. Fixes a real bug? **YES** — OF node reference leak
3. Important issue? **MEDIUM** — resource leak, not critical
4. Small and contained? **YES** — 3 lines, single file, single function
5. No new features or APIs? **YES** — pure fix
6. Can apply to stable trees? **YES** — code unchanged since 2006

### Step 9.3: Exception Categories
None applicable, but this is a standard reference counting bug fix — a
very common category for stable backports.

### Step 9.4: Decision
This is a small, obvious, correct fix for a real reference counting bug.
While the severity is low (resource leak on error path in niche
hardware), the fix carries essentially zero regression risk and meets
all stable kernel criteria.

---

## Verification

- [Phase 1] Parsed tags: Signed-off-by author + Takashi Iwai
  (maintainer), Link tag present
- [Phase 2] Diff analysis: 3 lines added to error path in
  `onyx_i2c_probe()`, adds missing `of_node_put()` after `of_node_get()`
  at line 980
- [Phase 3] git blame: buggy code introduced in `f3d9478b2ce468`
  (2006-06-21), present in all stable trees
- [Phase 3] Related fix `222bce5eb88d1`: same bug class fixed in `gpio-
  feature.c` but NOT in `onyx.c`
- [Phase 3] Confirmed `tas.c` has identical unfixed bug at lines 864-875
- [Phase 4] Lore blocked by anti-scraping; b4 dig failed to match. Patch
  applied by ALSA maintainer confirms review
- [Phase 5] `onyx_i2c_probe()` is standard I2C probe callback, called
  during device binding
- [Phase 5] `aoa_codec_register()` at `sound/aoa/core/core.c:57`
  confirmed: can return error from `attach_codec_to_fabric()`
- [Phase 6] Code structure unchanged since 2006; patch should apply
  cleanly to all stable trees
- [Phase 6] No related fix already in stable for this specific file
- [Phase 8] Failure mode: OF node reference leak, severity LOW-MEDIUM;
  risk of fix: essentially zero

**YES**

 sound/aoa/codecs/onyx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/aoa/codecs/onyx.c b/sound/aoa/codecs/onyx.c
index 04961c456d2c5..da0eebf5dfbc2 100644
--- a/sound/aoa/codecs/onyx.c
+++ b/sound/aoa/codecs/onyx.c
@@ -980,10 +980,12 @@ static int onyx_i2c_probe(struct i2c_client *client)
 	onyx->codec.node = of_node_get(node);
 
 	if (aoa_codec_register(&onyx->codec)) {
-		goto fail;
+		goto fail_put;
 	}
 	printk(KERN_DEBUG PFX "created and attached onyx instance\n");
 	return 0;
+ fail_put:
+	of_node_put(onyx->codec.node);
  fail:
 	kfree(onyx);
 	return -ENODEV;
-- 
2.53.0


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

end of thread, other threads:[~2026-04-20 13:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260420131539.986432-1-sashal@kernel.org>
2026-04-20 13:08 ` [PATCH AUTOSEL 6.18] ALSA: hda/realtek: add quirk for Framework F111:000F Sasha Levin
2026-04-20 13:08 ` [PATCH AUTOSEL 7.0-5.15] ALSA: pcm: Serialize snd_pcm_suspend_all() with open_mutex Sasha Levin
2026-04-20 13:08 ` [PATCH AUTOSEL 7.0-6.18] ALSA: hda/realtek: Add quirk for CSL Unity BF24B Sasha Levin
2026-04-20 13:08 ` [PATCH AUTOSEL 7.0-5.10] ALSA: aoa/onyx: Fix OF node leak on probe failure Sasha Levin

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