[PATCH AUTOSEL 6.18-5.10] comedi: ni_at_a2150: Fix sanity check in interrupt handler

[Sasha Levin]

From: Ian Abbott <abbotti@xxxxxxxxx>

[ Upstream commit e7356cdc7077a4f30fc5232a18e9fc6a4d404581 ]

The driver requests an interrupt handler for the device before it is
fully set up.

For safety, the interrupt handler checks the dev->attached flag to
ensure the device is fully set up, but it currently does that after
dereferencing various pointers which may be NULL if dev->attached is
false.  Move the check to avoid the possible null pointer dereferences.

Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
Link: https://patch.msgid.link/20260618102949.26607-5-abbotti@xxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit message forensics

**Step 1.1:** Record: `[comedi: ni_at_a2150:] [Fix] [Move the
`dev->attached` check in `a2150_interrupt()` so it runs before pointer
dereferences that can be NULL while the device is still attaching.]`

**Step 1.2:** Tags in `e7356cdc7077a`:
- `Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>` (author; comedi
  maintainer)
- `Link:
  https://patch.msgid.link/20260618102949.26607-5-abbotti@xxxxxxxxx`
- `Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>`
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
  by:`, or `Cc: stable` on the committed message (absence is expected
  here)

Mailbox recipients of the original mail also included H Hartley Sweeten,
Jaeyoung Chung, and a malformed Cc that was meant to be `Reported-by:
Sangyun Kim` plus Kyungwook Boo. Those trailers were not copied onto the
git commit.

**Step 1.3:** Record:
- Bug: the ISR is registered in `a2150_alloc_irq_and_dma()` during
  `a2150_attach()`, before postconfig sets `dev->attached`.
- Symptom: NULL pointer dereference in interrupt context if the handler
  runs while `dev->attached` is still false.
- Version info: none in the commit body.
- Root cause: the existing `if (!dev->attached)` guard sits *after*
  `dma->desc[0]`, `s->async`, `async->cmd`, and `desc->virt_addr`.

Cover letter for the 11-patch series: “Add/fix sanity check in ISA
drivers to avoid null-ptr-deref”.

**Step 1.4:** Record: Not a hidden fix. Subject and body are explicit
NULL-deref prevention.

## Phase 2: Diff analysis

**Step 2.1:** Record:
- File: `drivers/comedi/drivers/ni_at_a2150.c` (+9 / −4; 13 lines
  changed)
- Function: `a2150_interrupt()`
- Scope: single-file surgical ISR change

**Step 2.2:** Record: Before, locals were initialized with dereferences,
then `dev->attached` was checked. After, `desc`/`async`/`cmd`/`buf` are
declared uninitialized, the attached check runs, then those pointers are
filled. Normal attached-and-running IRQ path is unchanged.

**Step 2.3:** Record: `[NULL pointer dereference] [ISR used `dma`,
`read_subdev`, and `async` before the attached guard; those can still be
NULL between `request_irq()` and `dev->attached = true`, and `dma` can
already have been freed on detach before `free_irq()`.]`

Verified attach/detach order in this tree:
1. `comedi_alloc_devpriv()` then `a2150_alloc_irq_and_dma()` →
   `request_irq(a2150_interrupt, …)` then DMA alloc, then later
   `comedi_alloc_subdevices()` / `dev->read_subdev = s`.
2. `comedi_device_attach()` only then calls
   `comedi_device_postconfig()`, which sets `dev->attached = true`.
3. Detach sets `dev->attached = false` first, then `a2150_detach()`
   frees DMA, then `comedi_legacy_detach()` calls `free_irq()`.

So a premature IRQ during attach hits NULL `dma` or NULL `s`/`s->async`.
An IRQ between DMA free and `free_irq()` hits a freed `dma` (UAF) in the
old code.

Loads that remain before the check (`dev->private`, `devpriv->dma`,
`dev->read_subdev`) are pointer loads, not the crashing derefs.
`private` is allocated before `request_irq()`, and it is still valid
until after `free_irq()`.

**Step 2.4:** Record: Obviously correct and minimal. Regression risk is
very low: early return already existed; only the check moved. No
API/locking/data-structure change. Residual theoretical issue: if
`attached` were true with `dma` or `read_subdev` still NULL, later
derefs would still fault — that is not the attach/detach window this
patch targets, and `dev->irq` is only kept when DMA alloc succeeded.

## Phase 3: Git history

**Step 3.1:** Record: `if (!dev->attached)` is old (Frank Mori Hess,
2009). The dangerous pre-check initializers come from `1a97f144e3d3e9`
(“staging: comedi: ni_at_a2150: remove depends on ISA_DMA_API
limitation”, H Hartley Sweeten, 2015-01-14), first contained in
`v4.0-rc1`. Present in this tree (`git merge-base --is-ancestor
1a97f144e3d3e9 HEAD`).

**Step 3.2:** Record: No `Fixes:` tag. Introduced by `1a97f144e3d3e9`
(v4.0), which is in this checkout.

**Step 3.3:** Record: This is patch 4/11 of Ian Abbott’s June 2026
series. Neighbor commits are the same class of per-driver ISR guards
(`dt2811`, `aio_iiro_16`, `das6402`, `pcl711`, etc.). Independent; no
shared helper or prerequisite refactor. No later follow-up on
`ni_at_a2150.c`.

**Step 3.4:** Record: Ian Abbott is the long-time comedi maintainer.
Greg Kroah-Hartman applied the series. Related work: `17221216ae8ce`
(“comedi: comedi_parport: deal with premature interrupt”) for the same
premature-IRQ class, with syzbot + `Cc: stable`.

**Step 3.5:** Record: No extra commits required. The ISR,
`dev->attached`, `request_irq`, and `comedi_legacy_detach()` already
exist here. Standalone.

## Phase 4: Mailing list / external research

**Step 4.1:** Record:
- `b4 dig -c e7356cdc7077a` →
  https://patch.msgid.link/20260618102949.26607-5-abbotti@xxxxxxxxx
- `b4 dig -a`: v1 only (no v2/v3)
- Cover letter: “comedi: Add/fix sanity check in ISA drivers to avoid
  null-ptr-deref”
- No NAKs in the saved thread (12 series mails, no review replies)

**Step 4.2:** Record (`b4 dig -w`): linux-kernel, Greg KH, Hartley
Sweeten, Jaeyoung Chung, Sangyun Kim, Kyungwook Boo. Maintainer-
authored, applied by Greg.

**Step 4.3:** Record: Original report retrieved with `b4 mbox` msgid
`20260610115912.780131-1-jjy600901@xxxxxxxxx`:
- Subject: “comedi: KASAN null-ptr-deref in ISA driver IRQ handlers on
  early IRQ”
- From Jaeyoung Chung; Reported-by Sangyun Kim and Kyungwook Boo
- Failure: handler runs before `read_subdev` is set → KASAN NULL deref /
  panic
- Named drivers: das6402, ni_atmio16d, aio_iiro_16, pcmmio,
  comedi_parport, pcl711
- ni_at_a2150 was not in that list; Ian added it after finding the same
  `request_irq` / `read_subdev` / attached-check ordering

Related: `comedi_parport` premature IRQ was a syzbot GPF in
`comedi_get_is_subdevice_running()`.

**Step 4.4:** Record: 11 independent per-driver patches. This one does
not need the others.

**Step 4.5:** Record: UNVERIFIED — lore.kernel.org returned HTTP 403 for
`/stable/` search. No stable-list discussion confirmed.

## Phase 5: Code semantic analysis

**Step 5.1:** Record: `a2150_interrupt()` only.

**Step 5.2:** Record: Sole registration is `request_irq()` in
`a2150_alloc_irq_and_dma()`, called from `a2150_attach()`. Hardirq
context. Userspace can trigger attach/detach via comedi config
(`COMEDI_NI_AT_A2150`).

**Step 5.3:** Record: After the guard: `inw(STATUS_REG)`,
`comedi_handle_events()`, `comedi_isadma_disable()`, sample copy into
the async buffer.

**Step 5.4:** Record: Reachable from the comedi attach ioctl / config
path on ISA AT-A2150 (or a crafted I/O base/IRQ). Same class as the
KASAN/syzbot premature-IRQ bugs. Not a hot syscall path for typical
machines.

**Step 5.5:** Record: Same `if (!dev->attached)` pattern across many
`drivers/comedi/drivers/*.c` files; this series fixes the ones that
checked too late or not at all.

## Phase 6: Local tree (this checkout)

**Step 6.1:** Record:
- `make kernelversion` / Makefile: **7.3.0-rc4**
- `git describe HEAD`: `v7.3-rc4-27-gf0100363d8c37`
- File and ISR exist. Pre-fix ISR confirmed at `e7356cdc7077a^`. Bug
  dates to v4.0 (`1a97f144e3d3e9`). This is mainline, not a 6.x.y stable
  branch; the driver code the patch targets is present.

**Step 6.2:** Record: Clean apply on the pre-image in this history. No
extra backport rework needed for a tree that still has the old ISR.

**Step 6.3:** Record: No earlier ni_at_a2150-specific substitute.
`comedi_parport` got a related premature-IRQ fix; it does not cover this
driver.

## Phase 7: Subsystem / maintainer

**Step 7.1:** Record: `[drivers/comedi — ISA DAQ] [PERIPHERAL]`. Out of
staging (`8ffdff6a8cfbd`). Hardware is old NI AT-A2150 ISA cards
(`CONFIG_COMEDI_NI_AT_A2150`).

**Step 7.2:** Record: Mature, low churn. Ian Abbott still actively
fixing ISR attach races in 2026.

## Phase 8: Impact and risk

**Step 8.1:** Record: Driver-specific, `CONFIG_COMEDI_NI_AT_A2150`.
Small user set (legacy ISA DAQ).

**Step 8.2:** Record: Trigger is attach (IRQ registered before
`attached`/`read_subdev`) or detach (DMA freed before `free_irq()`).
Needs the device (or a spurious/pending IRQ on that line). Not a general
unprivileged remote exploit; privileged/local comedi attach is enough if
the IRQ fires. Likelihood is low, but the window is real and matches a
KASAN report for the same pattern.

**Step 8.3:** Record: `[NULL deref / possible UAF in hardirq →
oops/panic] [HIGH]` (CRITICAL for that machine; not data corruption).

**Step 8.4:** Record: Benefit HIGH for anyone using this driver (avoids
IRQ oops on attach/detach). Risk very low (13-line ISR reorder). Ratio
favors backport.

## Phase 9: Synthesis

**Evidence FOR:**
- Real NULL deref (and detach UAF) in an interrupt handler
- Same class as a KASAN report and a syzbot GPF on sibling comedi
  drivers
- Author is the comedi maintainer; Greg applied v1 unchanged
- Small, obviously correct, no new API
- Bug present since v4.0; code exists in this tree
- Independent of the rest of the series

**Evidence AGAINST:**
- Niche obsolete ISA hardware
- Not in the original SNU driver list (Ian’s extra finding)
- Committed message has no `Fixes:` / `Cc: stable` (expected for this
  review)
- No Tested-by on hardware in the thread

**UNRESOLVED:**
- Could not read lore.kernel.org in the browser (403); used `b4` mboxes
  instead
- No confirmed stable@ discussion
- No hardware Tested-by

**Stable rules:**
1. Obviously correct and tested? **PASS** — logic is local and matches
   core `attached` semantics; applied by Greg. No extra hardware Tested-
   by, but the defect is visible in code.
2. Real user-visible bug? **PASS** — KASAN NULL deref / panic for this
   ISR pattern.
3. Important (crash/security/corruption/deadlock)? **PASS** — hardirq
   oops/panic.
4. Small and contained? **PASS** — one function, +9/−4.
5. No new features/APIs? **PASS**
6. Can apply to this tree? **PASS** — pre-fix ISR is the historical form
   of this file; no missing helpers.

**Exception category:** none (ordinary crash fix).

This is a 13-line IRQ-handler NULL-deref (and detach UAF) fix in
existing comedi code. It meets the stable rules. The local 7.3.0-rc4
tree has the driver and had the bug since v4.0.

### What it fixes for stable users
If the AT-A2150 ISR runs after `request_irq()` but before
`dev->attached` (or after DMA is freed on detach), the old handler
oopses on NULL/dangling `dma`/`read_subdev`/`async`. Moving the existing
guard first prevents that crash.

### Risk vs benefit
Benefit is avoiding a kernel oops; risk is negligible. Other patches in
the 11-patch series are not required.

---

Verification:
- [Phase 1] Parsed `e7356cdc7077a`: Ian Abbott + Greg KH; msgid
  `20260618102949.26607-5`; no Fixes/Cc stable
- [Phase 1] Cover letter from mbox: series is explicitly “avoid null-
  ptr-deref”
- [Phase 2] Diff: +9/−4 in `a2150_interrupt()`; moved
  `desc`/`async`/`cmd`/`buf` after `if (!dev->attached)`
- [Phase 2] `a2150_attach` / `a2150_alloc_irq_and_dma` /
  `comedi_device_postconfig`: `request_irq` before `attached = true`
- [Phase 2] `comedi_device_detach_locked`: `attached = false` before
  `a2150_free_dma` then `free_irq`
- [Phase 3] `git blame` on pre-image: check from 2009; early derefs from
  `1a97f144e3d3e9` (v4.0-rc1)
- [Phase 3] `git merge-base --is-ancestor 1a97f144e3d3e9 HEAD` and
  `e7356cdc7077a HEAD`: both true
- [Phase 3] Series is 11 independent driver patches; no ni_at_a2150
  follow-up
- [Phase 4] `b4 dig -c/-a/-w`: v1 only; recipients include Greg, Hartley
  Sweeten, SNU reporters
- [Phase 4] `b4 mbox` of `20260610115912.780131-1-jjy600901@xxxxxxxxx`:
  KASAN null-ptr-deref report
- [Phase 4] Related `17221216ae8ce` comedi_parport fix: syzbot GPF + Cc
  stable
- [Phase 4] lore HTTP 403; UNVERIFIED stable@ archive search
- [Phase 5] `a2150_interrupt` only used from `request_irq` in this file
- [Phase 6] Local tree 7.3.0-rc4 (`v7.3-rc4-27-gf0100363d8c37`); pre-fix
  ISR confirmed at parent
- [Phase 7] `CONFIG_COMEDI_NI_AT_A2150`; peripheral ISA DAQ
- [Phase 8] Failure mode: hardirq NULL deref/UAF oops, severity HIGH;
  backport risk very low
- UNVERIFIED: hardware reproduction on a real AT-A2150; stable mailing-
  list mentions

**YES**

 drivers/comedi/drivers/ni_at_a2150.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/comedi/drivers/ni_at_a2150.c b/drivers/comedi/drivers/ni_at_a2150.c
index e4e5a0ebd195a..93e9591f5dd4c 100644
--- a/drivers/comedi/drivers/ni_at_a2150.c
+++ b/drivers/comedi/drivers/ni_at_a2150.c
@@ -132,11 +132,11 @@ static irqreturn_t a2150_interrupt(int irq, void *d)
 	struct comedi_device *dev = d;
 	struct a2150_private *devpriv = dev->private;
 	struct comedi_isadma *dma = devpriv->dma;
-	struct comedi_isadma_desc *desc = &dma->desc[0];
+	struct comedi_isadma_desc *desc;
 	struct comedi_subdevice *s = dev->read_subdev;
-	struct comedi_async *async = s->async;
-	struct comedi_cmd *cmd = &async->cmd;
-	unsigned short *buf = desc->virt_addr;
+	struct comedi_async *async;
+	struct comedi_cmd *cmd;
+	unsigned short *buf;
 	unsigned int max_points, num_points, residue, leftover;
 	unsigned short dpnt;
 	int status;
@@ -145,6 +145,11 @@ static irqreturn_t a2150_interrupt(int irq, void *d)
 	if (!dev->attached)
 		return IRQ_HANDLED;
 
+	desc = &dma->desc[0];
+	async = s->async;
+	cmd = &async->cmd;
+	buf = desc->virt_addr;
+
 	status = inw(dev->iobase + STATUS_REG);
 	if ((status & INTR_BIT) == 0)
 		return IRQ_NONE;
-- 
2.53.0