[PATCH AUTOSEL 7.0-5.10] media: pulse8-cec: Handle partial deinit

[Sasha Levin <sashal@kernel.org>]

From: Vicki Pfau <vi@endrift.com>

[ Upstream commit 323f52e02be68889c8630c4a0415ef5b78f9dc63 ]

In the event that the cec dev node is held open while the adapter is
disconnected the serio device will be cleaned up but the cec device won't
be. As the serio device is freed but the ping_eeprom_work is not canceled,
the next ping will still attempt to send, leading to a kernel oops.

This patch both cancels the ping_eeprom_work in the serio cleanup as well
as checking to make sure the serio is still present before attempting to
write to it. Note that while the added serio = NULL line looks similar to
one that was removed in commit 024e01dead12c ("media: pulse8-cec: fix
duplicate free at disconnect or probe error"), it notably happens before
calling cec_unregister_adapter, and as such shouldn't lead to the
user-after-free that removing it fixed.

Signed-off-by: Vicki Pfau <vi@endrift.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
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 my complete
analysis.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject Line
- **Subsystem**: media: pulse8-cec
- **Action verb**: "Handle" (indicates fixing incorrect handling - a bug
  fix indicator)
- **Summary**: Handle partial deinitialization when CEC device node is
  still open during adapter disconnect

### Step 1.2: Tags
- **Signed-off-by**: Vicki Pfau <vi@endrift.com> (author)
- **Signed-off-by**: Hans Verkuil <hverkuil+cisco@kernel.org> (subsystem
  maintainer)
- No Fixes: tag (expected for candidates under review)
- No Cc: stable (expected for candidates under review)
- No Reported-by (author likely found this themselves)

### Step 1.3: Commit Body Analysis
The commit clearly describes the bug: When the CEC device node is held
open while the USB adapter is disconnected, the serio device is cleaned
up but the CEC device is not (because a filehandle is still open). The
`ping_eeprom_work` is not cancelled, so the next periodic ping attempts
to write to the freed serio device, causing a **kernel oops**.

The author also carefully notes the relationship with commit
`024e01dead12c`, explaining that while the `serio = NULL` line looks
similar to one that was previously removed (because it caused a UAF when
placed *after* `cec_unregister_adapter`), this new placement is *before*
`cec_unregister_adapter`, avoiding that problem.

**Record**: Bug = kernel oops when adapter disconnected with CEC device
open; Symptom = oops on next ping; Root cause = ping_eeprom_work not
cancelled in disconnect, and serio pointer not invalidated.

### Step 1.4: Hidden Bug Fix Detection
This is explicitly a crash fix. The word "Handle" and the description of
"kernel oops" leave no ambiguity.

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
- **File**: `drivers/media/cec/usb/pulse8/pulse8-cec.c` (+7 lines)
- **Functions modified**:
  1. `pulse8_send_and_wait_once()` - added NULL check for
     `pulse8->serio`
  2. `pulse8_disconnect()` - added work cancellation and serio pointer
     invalidation
- **Scope**: Single-file, single-driver, surgical fix

### Step 2.2: Code Flow Changes

**Hunk 1** (`pulse8_send_and_wait_once`):
- **Before**: Directly accesses `pulse8->serio` to send data
- **After**: Checks `if (!pulse8->serio) return -ENODEV;` before
  accessing serio
- This is a safety check that prevents NULL dereference

**Hunk 2** (`pulse8_disconnect`):
- **Before**: Immediately calls `cec_unregister_adapter`, sets drvdata
  NULL, closes serio
- **After**: First cancels `ping_eeprom_work`, then sets `pulse8->serio
  = NULL` under mutex lock, then proceeds with existing cleanup
- This ensures no in-flight work can access the freed serio device

### Step 2.3: Bug Mechanism
This is a **use-after-free / NULL pointer dereference** fix. The race
condition:
1. Userspace has `/dev/cecX` open
2. USB adapter is disconnected -> `pulse8_disconnect()` runs
3. `cec_unregister_adapter()` does NOT free the pulse8 struct because a
   filehandle is open (deferred via refcount)
4. `serio_close()` tears down the serio
5. `ping_eeprom_work` fires -> calls `pulse8_send_and_wait()` ->
   `pulse8_send_and_wait_once()` -> dereferences `pulse8->serio` ->
   **OOPS** (freed memory)

### Step 2.4: Fix Quality
- **Obviously correct**: Yes. The fix cancels the work before serio
  teardown, sets serio=NULL under the existing mutex, and adds a NULL
  check in the function that all callers invoke under the same mutex.
- **Minimal/surgical**: Yes, 7 lines added.
- **Regression risk**: Very low. Setting serio=NULL before
  `cec_unregister_adapter` (not after) avoids the UAF that the earlier
  commit `024e01dead12c` fixed. The author explicitly addresses this.

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
The disconnect function was introduced in `cea28e7a55e7af` (2019-12-11).
The `pulse8_send_and_wait_once` function dates from the same commit.
This code has been in the tree since kernel v5.5 era, affecting all
active stable trees.

### Step 3.2: Fixes Tag Analysis
No Fixes: tag, but the commit references `024e01dead12c` which was a
prior fix to a related UAF issue in the disconnect path. That commit was
explicitly Cc: stable and is present in stable trees.

### Step 3.3: File History
The file has had limited changes over the years. Most recent substantive
changes:
- `92cbf865ea2e0` - handle possible ping error (2023)
- `024e01dead12c` - fix duplicate free at disconnect (2020)
- `aa9eda76129c` - close serio in disconnect, not adap_free (2020)

### Step 3.4: Author Context
Vicki Pfau is a known kernel contributor (10 commits visible in this
tree, primarily HID and input). Hans Verkuil, who signed off, is the CEC
subsystem maintainer and original author of the pulse8-cec driver.

### Step 3.5: Dependencies
This fix is standalone. It does not depend on any other uncommitted
patches. The code structure it modifies (disconnect function,
send_and_wait_once) has been stable since 2020.

## PHASE 4: MAILING LIST RESEARCH

### Step 4.1: Discussion
The commit was found in the linuxtv-commits mailing list, posted
2026-03-09. Hans Verkuil (subsystem maintainer) committed it directly.
No objections or NAKs were found.

### Step 4.2: Review
The patch was signed off by Hans Verkuil, the CEC subsystem maintainer
and original pulse8-cec author - this carries significant weight.

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key Functions
- `pulse8_send_and_wait_once()` - core send function
- `pulse8_disconnect()` - serio disconnect callback

### Step 5.2: Callers of `pulse8_send_and_wait_once`
Called through `pulse8_send_and_wait()` from:
- `pulse8_tx_work_handler()` (line 295: holds mutex)
- `pulse8_cec_adap_enable()` (line 488: holds mutex)
- `pulse8_cec_adap_log_addr()` (line 509: holds mutex)
- `pulse8_ping_eeprom_work_handler()` (line 810: holds mutex)
- `pulse8_setup()` - only during probe, no race

All runtime callers hold `pulse8->lock` mutex, which means the fix's
`serio = NULL` under mutex + NULL check provides proper synchronization.

### Step 5.3-5.5: Impact Surface
The `ping_eeprom_work` fires every 15 seconds (`PING_PERIOD = 15 * HZ`).
This means within 15 seconds of disconnecting the adapter while the CEC
device node is open, a kernel oops will occur. This is highly
reproducible.

## PHASE 6: STABLE TREE ANALYSIS

### Step 6.1: Buggy Code in Stable
The buggy code has been present since the `601282d65b96` commit (v5.5
era, 2019) which introduced the `adap_free` callback pattern. The
`ping_eeprom_work` not being cancelled in disconnect has been a latent
bug since then. This affects all active stable trees (5.15.y, 6.1.y,
6.6.y, 6.12.y, 7.0.y).

### Step 6.2: Backport Complications
The patch should apply cleanly. The file has had minimal changes (only
the `kzalloc_obj` treewide conversion and a timestamp fix) since the
relevant code was last modified. Minor fuzz at most.

### Step 6.3: Related Fixes in Stable
Commit `024e01dead12c` (fixing a related UAF in disconnect) is already
in stable and is a prerequisite that is already present.

## PHASE 7: SUBSYSTEM CONTEXT

### Step 7.1: Subsystem Criticality
- **Subsystem**: drivers/media/cec/usb/pulse8 - USB CEC adapter driver
- **Criticality**: PERIPHERAL (specific hardware), but the Pulse-Eight
  adapter is a popular CEC adapter used by many home theater setups and
  HTPC users

### Step 7.2: Subsystem Activity
Low activity - this is a mature driver with infrequent changes, meaning
the bug has been latent for years.

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Affected Users
Users of the Pulse-Eight USB CEC adapter who disconnect the adapter
while the CEC device node is held open by a CEC-aware application (e.g.,
libcec, Kodi).

### Step 8.2: Trigger Conditions
- User unplugs the Pulse-Eight adapter while a CEC application has
  `/dev/cecX` open
- The oops occurs within 15 seconds (next ping period)
- No privilege required - any user with access to the CEC device can
  trigger this
- Highly reproducible

### Step 8.3: Failure Mode
**Kernel oops** (NULL pointer dereference or use-after-free of the serio
device). Severity: **CRITICAL** - kernel crash.

### Step 8.4: Risk-Benefit Ratio
- **Benefit**: HIGH - prevents a kernel oops on a common user operation
  (unplugging USB device)
- **Risk**: VERY LOW - 7-line change, obviously correct, touches only
  error/teardown paths, signed off by subsystem maintainer

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence Compilation
**FOR backporting**:
- Fixes a kernel oops (crash) - critical bug
- Highly reproducible (unplug with open filehandle, wait 15 seconds)
- Small, surgical fix (7 lines added)
- Single file, single driver
- Signed off by subsystem maintainer (Hans Verkuil)
- The fix is obviously correct (cancel work + NULL guard under existing
  mutex)
- Buggy code exists in all stable trees (since v5.5)
- No dependencies on other uncommitted patches

**AGAINST backporting**:
- None significant

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** - simple cancel + NULL check
   pattern, signed by maintainer
2. Fixes a real bug? **YES** - kernel oops on adapter disconnect
3. Important issue? **YES** - kernel crash
4. Small and contained? **YES** - 7 lines, one file
5. No new features or APIs? **YES** - purely defensive checks
6. Can apply to stable? **YES** - should apply cleanly

### Step 9.3: Exception Categories
Not needed - this qualifies as a standard important bug fix.

## Verification

- [Phase 1] Parsed tags: Signed-off-by from Vicki Pfau (author) and Hans
  Verkuil (subsystem maintainer)
- [Phase 2] Diff analysis: 7 lines added across 2 hunks: NULL check in
  `pulse8_send_and_wait_once()` and work cancellation + serio=NULL in
  `pulse8_disconnect()`
- [Phase 3] git blame: buggy code in `pulse8_disconnect()` introduced in
  `cea28e7a55e7af` (2019-12-11), present in all stable trees
- [Phase 3] git show 024e01dead12c: confirmed related prior fix
  (duplicate free) is already in stable trees, Cc: stable explicitly
- [Phase 3] git show aa9eda76129c: confirmed the serio close was moved
  back to disconnect in 2020
- [Phase 3] git show 601282d65b96: confirmed adap_free callback pattern
  introduced the deferred cleanup that creates the race window
- [Phase 4] linuxtv-commits archive: found commit notification, signed
  by Hans Verkuil (maintainer), no objections
- [Phase 5] Grep callers: all runtime callers of
  `pulse8_send_and_wait_once` hold `pulse8->lock` mutex, confirming the
  fix's synchronization is correct
- [Phase 5] Verified `pulse8_ping_eeprom_work_handler` acquires mutex at
  line 810 before calling `pulse8_send_and_wait` at line 812
- [Phase 5] Verified re-schedule at line 831 happens before mutex
  unlock, but after the `cancel_delayed_work_sync` in the fix, no new
  work can be scheduled
- [Phase 6] File has had minimal changes - patch should apply cleanly to
  stable trees
- [Phase 8] Failure mode: kernel oops from accessing freed serio device,
  severity CRITICAL

**YES**

 drivers/media/cec/usb/pulse8/pulse8-cec.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/cec/usb/pulse8/pulse8-cec.c b/drivers/media/cec/usb/pulse8/pulse8-cec.c
index 0df3af152762c..fa5df10627539 100644
--- a/drivers/media/cec/usb/pulse8/pulse8-cec.c
+++ b/drivers/media/cec/usb/pulse8/pulse8-cec.c
@@ -235,6 +235,9 @@ static int pulse8_send_and_wait_once(struct pulse8 *pulse8,
 {
 	int err;
 
+	if (!pulse8->serio)
+		return -ENODEV;
+
 	if (debug > 1)
 		dev_info(pulse8->dev, "transmit %s: %*ph\n",
 			 pulse8_msgname(cmd[0]), cmd_len, cmd);
@@ -655,6 +658,10 @@ static void pulse8_disconnect(struct serio *serio)
 {
 	struct pulse8 *pulse8 = serio_get_drvdata(serio);
 
+	cancel_delayed_work_sync(&pulse8->ping_eeprom_work);
+	mutex_lock(&pulse8->lock);
+	pulse8->serio = NULL;
+	mutex_unlock(&pulse8->lock);
 	cec_unregister_adapter(pulse8->adap);
 	serio_set_drvdata(serio, NULL);
 	serio_close(serio);
-- 
2.53.0