[PATCH AUTOSEL 6.18-5.10] comedi: pcmmio: Add sanity check to interrupt handler
From: Sasha Levin
Date: Mon Oct 19 2026 - 11:17:00 EST
From: Ian Abbott <abbotti@xxxxxxxxx>
[ Upstream commit ea72e2fc9026f375314421ddfa4dbdc49432a899 ]
The driver requests an interrupt handler for the device, after setting
device registers to disable interrupt generation. The interrupt handler
should not be called prematurely unless the user-configured I/O port
base address and/or IRQ number are incorrect or the hardware is bad.
For safety, check the dev->attached flag in the interrupt handler to
ensure the device has been fully set up, avoiding a possible null
pointer dereference of dev->read_subdev.
Reported-by: Jaeyoung Chung <jjy600901@xxxxxxxxx>
Link: https://lore.kernel.org/lkml/20260610115912.780131-1-jjy600901@xxxxxxxxx/
Reported-by: Sangyun Kim <sangyun.kim@xxxxxxxxx>
Reported-by: Kyungwook Boo <bookyungwook@xxxxxxxxx>
Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
Link: https://patch.msgid.link/20260618102949.26607-10-abbotti@xxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
LLM Generated explanations, may be completely bogus:
This is a 3-line IRQ-handler guard against a KASAN NULL deref in an
existing COMEDI driver. The local tree is **Linux 7.3.0-rc4**
(`v7.3-rc4-27-gf0100363d8c37`). The driver and the buggy attach/IRQ
ordering are present here; the patch is self-contained.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[comedi: pcmmio:] [Add] [sanity check in `interrupt_pcmmio()`
so a premature IRQ cannot dereference a NULL `dev->read_subdev`]**
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Fixes:** none (expected for this review)
- **Cc: stable:** none (expected; not a negative signal)
- **Reported-by:** Jaeyoung Chung `<jjy600901@xxxxxxxxx>`; Sangyun Kim
`<sangyun.kim@xxxxxxxxx>`; Kyungwook Boo `<bookyungwook@xxxxxxxxx>`
(three reporters)
- **Link:** https://lore.kernel.org/lkml/20260610115912.780131-1-
jjy600901@xxxxxxxxx/ (original KASAN/NULL-deref report)
- **Link:**
https://patch.msgid.link/20260618102949.26607-10-abbotti@xxxxxxxxx
(patch 09/11 of Ian Abbott’s series; msgid `-10` is 0-based git-send-
email numbering)
- **Signed-off-by:** Ian Abbott (author, COMEDI maintainer); Greg Kroah-
Hartman (COMEDI co-maintainer / applied the patch)
- Ignore pipeline SOBs/upstream markers if present.
Notable: three reporters and an explicit KASAN NULL-deref report that
names `pcmmio.c`.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `pcmmio_attach()` calls `request_irq()` after disabling board
interrupt generation, but **before** subdevices are allocated and
before `dev->read_subdev` is set. `dev->attached` is only set later in
`comedi_device_postconfig()`.
- **Symptom:** premature IRQ → `interrupt_pcmmio()` uses a NULL
`read_subdev` → NULL deref in `pcmmio_handle_dio_intr()`.
- **Trigger (author):** should not happen on good hardware with a
correct I/O base and IRQ; can happen if the user-configured I/O
base/IRQ is wrong or the hardware is bad.
- **Root cause:** IRQ handler registered before the comedi device is
fully set up.
- **Version info:** none in the message. Driver dates to 2009.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not a disguised cleanup.** Subject says “Add sanity check”;
body describes a NULL deref. Cover letter: “Add/fix sanity check in ISA
drivers to avoid null-ptr-deref.”
===============================================================
PHASE 2: DIFF ANALYSIS
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `drivers/comedi/drivers/pcmmio.c` — **+3 / −0**
- **Function:** `interrupt_pcmmio()`
- **Scope:** single-file, surgical
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** handler always copies `s = dev->read_subdev`, then `inb()`
of the pending register; if any pending bits are set, calls
`pcmmio_handle_dio_intr(dev, s, triggered)`.
- **After:** if `!dev->attached`, return `IRQ_NONE` before touching the
pending register or `s`.
- **Path:** IRQ during attach (and any other time `attached` is false).
Not a new userspace API.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[NULL pointer dereference / memory safety]**
- `pcmmio_handle_dio_intr()` does `struct comedi_cmd *cmd =
&s->async->cmd;` as its first real use of `s`. If `s` is NULL, that is
a kernel oops.
- `s = dev->read_subdev` is only a pointer copy; the crash is the later
use. The attached check sits before that use.
- `pcmmio_attach()` order (verified in this tree): `pcmmio_reset()` →
`request_irq(..., interrupt_pcmmio, ...)` →
`comedi_alloc_subdevices()` → `dev->read_subdev = s`.
`comedi_device_attach()` then calls `comedi_device_postconfig()`,
which sets `dev->attached = true` last.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** matches the long-standing COMEDI pattern (`if
(!dev->attached) return IRQ_NONE;`) used in many drivers (`das800`,
`comedi_parport`, `ni_660x`, etc.).
- **Minimal:** three lines, no API/data-structure change.
- **Regression risk:** very low. Returning `IRQ_NONE` for an unattached
device is correct (this IRQ is requested with flags `0`, not shared).
- **Not a follow-up candidate:** sibling “Fix sanity check” patches
moved an *existing* attached test *before* a real dereference
(`s->async->cmd`). Here the new test is before `s` is used. No later
pcmmio follow-up exists on this file.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Handler introduced with the driver: `6baef150380d5` (“Staging: comedi:
add pcmmio and pcmuio drivers”, 2009-04-03, contained in v2.6.30-rc1).
- `s = dev->read_subdev` from `19e0bf12358e59` (2013).
- Driver left staging in `8ffdff6a8cfbd` (2021-04-15, v5.13).
- Buggy ordering (IRQ before `read_subdev`) has been in this code for
many years, including this 7.3 tree.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Bug is the original attach/IRQ ordering, not
a recent regression.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Same-file recent commit: `78ba300a99917` “comedi: pcmmio: Add sanity
checks for I/O base address” — **independent**; this IRQ patch does
not use that helper.
- This commit is standalone. Cover letter lists it as **9/11**; each
patch is a different driver. No shared helper, no data-structure
prerequisite.
- No later pcmmio commits after `ea72e2fc9026f` on this file.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Ian Abbott is **the COMEDI maintainer**. Same-day series
adds/fixes the same attached check in aio_iiro_16, das6402, dt2811,
ni_at_a2150, ni_atmio16d, pcl711, pcl816, pcl818, pcmmio, pcmuio,
quatech_daqp_cs. Earlier `17221216ae8ce` (“comedi: comedi_parport: deal
with premature interrupt”) is the same class, and that one had syzbot +
`Cc: stable`.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: **None required.** `dev->attached` and `interrupt_pcmmio()`
exist in this tree. Patch applies as a 3-line insertion.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- **b4 dig:**
https://patch.msgid.link/20260618102949.26607-10-abbotti@xxxxxxxxx
- **Series:** v1 only (`[PATCH 01/11]` … `11/11`), 2026-06-18. No v2/v3.
- **Cover letter:** “comedi: Add/fix sanity check in ISA drivers to
avoid null-ptr-deref”. Names pcmmio among the reported drivers.
- **Mbox:** 12 messages, all from the author (cover + 11 patches). **No
replies, no NAK, no reviewer Cc: stable.** Greg applied the series.
- Lore HTML fetch returned **403**; discussion recovered via `b4 dig -m`
and `b4 mbox`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (**b4 dig -w**): To/Cc were Ian Abbott, linux-kernel, Greg Kroah-
Hartman, H Hartley Sweeten, and the three reporters. Maintainers were on
the original mail. No extra Reviewed-by/Acked-by.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record (original mail, msgid
`20260610115912.780131-1-jjy600901@xxxxxxxxx`):
- Subject: **“comedi: KASAN null-ptr-deref in ISA driver IRQ handlers on
early IRQ”**
- Explicitly lists **pcmmio.c / `interrupt_pcmmio()` /
`pcmmio_attach()`**
- Mechanism: `request_irq()` before `dev->read_subdev = s`; handler then
uses a NULL subdevice
- Suggested fix: move `request_irq()` later **or** guard the handler
- Ian chose the established attached-flag guard (same as many other
COMEDI drivers)
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: 11 independent per-driver patches. This one does not need the
others. The parport sibling (`17221216ae8ce`) already has `Cc: stable`
because syzbot hit `comedi_parport`. Same bug class.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: **UNVERIFIED** for lore.kernel.org/stable (HTTP 403). The
submission mbox has no stable-list discussion.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `interrupt_pcmmio()` modified; callees of interest:
`pcmmio_handle_dio_intr()`, `pcmmio_attach()`.
**Step 5.2: TRACE CALLERS**
Record:
- `interrupt_pcmmio` is registered only from `pcmmio_attach()` via
`request_irq(it->options[1], interrupt_pcmmio, 0, ...)`.
- Attach is reached from `comedi_device_attach()` ←
`do_devconfig_ioctl()` (COMEDI_DEVCONFIG). That ioctl requires
**`CAP_SYS_ADMIN`**.
- Context: hard IRQ. Not a hot syscall path for unprivileged users.
**Step 5.3: TRACE CALLEES**
Record: `inb()` of `PCMMIO_INT_PENDING_REG`; `pcmmio_dio_read/write`;
`pcmmio_handle_dio_intr()` which immediately does `&s->async->cmd`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record:
- Reachable from userspace **with CAP_SYS_ADMIN**: load `pcmmio`,
`COMEDI_DEVCONFIG` with an IRQ in `options[1]`.
- Crash window: IRQ between `request_irq()` and `dev->attached = true`.
- If the pending register reads as zero, the old handler returned
`IRQ_NONE` without using `s`. The NULL deref needs a premature IRQ
**and** a non-zero pending read (wrong I/O base returning garbage, or
bad/mis-routed hardware). That is exactly the ISA comedi_config model,
and it is how syzbot hit parport.
- **Not** unprivileged-triggerable. Still a real kernel oops on a
supported ioctl.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: dozens of COMEDI IRQ handlers already check `dev->attached`.
This is aligning pcmmio with that pattern, not inventing a new one.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.**
- Tree: **7.3.0-rc4** (mainline-like checkout, detached HEAD).
- `drivers/comedi/drivers/pcmmio.c` exists; `pcmmio_attach()` still
requests the IRQ before `read_subdev` and before postconfig sets
`attached`.
- Commit `ea72e2fc9026f` **is** an ancestor of HEAD (`git merge-base
--is-ancestor` exit 0). That is expected for a mainline candidate; it
is **not** used as “already selected for stable.”
- Bug introduced ~2009, not after this tree branched.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply.** Hunk is a 3-line insert into a handler whose
shape has been stable since 2013. No refactor of this function in this
tree beyond this commit.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This is the fix. No alternate pcmmio IRQ NULL-deref fix. The
I/O-base check (`78ba300a99917`) is a different issue and is not
required for this hunk.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **[drivers/comedi — COMEDI DAQ]** **[PERIPHERAL]**
Winsystems PCM-MIO PC/104 board (`CONFIG_COMEDI_PCMMIO`). Niche
industrial hardware, but a real in-tree driver (out of staging since
v5.13).
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature. Ian Abbott still actively fixing ISA attach/IRQ races in
2026. Not staging.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **driver-specific / config-specific** — users who load `pcmmio`
and attach it with an IRQ via comedi_config.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Trigger: attach with `options[1]` (IRQ) set; IRQ fires before
`attached`; pending register reads non-zero.
- Likelihood on correctly jumpered good hardware: low (driver disables
interrupt generation before `request_irq()`).
- Likelihood with wrong I/O base/IRQ (admin ioctl, fuzzer, bad card):
realistic. Original report treated it as a KASAN NULL deref across
several ISA drivers, including this one.
- Unprivileged: **no** (CAP_SYS_ADMIN for COMEDI_DEVCONFIG).
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Kernel oops / KASAN null-ptr-deref** → **HIGH** (crash of the
machine or at least the IRQ path; not data corruption on a mounted fs,
but a serious crash).
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** prevents a documented NULL deref oops on a long-lived
driver. High for anyone who hits it; population is small.
- **Risk:** very low (3 lines, established pattern, IRQ_NONE on
unattached device).
- **Ratio:** favorable. Small crash fixes in obscure drivers are
standard stable material.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
**FOR backporting:**
- Real NULL deref in an IRQ handler; original report is titled as KASAN
null-ptr-deref and names this file.
- Three reporters; maintainer-authored; applied by Greg KH.
- 3-line, obviously correct, same pattern as many other COMEDI drivers.
- Buggy ordering has been in the driver since 2009; present in this
tree.
- Standalone; no prerequisites; clean apply.
- Sibling parport fix of the same class was nominated for stable
(syzbot).
- Sanitizer-class crash; stable rules treat serious crashes as in-scope
even for niche drivers.
**AGAINST:**
- Niche PC/104 board; interrupt support in the driver comments is
“untested.”
- Author frames it as “for safety” and says a premature IRQ needs bad
config/hardware.
- Needs CAP_SYS_ADMIN to attach.
- No Fixes:/Cc: stable on this specific commit (absence is expected
here; still a weaker signal than parport).
- No mailing-list review replies.
**UNRESOLVED:**
- Could not search lore.kernel.org/stable (403).
- Original report has no pcmmio-specific reproducer/stack in the mail
body (code analysis + KASAN class).
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — 3-line attached check,
established COMEDI idiom; applied by subsystem maintainers.
2. Fixes a real bug that affects users? **PASS** — NULL deref if IRQ
runs before attach completes; reported against this driver.
3. Important issue? **PASS** — kernel oops / KASAN null-ptr-deref (HIGH,
not a core-mm panic, but a serious crash).
4. Small and contained? **PASS** — +3 lines, one function.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — file and handler exist; hunk
is trivial.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: none (not a device ID, quirk, DT, build, or docs exception). It
qualifies as a normal crash fix.
**Step 9.4: MAKE YOUR DECISION**
This tree has the driver, the attach/IRQ race, and the NULL deref path.
The patch is a minimal, standard guard. Privilege and niche hardware
lower the blast radius; they do not make a KASAN NULL deref in an IRQ
handler unsuitable for stable.
===============================================================
What the commit solves, and why it matters here
===============================================================
`pcmmio_attach()` registers `interrupt_pcmmio` before subdevices exist
and before `dev->attached` is set. If that handler runs in that window
and the pending register is non-zero, `pcmmio_handle_dio_intr()`
dereferences a NULL `read_subdev` (`s->async->cmd`) and oopses. The
patch returns `IRQ_NONE` until the device is fully attached.
That matches the original SNU report and the usual COMEDI IRQ-safety
pattern. For this 7.3 checkout the code exists, the fix is three lines,
and regression risk is negligible.
===============================================================
Verification
===============================================================
- **[Phase 1]** Parsed tags: three Reported-by; two Links; no Fixes:/Cc:
stable; SOBs Ian Abbott + Greg KH.
- **[Phase 1]** Cover letter (mbox): series is explicitly “avoid null-
ptr-deref”; pcmmio is listed.
- **[Phase 2]** Diff: +3 lines in `interrupt_pcmmio()` only.
- **[Phase 2]** Read `pcmmio_attach()`: `request_irq` at line 688,
`read_subdev` at 749; `comedi_device_postconfig()` sets `attached`
after `attach()` returns.
- **[Phase 2]** Read `pcmmio_handle_dio_intr()`: first use of `s` is
`&s->async->cmd`.
- **[Phase 3]** `git blame`: handler from 2009 (`6baef150380d5`); this
check from `ea72e2fc9026f`.
- **[Phase 3]** `git log --oneline -20 --
drivers/comedi/drivers/pcmmio.c`: no later pcmmio follow-up.
- **[Phase 3]** No Fixes: tag to follow.
- **[Phase 4]** `b4 dig -c ea72e2fc9026f`: lore URL
`20260618102949.26607-10-abbotti@xxxxxxxxx`.
- **[Phase 4]** `b4 dig -a`: v1 series only, 11 patches.
- **[Phase 4]** `b4 dig -w`: Greg KH + Hartley Sweeten + reporters on
Cc.
- **[Phase 4]** `b4 mbox` of
`20260610115912.780131-1-jjy600901@xxxxxxxxx`: KASAN report names
`interrupt_pcmmio`.
- **[Phase 4]** UNVERIFIED: lore.kernel.org HTML and stable@ search
(403).
- **[Phase 5]** Callers: only `request_irq` from `pcmmio_attach`; ioctl
path needs CAP_SYS_ADMIN (`do_devconfig_ioctl`).
- **[Phase 6]** `git describe HEAD` / `make kernelversion`:
**7.3.0-rc4**.
- **[Phase 6]** `git merge-base --is-ancestor ea72e2fc9026f HEAD`: yes
(candidate is in this history).
- **[Phase 8]** Failure mode: NULL deref oops in IRQ handler; severity
HIGH.
- **UNVERIFIED:** live reproduction on PCM-MIO hardware (not available).
The fix is small, surgical, and meets the stable criteria for a real
crash.
**YES**
drivers/comedi/drivers/pcmmio.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/comedi/drivers/pcmmio.c b/drivers/comedi/drivers/pcmmio.c
index c2402239d5513..db9cb3fedb48d 100644
--- a/drivers/comedi/drivers/pcmmio.c
+++ b/drivers/comedi/drivers/pcmmio.c
@@ -362,6 +362,9 @@ static irqreturn_t interrupt_pcmmio(int irq, void *d)
unsigned int triggered;
unsigned char int_pend;
+ if (!dev->attached)
+ return IRQ_NONE;
+
/* are there any interrupts pending */
int_pend = inb(dev->iobase + PCMMIO_INT_PENDING_REG) & 0x07;
if (!int_pend)
--
2.53.0