* [PATCH AUTOSEL 6.18-5.15] virtio_ring: fix infinite loop in virtnet_poll_cleantx when device is broken
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:47 ` Sasha Levin
2026-10-19 14:25 ` sashiko-bot
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-6.6] vdpa_sim: hold iommu_lock across dma_unmap passthrough transition Sasha Levin
` (8 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Sasha Levin @ 2026-10-19 13:47 UTC (permalink / raw)
To: patches, stable
Cc: Jinqian Yang, Xuan Zhuo, Michael S. Tsirkin, Sasha Levin,
jasowangio, eperezma, virtualization, linux-kernel
From: Jinqian Yang <yangjinqian1@huawei.com>
[ Upstream commit 0d0eff39ceb3dcbf7847a6f4517086c207c60081 ]
virtnet_poll_cleantx() contains a do-while loop that cleans up
transmitted TX buffers and calls virtqueue_enable_cb_delayed() to check
whether more buffers need processing. When the virtio backend stops
responding during guest reboot, used->idx is never updated, so
virtqueue_enable_cb_delayed() always returns false and the loop never
terminates. Then it will block reboot process, and the guest will hang.
The problem occurs during guest reboot under network traffic:
1. kernel_restart() -> device_shutdown() traverses the device list
2. virtio_dev_shutdown() calls virtio_break_device() which sets
vq->broken = true
3. virtio_dev_shutdown() then calls virtio_synchronize_cbs() to wait
for in-flight callbacks to complete
4. A virtio interrupt fires, softirq is deferred to ksoftirqd which
calls net_rx_action() -> virtnet_poll() -> virtnet_poll_cleantx()
5. virtnet_poll_cleantx() enters the do-while loop and never exits
because the QEMU backend has stopped updating used->idx, despite
vq->broken having been set to true in step 2.
Since the loop runs inside ksoftirqd (a SCHED_OTHER kthread), it is
visible to the scheduler and does not trigger a hard lockup. However,
the kthread never leaves the loop, so RCU detects it as a CPU stall
and reports it periodically. Meanwhile, the reboot process remains
blocked in device_shutdown() because virtio_dev_shutdown() cannot
complete its synchronization step, and the guest hangs permanently.
This can be reproduced on a guest with a virtio-net device: run iperf3
traffic in the guest, then trigger reboot. The reboot occasionally hangs
permanently with RCU stall on ksoftirqd.
Observed on ARM64 KVM guest:
CPU#1 RCU stall (ksoftirqd/1), repeated periodically:
virtqueue_enable_cb_delayed_split <- virtnet_poll <- __napi_poll <-
net_rx_action <- handle_softirqs <- run_ksoftirqd <-
smpboot_thread_fn <- kthread
Fix by adding a vq->broken check in virtqueue_enable_cb_delayed(), so
that the loop exits immediately when the device is broken, allowing
the device shutdown to proceed.
Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-ID: <20260716115940.394832-1-yangjinqian1@huawei.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have the local-tree picture: the cleanup loop is here,
`virtio_dev_shutdown()` is not, and the missing `vq->broken` check is
still a hang. Next I’ll write the full nine-phase verdict from that
evidence.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: [virtio_ring] [fix] [stop infinite loop in virtnet_poll_cleantx
when the virtqueue is marked broken]
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com> (author)
- Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> (virtio co-
maintainer)
- Signed-off-by: Michael S. Tsirkin <mst@redhat.com> (virtio maintainer)
- Message-ID: <20260716115940.394832-1-yangjinqian1@huawei.com>
- No Fixes: tag (expected)
- No Cc: stable (expected)
- No Reported-by / Tested-by / syzbot / Link: tags in the provided
message
- Ignore pipeline markers (none present in the provided text besides
normal maintainer SOBs)
Notable: maintainer + co-maintainer sign-off; author describes a real
reproduced hang, not a theoretical cleanup.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- Bug: `virtnet_poll_cleantx()` do-while loops on
`virtqueue_enable_cb_delayed()` returning false. If the backend never
advances `used->idx`, the loop never ends.
- Symptom: guest reboot hang under virtio-net traffic; RCU stalls on
ksoftirqd (`virtqueue_enable_cb_delayed_split <- virtnet_poll <-
net_rx_action <- run_ksoftirqd`); `device_shutdown()` blocked in
`virtio_synchronize_cbs()`.
- Trigger described: `kernel_restart()` → `device_shutdown()` →
`virtio_dev_shutdown()` sets `vq->broken` then waits for callbacks; a
deferred NAPI poll then spins forever.
- Root cause: `virtqueue_enable_cb_delayed()` keeps polling `used->idx`
even after `vq->broken` is true.
- Reproduction: iperf3 + reboot on ARM64 KVM guest; occasional permanent
hang.
- Version info in the message: none named; the reboot path names
`virtio_dev_shutdown()`.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a hidden fix. Subject and body are an explicit
hang/infinite-loop fix. Not cleanup, refactor, or a feature.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/virtio/virtio_ring.c` only
- Scope: +8 lines (comment + `if (unlikely(vq->broken)) return true;`)
- Function: `virtqueue_enable_cb_delayed()`
- Classification: single-file surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- Before: convert `_vq` to `vring_virtqueue`, optionally clear
`event_triggered`, then call split/packed `enable_cb_delayed`, which
looks at `used->idx` / packed used descriptors and returns false if
“many pending buffers” remain.
- After: if `vq->broken`, return true immediately so callers of the form
`while (!virtqueue_enable_cb_delayed())` exit.
- Path affected: any enable-cb-delayed poll/cleanup loop after the
device is marked broken. Not a new feature path.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- Category: logic/correctness that produces an infinite loop / hang
(also a missing “device broken” abort, same family as other virtqueue
ops).
- Mechanism: `virtqueue_enable_cb_delayed()` did not honor `vq->broken`,
unlike `virtqueue_poll()`, `virtqueue_add_*()`,
`virtqueue_get_buf_*()`, and `virtqueue_notify()` in this tree.
Callers treat a false return as “more work remains”; on a dead backend
that is never true, so they spin.
- Fix: return true when broken, matching `virtqueue_enable_cb()` which
already becomes true because `virtqueue_poll()` returns false when
broken.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: one `unlikely(vq->broken)` check at the API
boundary, same pattern as sibling helpers.
- Minimal: no unrelated edits.
- Regression risk: low. Returning true skips further used-ring polling
and skips re-enabling callbacks. `vring_interrupt()` already refuses
to invoke callbacks when broken (returns IRQ_HANDLED / IRQ_NONE
without calling `vq->callback`). Leftover TX buffers are still
collected later by `free_unused_bufs()` /
`virtqueue_detach_unused_buf()` on teardown.
- Red flags: none (no API change, no new lock, no data-structure
change).
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- `virtqueue_enable_cb_delayed()` wrapper: e6f633e5beab65 /
1ce9e6055fa0a9 (Tiwei Bie, 2018) — packed/split split-up; the function
itself is older.
- The spinning loop in `virtnet_poll_cleantx` / `start_xmit`:
a7766ef18b3367 (“virtio_net: disable cb aggressively”, 2021-07-08).
Present in this tree (`git merge-base --is-ancestor` exit 0).
- `virtqueue_poll()` already has `if (unlikely(vq->broken)) return
false;` in this tree.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No Fixes: tag. The hang the author describes is the interaction
of that 2021 loop with later reboot teardown, not a one-line typo in a
single commit.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Recent `virtio_ring.c` work here is KCSAN tagging, resize/reset args,
premapped DMA — unrelated.
- This is standalone, not patch X/Y of a series in-tree.
- Mailing-list series is v1→v3 of the same fix (see Phase 4).
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record:
- Jinqian Yang has no other virtio_ring/virtio_net commits in this tree.
Not the subsystem maintainer.
- Weight comes from Reviewed-by Xuan Zhuo and SOB Michael S. Tsirkin.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record:
- The insertion does not use `VIRTQUEUE_CALL` (mainline) vs the 6.12
ternary packed/split call. The new code sits before that dispatch.
- No new helpers. Can apply standalone.
- The *reboot* scenario names `virtio_dev_shutdown()`, which is **not**
in this tree (see Phase 6). The *code defect* does not depend on that
function.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c <message-id>`: no matching commit object in this checkout
(candidate is not on HEAD).
- `b4 mbox` for Message-ID
20260716115940.394832-1-yangjinqian1@huawei.com: lore returned 404.
- Fallback mirrors (verified):
- v3: https://lists.openwall.net/linux-kernel/2026/07/16/1135
- v1: https://lkml.iu.edu/2607.1/10830.html (changed `virtio_net.c`
loop to also test `virtqueue_is_broken()`)
- v2 changelog: moved the check into `virtqueue_enable_cb_delayed()`
- v3: subject prefix only (`virtio_net:` → `virtio_ring:`)
- Xuan Zhuo Reviewed-by on v3: https://lkml.rescloud.iu.edu/hypermail/
linux/kernel/2607.2/09380.html
- No NAK found. Applied version is v3 (latest).
- No reviewer “Cc: stable” found in the retrieved mail.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record: v3 To/Cc (from the v3 post): mst@redhat.com,
jasowang@redhat.com, xuanzhuo@linux.alibaba.com, eperezma@redhat.com,
netdev maintainers; lists netdev, virtualization, linux-kernel.
Appropriate virtio + netdev audience. Reviewed-by Xuan Zhuo; merged via
MST.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla. Author reports a reproduced ARM64 KVM guest
hang (iperf3 + reboot). That is a real user-visible failure, but the
reboot recipe uses `virtio_dev_shutdown()` (Phase 6).
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Single-patch series, v1–v3. v1 only fixed the virtio_net loop;
v2/v3 fixed the core helper so every looping caller is covered
(`start_xmit` in this tree has the same pattern). No extra patches
required.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable fetch returned 403; web search of stable
archives did not show this subject. UNVERIFIED: no evidence this was
already requested or rejected for stable.
Related context (verified via git objects + public commit page):
8bd2fa086a04 (“virtio: break and reset virtio devices on
device_shutdown()”) added `virtio_dev_shutdown()`. `git merge-base --is-
ancestor 8bd2fa086a04 HEAD` → not an ancestor. Same SHA **is** an
ancestor of v6.15, not of v6.12.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `virtqueue_enable_cb_delayed()` (exported). Indirectly
`virtqueue_enable_cb_delayed_split()` / `_packed()` are skipped when
broken.
**Step 5.2: TRACE CALLERS**
Record (this tree, `git grep`):
- `drivers/net/virtio_net.c`:
- `virtnet_poll_cleantx()` — do-while until enable_cb_delayed is true
(the hang)
- `start_xmit()` — same loop when napi-tx and !xmit_more
- TX-stop path (~1066/1068) — not a spin loop; false just schedules
NAPI / frees old bufs
- `tools/virtio/*` test code only
No other in-tree driver loops on this helper.
Call context: NAPI/`ksoftirqd` (`virtnet_poll` → `virtnet_poll_cleantx`)
and `ndo_start_xmit`. Userspace-reachable via normal virtio-net RX/TX.
**Step 5.3: TRACE CALLEES**
Record: `to_vvq()`, optional `event_triggered` clear, then split/packed
enable which does `START_USE`, writes used-event idx, compares
`used->idx` vs `last_used_idx`. No alloc; ring MMIO/memory only.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record:
- `virtnet_poll()` (line 2892) always calls `virtnet_poll_cleantx()`
then `virtnet_receive()`.
- `virtnet_poll` is the RX NAPI poll; with napi-tx it also reaps TX.
- Reachable from virtio-net interrupts / NAPI. Yes, userspace traffic
can keep this hot.
Broken-device entry points in **this** tree (not reboot shutdown):
- `virtio_pci_remove()`: `if (!pci_device_is_present(pci_dev))
virtio_break_device(...)` then `unregister_virtio_device()` (verified
in `drivers/virtio/virtio_pci_common.c`).
- `virtqueue_notify()`: sets `vq->broken` if `notify()` fails.
- `virtio_reset_device()`: `virtio_break_device` +
`virtio_synchronize_cbs` only under
`CONFIG_VIRTIO_HARDEN_NOTIFICATION`, which `depends on BROKEN` here —
not a production 6.12 config.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same do-while in `start_xmit()`. Core virtqueue ops already
abort on `vq->broken`. This helper was the inconsistent one. No second
copy of the bug in other drivers in this tree.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Tree: **6.12.111** (`make kernelversion`), branch `pending-6.12`, `git
describe` p-6.12-3-gc504cf0baa3d8.
- `virtqueue_enable_cb_delayed()` at `drivers/virtio/virtio_ring.c:2528`
has **no** `vq->broken` check.
- `virtnet_poll_cleantx()` at `drivers/net/virtio_net.c:2842-2845` still
has:
`do { virtqueue_disable_cb(); free_old_xmit(); } while
(unlikely(!virtqueue_enable_cb_delayed(sq->vq)));`
- `virtio_bus` has probe/remove only — **no** `.shutdown`, **no**
`virtio_dev_shutdown()`.
- Therefore: the **infinite loop** exists; the **reboot-via-
virtio_dev_shutdown** trigger does **not**.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Clean/minor. Mainline uses `VIRTQUEUE_CALL(vq,
enable_cb_delayed)` after the event_triggered block; 6.12 uses a
packed/split ternary. The new lines go immediately after `to_vvq()` and
before `event_triggered`, which exists here (8d622d21d24803 /
b730cb109633c). Expected apply: clean or trivial context.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: Comment “When the device is broken there is no point in polling
used->idx” is absent. `git log --grep='infinite loop in
virtnet_poll_cleantx'` on these files: empty. No earlier equivalent fix.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: virtio core (`drivers/virtio/virtio_ring.c`) used by virtio-net.
Criticality: **IMPORTANT** (default NIC in KVM/QEMU/cloud guests; not
mm/core, but extremely common).
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Active but mature. The looping TX cleanup dates to 2021; broken-
bit handling is long-standing. This is a hole in an old contract, not a
bug in brand-new 6.12-only code.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: virtio-net guests (and any other future/looping caller of
`virtqueue_enable_cb_delayed`). Driver-specific, but a very large VM
population.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Documented reboot hang: requires `virtio_dev_shutdown()` — **not in
6.12**.
- In this tree, hang requires (1) napi-tx cleanup loop running, (2)
`used->idx` stuck, (3) `vq->broken` true so this patch can abort.
Realistic 6.12 cases: virtio-pci surprise removal under traffic;
notify failure / backend disconnect with NAPI still polling.
- Unprivileged guest userspace can generate the traffic;
unplug/reboot/backend death is admin/hypervisor-side.
- Timing-dependent (NAPI already running when the queue is broken).
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: Infinite loop in `ksoftirqd` → RCU stalls; teardown paths that
wait for NAPI (`unregister_netdev` / `napi_disable`) block forever.
Severity: **CRITICAL** (guest hang), when the trigger hits. Frequency:
uncommon (needs broken device + in-flight TX cleanup), but consequences
are a stuck VM, not a warning.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- Benefit: stops a hard hang in virtio-net TX cleanup once `vq->broken`
is set. On 6.12 this is not the 6.15+ reboot path, but it is the same
loop vs the same broken flag this kernel already sets on surprise
removal and notify failure.
- Risk: very low (8 lines, existing pattern, `unlikely` fast path).
- Ratio: favorable.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR backporting:
- Real hang: unbounded loop in NAPI/`ksoftirqd` when `enable_cb_delayed`
never returns true.
- Loop and helper both exist in 6.12.111; fix is not already applied.
- `vq->broken` is the documented abort switch; this helper uniquely
ignored it.
- Same semantics as `virtqueue_enable_cb()` via `virtqueue_poll()`.
- 8-line, obviously correct, reviewed by Xuan Zhuo, merged by MST.
- virtio-net is the common VM NIC; hangs on unplug/backend death matter
to stable users.
- Applies with no prerequisites.
AGAINST backporting:
- The author’s reboot/kexec story depends on `virtio_dev_shutdown()`
from 8bd2fa086a04 (v6.15), which is **not** in this tree. That
specific “reboot hangs in device_shutdown” bug was introduced after
6.12 branched.
- No Fixes:/Cc: stable (expected, not a negative).
- No second independent reporter, no syzbot.
- Surprise-removal/notify-fail hang is reasoned from call chains, not a
6.12-specific user report.
UNRESOLVED:
- Could not run `b4 dig` against a mainline SHA (commit not in this
repo); lore.kernel.org 403/404 for some URLs.
- Stable list discussion: UNVERIFIED (403).
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — matches sibling broken
checks; author reproduced; virtio maintainers reviewed/applied.
2. Fixes a real bug that affects users? **PASS** — infinite NAPI loop
when the queue is broken; the loop exists here. The 6.15 reboot
recipe does not, but 6.12 still marks queues broken and still runs
this loop.
3. Important issue (crash, security, corruption, deadlock)? **PASS** —
hang / RCU stall / teardown stuck. Severity CRITICAL when triggered.
4. Small and contained? **PASS** — 8 lines, one function.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — same function, same insertion
point.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. Ordinary hang
fix.
**Step 9.4: MAKE YOUR DECISION**
This tree is 6.12.111. The reboot hang named in the commit message needs
`virtio_dev_shutdown()`, which landed in 6.15 and is not here — that
trigger alone would not justify a backport.
The patch still belongs on 6.12 because it does not fix that later
function; it closes a hole in `virtqueue_enable_cb_delayed()` that 6.12
already has. `virtnet_poll_cleantx()` / `start_xmit()` will spin forever
if `used->idx` stops moving, and they never look at `vq->broken`. This
kernel already sets that flag (PCI surprise removal, failed notify).
Returning true there is the same contract as `virtqueue_poll()` /
`virtqueue_enable_cb()`, is eight lines, and cannot wait on new APIs.
Benefit is avoiding a virtio-net hang in a widely used guest NIC;
regression risk is negligible.
Verification:
- [Phase 1] Parsed subject/tags/body: hang fix; Reviewed-by Xuan Zhuo;
SOB MST; Message-ID 20260716115940.394832-1; no Fixes/syzbot
- [Phase 2] Diff: +8 lines in `virtqueue_enable_cb_delayed()`; return
true if `vq->broken`
- [Phase 3] Blame: wrapper 2018 (e6f633e5beab65); poll_cleantx loop
a7766ef18b3367 (2021), ancestor of HEAD
- [Phase 3] No Fixes: tag; no Jinqian Yang virtio history in this tree
- [Phase 4] b4 dig on Message-ID: no commit in this repo; b4 mbox: lore
404
- [Phase 4] v1 patched virtio_net loop; v2/v3 moved check to
virtio_ring; Xuan Zhuo Reviewed-by on v3 (lkml.rescloud 09380)
- [Phase 4] 8bd2fa086a04 exists as object, NOT ancestor of HEAD; IS
ancestor of v6.15; NOT of v6.12
- [Phase 4] UNVERIFIED: stable@ discussion (lore 403)
- [Phase 5] Callers: virtnet_poll_cleantx, start_xmit, TX-stop;
poll_cleantx invoked from virtnet_poll line 2892
- [Phase 5] virtio_pci_remove surprise-removal calls virtio_break_device
(line 758-759)
- [Phase 5] virtqueue_poll/add/get_buf/notify already check broken;
enable_cb_delayed does not
- [Phase 5] CONFIG_VIRTIO_HARDEN_NOTIFICATION depends on BROKEN in this
Kconfig
- [Phase 6] kernelversion 6.12.111; virtio_bus has no .shutdown; fix
comment absent
- [Phase 6] Insertion point (after to_vvq, before event_triggered)
present
- [Phase 8] Failure mode: infinite ksoftirqd loop / RCU stall / teardown
hang
- UNVERIFIED: whether a 6.12 user has actually hit surprise-unplug hang
(code path verified, field report not found)
**YES**
drivers/virtio/virtio_ring.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b8199e578ea98..5c0139f3b9811 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2667,6 +2667,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
+ /*
+ * When the device is broken there is no point in polling used->idx,
+ * the backend will never update it. Return true to let callers
+ * exit their cleanup loops instead of spinning forever.
+ */
+ if (unlikely(vq->broken))
+ return true;
+
if (vq->event_triggered)
data_race(vq->event_triggered = false);
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH AUTOSEL 6.18-5.15] virtio_ring: fix infinite loop in virtnet_poll_cleantx when device is broken
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.15] virtio_ring: fix infinite loop in virtnet_poll_cleantx when device is broken Sasha Levin
@ 2026-10-19 14:25 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-19 14:25 UTC (permalink / raw)
To: Sasha Levin; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> From: Jinqian Yang <yangjinqian1@huawei.com>
>
> [ Upstream commit 0d0eff39ceb3dcbf7847a6f4517086c207c60081 ]
>
> virtnet_poll_cleantx() contains a do-while loop that cleans up
> transmitted TX buffers and calls virtqueue_enable_cb_delayed() to check
> whether more buffers need processing. When the virtio backend stops
> responding during guest reboot, used->idx is never updated, so
> virtqueue_enable_cb_delayed() always returns false and the loop never
> terminates. Then it will block reboot process, and the guest will hang.
>
> The problem occurs during guest reboot under network traffic:
>
> 1. kernel_restart() -> device_shutdown() traverses the device list
> 2. virtio_dev_shutdown() calls virtio_break_device() which sets
> [ ... ]
> Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Message-ID: <20260716115940.394832-1-yangjinqian1@huawei.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-133-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH AUTOSEL 6.18-6.6] vdpa_sim: hold iommu_lock across dma_unmap passthrough transition
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.15] virtio_ring: fix infinite loop in virtnet_poll_cleantx when device is broken Sasha Levin
@ 2026-10-19 13:47 ` Sasha Levin
2026-10-19 14:29 ` sashiko-bot
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-6.1] drm/virtio: reclaim pending vbufs before tearing down vqs Sasha Levin
` (7 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Sasha Levin @ 2026-10-19 13:47 UTC (permalink / raw)
To: patches, stable
Cc: Xiong Weimin, Michael S. Tsirkin, Sasha Levin, jasowangio,
eperezma, virtualization, linux-kernel
From: Xiong Weimin <xiongweimin@kylinos.cn>
[ Upstream commit 7b411448c668bfab0d0e572cee5a592cea396ce7 ]
vdpasim_dma_map() updates the IOTLB and the passthrough (iommu_pt)
state under iommu_lock. vdpasim_dma_unmap() clears iommu_pt and
resets the IOTLB before taking iommu_lock, then deletes the mapping
while holding the lock.
A concurrent dma_map(), dma_unmap(), or reset path that also touches
the same address space can therefore observe or modify the IOTLB and
iommu_pt state without consistent locking.
Perform the passthrough transition and range deletion under the same
iommu_lock scope, matching dma_map().
Tested-on: openEuler VM (6.16.8, /usr/src/linux-6.16.8)
Tested-by: Xiong Weimin <xiongweimin@kylinos.cn>
Signed-off-by: Xiong Weimin <xiongweimin@kylinos.cn>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-ID: <20260626020545.607600-3-15927021679@163.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have enough to judge this for 6.12: the buggy unlocked `dma_unmap`
path is present, the race with the simulator datapath is real, and the
fix is a one-lock-move. Compiling the nine-phase analysis next.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[vdpa_sim] [hold] [take `iommu_lock` for the full `dma_unmap`
passthrough-to-custom IOTLB transition]**
- Subsystem: `vdpa_sim` (vDPA device simulator)
- Verb: “hold” — locking/correctness, not a feature
- Intent: make `vdpasim_dma_unmap()` lock the same way
`vdpasim_dma_map()` already does
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Tested-on:** openEuler VM (6.16.8, `/usr/src/linux-6.16.8`)
- **Tested-by:** Xiong Weimin `<xiongweimin@kylinos.cn>` (author; not
independent)
- **Signed-off-by:** Xiong Weimin `<xiongweimin@kylinos.cn>` (author)
- **Signed-off-by:** Michael S. Tsirkin `<mst@redhat.com>` (vhost/vdpa
maintainer; applied the patch)
- **Message-ID:** `<20260626020545.607600-3-15927021679@163.com>`
Absent (expected for this review pipeline, not a negative):
- No `Fixes:`
- No `Cc: stable@vger.kernel.org`
- No `Reported-by:` / syzbot
- No `Reviewed-by:` / `Acked-by:`
- No `Link:`
Ignored per instructions: any `[Upstream commit …]` / `Signed-off-by:
Sasha Levin` pipeline markers.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `vdpasim_dma_map()` updates IOTLB + `iommu_pt` under
`iommu_lock`. `vdpasim_dma_unmap()` clears `iommu_pt` and calls
`vhost_iotlb_reset()` *before* taking the lock, then deletes the range
under the lock.
- **Symptom / failure mode:** concurrent `dma_map` / `dma_unmap` / reset
can observe or mutate IOTLB and `iommu_pt` without consistent locking.
That is interval-tree mutation vs walk → corruption / UAF / crash, not
a log-only issue.
- **Version info:** tested on 6.16.8; no “introduced in vX” text. The
buggy passthrough logic is from v6.3 (see Phase 3).
- **Root cause (author):** passthrough-exit in `dma_unmap` is outside
the lock that `dma_map` already uses.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not disguised.** This is an explicit locking/race fix. Subject
uses “hold … across”, body describes a concurrent IOTLB/`iommu_pt`
inconsistency. Hidden-fix heuristics are N/A.
---
## PHASE 2: DIFF ANALYSIS - LINE BY LINE
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `drivers/vdpa/vdpa_sim/vdpa_sim.c` only
- **Scope:** ~1 insertion, ~2 deletions (move `spin_lock` up, drop a
blank line)
- **Function:** `vdpasim_dma_unmap()`
- **Classification:** single-file surgical locking fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record — this tree today (`vdpasim_dma_unmap`):
```725:743:drivers/vdpa/vdpa_sim/vdpa_sim.c
static int vdpasim_dma_unmap(struct vdpa_device *vdpa, unsigned int
asid,
u64 iova, u64 size)
{
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
if (asid >= vdpasim->dev_attr.nas)
return -EINVAL;
if (vdpasim->iommu_pt[asid]) {
vhost_iotlb_reset(&vdpasim->iommu[asid]);
vdpasim->iommu_pt[asid] = false;
}
spin_lock(&vdpasim->iommu_lock);
vhost_iotlb_del_range(&vdpasim->iommu[asid], iova, iova + size -
1);
spin_unlock(&vdpasim->iommu_lock);
return 0;
}
```
- **Before:** if still in passthrough (`iommu_pt[asid]`), reset the
whole IOTLB interval tree and clear `iommu_pt` with **no lock**; then
take `iommu_lock` only for `vhost_iotlb_del_range()`.
- **After:** one `spin_lock` covers passthrough-exit
(`vhost_iotlb_reset` + `iommu_pt = false`) **and**
`vhost_iotlb_del_range()`, then unlock. Same shape as
`vdpasim_dma_map()`.
- **Path:** vhost-vdpa IOTLB invalidate / AS teardown
(`ops->dma_unmap`), not probe init.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Synchronization / race] [IOTLB interval tree and `iommu_pt`
mutated without `iommu_lock`]**
`vhost_iotlb_reset()` is `vhost_iotlb_del_range(iotlb, 0, ~0ULL)` — it
walks and frees the rbtree (`drivers/vhost/iotlb.c`). The datapath walks
that tree under `iommu_lock` via `iotlb_translate()` in
`drivers/vhost/vringh.c` (`spin_lock(vrh->iotlb_lock)` then
`vhost_iotlb_itree_first()`).
Unlocked reset vs locked walk is concurrent rbtree mutation → UAF /
corruption / oops.
Same lock is used by `vdpasim_dma_map()`, `vdpasim_set_map()`,
`vdpasim_reset_map()`, and `vdpasim_do_reset()`.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** obviously correct. It only moves an existing `spin_lock`
so the passthrough block matches `dma_map`. No new APIs, no new state.
- **Regression risk:** very low. Same lock already used on the sibling
path; no lock-order change vs `vdpasim->mutex` (unmap still does not
take that mutex). No new sleep under spinlock
(`vhost_iotlb_reset`/`del_range` are tree ops).
- **Red flags:** none. Independent of series patch 1/2 (`pending_kick`);
cover letter calls them independent.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Unlocked `iommu_pt` / `vhost_iotlb_reset` in `dma_unmap`:
**`6c3d329e6486dd`** — `vdpa_sim: get rid of DMA ops` (Jason Wang,
committed 2023-02-20).
- First in **v6.3-rc1** (`git describe --contains` →
`v6.3-rc1~84^2~23`).
- `spin_lock` around `del_range`: **`0ea9ee430e74b1`** (Max Gurtovoy,
2020).
- Present in this 6.12 tree for the whole lifetime of 6.12.y.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No `Fixes:` tag.** The introducing change is still
**`6c3d329e6486dd`**, and `git merge-base --is-ancestor 6c3d329e6486dd
HEAD` succeeded. No later fix of this lock split was found on this file.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record (recent `drivers/vdpa/vdpa_sim/vdpa_sim.c`):
- `05ddc94afb69e` vdpa_sim: fix cleanup after worker creation failure
- `9588e7fc511f9` vdpa_sim: reset must not run
- `86f6c224c9791` vdpa_sim: implement `.reset_map` support
- `6c3d329e6486d` vdpa_sim: get rid of DMA ops (introducer)
This commit is **standalone**. Cover letter: “[PATCH 0/2] vdpa_sim: fix
suspend/reset and dma_unmap locking” — two independent bugs. Sibling
`clear pending_kick on device reset` is **not** a prerequisite and is
**not** in this tree.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: **Xiong Weimin** has no other `vdpa_sim` commits in this tree.
Maintainer of record is **Michael S. Tsirkin** (SOB on apply) /
historical authors Jason Wang, Eugenio Pérez, Si-Wei Liu. Author is a
contributor, not the subsystem maintainer; MST still took the patch.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: **None.** Needs only `iommu_lock`, `iommu_pt`,
`vhost_iotlb_reset`, `vhost_iotlb_del_range` — all present. Applies as a
lock-line move. **Can apply standalone.**
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 mbox` for `20260626020545.607600-3-15927021679@163.com` saved the
thread (3 messages).
- Cover: `[PATCH 0/2] vdpa_sim: fix suspend/reset and dma_unmap locking`
- This patch is **2/2** (`-3` is cover+two patches).
- **No replies in the thread** (no NAK, no `Cc: stable` from reviewers,
no v2).
- Committed Message-ID matches this v1 2/2; applied version is the
submitted version.
- Lore HTML via WebFetch/curl: **403** — could not read lore web UI;
thread content came from `b4 mbox`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record: Thread is author-only. **Michael S. Tsirkin** SOBed on merge
(vhost/vdpa maintainer). No `Reviewed-by` on the list copy. Recipients
of the original mail were not dumped (`b4 dig -w` needs a local commit
SHA; this SHA is not in this tree).
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: **No external bug report.** No syzbot, bugzilla, or `Reported-
by`. Evidence is code inspection plus author test that the module still
loads on 6.16.8 — that does **not** prove the race was reproduced.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Two-patch series, independent. Patch 1/2 (`pending_kick`) is a
different bug and is not required. No other series members needed.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: `curl` to `lore.kernel.org/stable/?q=vdpa_sim+iommu_lock`
returned **403**. **UNVERIFIED:** whether stable@ already discussed
this. No local `git log --grep` hit for this subject on this file.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: **`vdpasim_dma_unmap()`** only. Registered as `.dma_unmap` in
`vdpasim_config_ops` / `vdpasim_batch_config_ops`.
**Step 5.2: TRACE CALLERS**
Record:
- `vhost_vdpa_general_unmap()` → `ops->dma_unmap()`
(`drivers/vhost/vdpa.c`)
- Called from `vhost_vdpa_pa_unmap()` / `vhost_vdpa_va_unmap()` →
`vhost_vdpa_iotlb_unmap()` → `vhost_vdpa_unmap()`
- Userspace: `vhost_vdpa_process_iotlb_msg()` `VHOST_IOTLB_INVALIDATE`
under `dev->mutex`
- Teardown: `vhost_vdpa_remove_as()` → `vhost_vdpa_iotlb_unmap()`
(release/cleanup, also under `dev->mutex`)
No other `ops->dma_unmap` users for this simulator in-tree.
**Step 5.3: TRACE CALLEES**
Record: `vhost_iotlb_reset()` → `vhost_iotlb_del_range()` → interval-
tree delete/free; `spin_lock`/`unlock` on `iommu_lock`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record:
- Reachable from **vhost-vdpa character-device IOTLB messages**
(privileged userspace: QEMU/DPDK with vhost-vdpa).
- Simulator datapath: `vdpasim_kick_vq()` → kthread `vdpasim_work_fn()`
→ `vdpa_sim_net`/`blk` `vringh_getdesc_iotlb()` /
`vringh_iov_{pull,push}_iotlb()` → `iotlb_translate()` under
`iommu_lock`.
- **Yes, userspace can trigger both sides:** IOTLB invalidate vs running
virtqueue processing.
**Nuance (verified):** vhost serializes `dma_map`, `dma_unmap`, and
`vdpa_reset` with `vdev.mutex`. So **map vs unmap vs reset from vhost
ioctls do not run together**. The commit text overstates that pair. The
**verified** concurrent party is the **simulator worker**, which does
**not** take `vdev.mutex` and walks the IOTLB under `iommu_lock` only.
This tree also advertises `VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK`
(`vdpasim` getter at line 436), so DRIVER_OK / kicks can happen while
still in passthrough (`iommu_pt == true`). First `dma_unmap` then takes
the unlocked reset path while the worker may be in `iotlb_translate()`.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: `dma_map`, `set_map`, `reset_map`, and `do_reset` already take
`iommu_lock` around `iommu_pt` and IOTLB updates. **`dma_unmap` is the
only passthrough-exit that does not.** No sibling driver has this exact
split; mlx5 has a different `iommu_lock`.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Tree: **Linux 6.12.111** (`Makefile` VERSION=6 PATCHLEVEL=12
SUBLEVEL=111; `git describe HEAD` → `p-6.12-3-gc504cf0baa3d8`; branch
`pending-6.12`).
- Buggy code is in `vdpasim_dma_unmap()` as cited above.
- Introduced **v6.3**, so **before** 6.12 branched. Bug **does** exist
here.
- This exact fix is **not** already applied (`git log --grep` empty for
this subject).
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply.** Surrounding function matches the hunk exactly.
No refactor of this path after 6.12. Expected difficulty: **clean
apply**.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: Other `vdpa_sim` bugfixes are in this pending tree (worker-
creation cleanup, sim_blk sector check, sim_net TX-pull check).
**Nothing already fixes this lock split.** Sibling `pending_kick` fix is
not here and is not required.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record:
- **Subsystem:** `drivers/vdpa/vdpa_sim` — vDPA **simulator** (net/blk
loopback for testing).
- **Kconfig:** `CONFIG_VDPA_SIM` “vDPA device simulator core”,
**`depends on RUNTIME_TESTING_MENU`**, help text: “testing,
prototyping and development of vDPA”.
- **Criticality:** **PERIPHERAL** (not mm/VFS/net core). Not a
production NIC/block driver. Still in-tree and used with vhost-vdpa in
labs/CI (author: openEuler).
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature-but-active. Core simulator from ~2020; 6.12 still
receives sim bugfixes (including 2026 worker-cleanup). Not a dead file.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **config-specific / driver-specific.** Only `CONFIG_VDPA_SIM`
(and `_NET`/`_BLOCK`) plus vhost-vdpa IOTLB programming. Not universal.
Production kernels often leave the module off; anyone who loads it (QEMU
vDPA sim, distro debug kernels) can hit it.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- **Trigger:** IOTLB invalidate / unmap while `iommu_pt[asid]` is true,
overlapping simulator worker IOTLB walk.
- **Likelihood:** not every boot; realistic with
`ENABLE_AFTER_DRIVER_OK` and live IOTLB updates while VQs run. Hard to
hit, timing-dependent.
- **Privilege:** vhost-vdpa + creating a sim device is typically
**root**, not an unprivileged syscall primitive. Not a generic
syzbot/LPE story.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- **Failure mode:** concurrent interval-tree reset vs walk → use-after-
free / memory corruption / oops.
- **Severity:** **HIGH** (crash/UAF) on that config path; not a
production-wide panic.
- Unlocked `iommu_pt` store vs locked readers is also a data race; the
tree walk is the severe part.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** medium for sim users (prevents crash/UAF); low for
generic 6.12 fleet that never loads `vdpa_sim`.
- **Risk:** **very low** (1–2 line lock move, matches existing `dma_map`
protocol).
- **Ratio:** benefit exceeds risk. A no-op on systems without the
module; cannot change unrelated drivers.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR backporting:**
- Real locking bug: IOTLB rbtree reset without the lock the walker uses
- Crash/UAF-class, not style
- Tiny, obviously correct, matches `dma_map`
- Buggy code present since v6.3; in this 6.12 tree
- Applies cleanly; no prerequisites
- Maintainer (MST) applied it in mainline
- This pending-6.12 tree already takes other `vdpa_sim` correctness
fixes
- Race window is real against the kthread datapath (verified call chain)
**AGAINST:**
- Simulator behind `RUNTIME_TESTING_MENU`, not production hardware
- No syzbot / third-party crash report; `Tested-by` is the author
- No list review beyond maintainer SOB
- Commit’s “concurrent dma_map/dma_unmap/reset” is **overstated** given
vhost `dev->mutex`
- Peripheral impact population
**UNRESOLVED:**
- Lore/stable HTML 403; no public stable@ discussion found
- Whether the author actually reproduced the race vs only smoke-tested
module load
- `b4 dig -w` recipients (no local mainline SHA)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — lock move mirrors `dma_map`;
author + maintainer SOB. Testing is light but the change is
reviewable by inspection.
2. Fixes a real bug that affects users? **PASS** — unlocked IOTLB reset
vs datapath walk. Affects `vdpa_sim` + vhost-vdpa users, which this
tree already treats as in-scope.
3. Important issue (crash, security, corruption, deadlock)? **PASS** —
UAF/oops on IOTLB tree; severity HIGH on that path.
4. Small and contained? **PASS** — ~2 lines, one function.
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — context identical in 6.12.111
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **None** (not a device ID, quirk, DT, build, or doc-only
change). It qualifies as a normal small race/crash fix.
**Step 9.4: MAKE YOUR DECISION**
This should be backported to **this** 6.12 tree. The bug is in this
checkout, the fix is surgical, and the failure mode is memory-safety on
a path this tree already ships. Simulator-only scope lowers fleet-wide
benefit but does not make a 2-line IOTLB UAF fix inappropriate for
stable, especially with near-zero regression risk.
---
### What the commit solves (for this tree)
`vdpasim_dma_unmap()` drops the whole passthrough IOTLB
(`vhost_iotlb_reset`) and clears `iommu_pt` without `iommu_lock`, while
`dma_map` and the vringh translator use that lock. A vhost-vdpa IOTLB
unmap can therefore tear down the interval tree while the simulator
worker is in `iotlb_translate()`. That is a kernel oops/UAF, not a
benign inconsistency. Holding `iommu_lock` across that transition closes
the hole.
### Stable rules / risk vs benefit
Meets the rules: real bug, small, no new API, applies here. Benefit is
crash prevention for `CONFIG_VDPA_SIM`; risk is negligible. The missing
`Fixes:`/`Cc: stable` tags are irrelevant to this pipeline.
### Concerns
- No independent repro; vhost mutex means map/unmap/reset do not race
with each other — the datapath race is the one that matters and is
verified.
- Patch 1/2 of the series is unrelated and must not be treated as a
dependency.
- Peripheral testing driver; still in this tree and already receiving
similar backports.
---
### Verification
- **[Phase 1]** Parsed subject/body/tags from the provided message and
from `b4 mbox` (cover + 2/2). Tags: Tested-on/Tested-by/SOB author,
SOB MST, Message-ID. No Fixes/Cc stable/Reported-by/Reviewed-by.
- **[Phase 2]** Read `vdpasim_dma_unmap`/`vdpasim_dma_map` in this tree;
compared to the diff. Confirmed unlocked reset vs locked `dma_map`.
- **[Phase 2]** Read `vhost_iotlb_reset`/`vhost_iotlb_del_range` in
`drivers/vhost/iotlb.c` (interval tree free).
- **[Phase 3]** `git blame -L 725,743` → introducer `6c3d329e6486dd`
(Jason Wang, v6.3-rc1).
- **[Phase 3]** `git merge-base --is-ancestor 6c3d329e6486dd HEAD` →
yes. `git describe --contains` → `v6.3-rc1~84^2~23`.
- **[Phase 3]** `git show 6c3d329e6486dd` — “get rid of DMA ops” added
`iommu_pt` passthrough; unmap’s reset was left outside the lock.
- **[Phase 3]** `git log --oneline -20 --
drivers/vdpa/vdpa_sim/vdpa_sim.c`; `--grep` for this subject /
`pending_kick` → fix not present; sibling not present.
- **[Phase 3]** `git log --author="Xiong Weimin" -- drivers/vdpa/` — no
vdpa_sim commits in this tree.
- **[Phase 4]** `b4 mbox` Message-ID → 3-message v1 series; no review
replies; cover says issues are independent.
- **[Phase 4]** WebFetch/curl lore.kernel.org → 403. UNVERIFIED: stable@
archives and HTML lore UI.
- **[Phase 5]** Callers: `ops->dma_unmap` only from
`drivers/vhost/vdpa.c`. Datapath: `vdpa_sim_net.c`/`vdpa_sim_blk.c`
`vringh_*_iotlb` → `iotlb_translate` + `iommu_lock`.
- **[Phase 5]** `vhost_vdpa_process_iotlb_msg` holds `dev->mutex` —
map/unmap/reset from vhost do not interleave (commit text overstated;
datapath race still real).
- **[Phase 5]** `VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK` offered in this
file (line 436).
- **[Phase 6]** `head Makefile` / `git describe HEAD` / `make
kernelversion` → **6.12.111**, branch `pending-6.12`.
- **[Phase 6]** File context matches the hunk; fix not in tree.
- **[Phase 7]** `drivers/vdpa/Kconfig`: `VDPA_SIM` depends on
`RUNTIME_TESTING_MENU`; testing/prototyping help text.
- **[Phase 8]** Failure mode: IOTLB rbtree UAF/oops, HIGH on that
config; trigger: IOTLB unmap in passthrough vs worker.
- **UNVERIFIED:** lore web discussion beyond b4 mbox; whether author
reproduced the race vs module-load smoke test; `b4 dig -w` recipient
list.
The fix is small, correct, already matched by `dma_map` in this 6.12
tree, and prevents a real IOTLB UAF race on a driver this tree ships.
**YES**
drivers/vdpa/vdpa_sim/vdpa_sim.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 9df300c467452..44055778b6940 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -736,12 +736,11 @@ static int vdpasim_dma_unmap(struct vdpa_device *vdpa, unsigned int asid,
if (asid >= vdpasim->dev_attr.nas)
return -EINVAL;
+ spin_lock(&vdpasim->iommu_lock);
if (vdpasim->iommu_pt[asid]) {
vhost_iotlb_reset(&vdpasim->iommu[asid]);
vdpasim->iommu_pt[asid] = false;
}
-
- spin_lock(&vdpasim->iommu_lock);
vhost_iotlb_del_range(&vdpasim->iommu[asid], iova, iova + size - 1);
spin_unlock(&vdpasim->iommu_lock);
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH AUTOSEL 6.18-6.6] vdpa_sim: hold iommu_lock across dma_unmap passthrough transition
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-6.6] vdpa_sim: hold iommu_lock across dma_unmap passthrough transition Sasha Levin
@ 2026-10-19 14:29 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-19 14:29 UTC (permalink / raw)
To: Sasha Levin; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> From: Xiong Weimin <xiongweimin@kylinos.cn>
>
> [ Upstream commit 7b411448c668bfab0d0e572cee5a592cea396ce7 ]
>
> vdpasim_dma_map() updates the IOTLB and the passthrough (iommu_pt)
> state under iommu_lock. vdpasim_dma_unmap() clears iommu_pt and
> resets the IOTLB before taking iommu_lock, then deletes the mapping
> while holding the lock.
>
> A concurrent dma_map(), dma_unmap(), or reset path that also touches
> the same address space can therefore observe or modify the IOTLB and
> iommu_pt state without consistent locking.
>
> Perform the passthrough transition and range deletion under the same
> iommu_lock scope, matching dma_map().
>
> Tested-on: openEuler VM (6.16.8, /usr/src/linux-6.16.8)
> Tested-by: Xiong Weimin <xiongweimin@kylinos.cn>
>
> Signed-off-by: Xiong Weimin <xiongweimin@kylinos.cn>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Message-ID: <20260626020545.607600-3-15927021679@163.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-159-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH AUTOSEL 6.18-6.1] drm/virtio: reclaim pending vbufs before tearing down vqs
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.15] virtio_ring: fix infinite loop in virtnet_poll_cleantx when device is broken Sasha Levin
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-6.6] vdpa_sim: hold iommu_lock across dma_unmap passthrough transition Sasha Levin
@ 2026-10-19 13:47 ` Sasha Levin
2026-10-19 14:51 ` sashiko-bot
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.1] vdpa: alibaba: add missing MODULE_DEVICE_TABLE() Sasha Levin
` (6 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Sasha Levin @ 2026-10-19 13:47 UTC (permalink / raw)
To: patches, stable
Cc: Anuj Bolewar, syzbot+06f9b2a53ba4a5a47644, Dmitry Osipenko,
Sasha Levin, airlied, kraxel, maarten.lankhorst, mripard,
tzimmermann, simona, dri-devel, virtualization, linux-kernel
From: Anuj Bolewar <bolewara@gmail.com>
[ Upstream commit 61d85f99b5a55d4f717c4bb2f4c4acabf22ee3fc ]
virtio_gpu_free_vbufs() destroys the vbufs kmem_cache after the virtqueues
have already been released. Commands that were queued but never completed
by the device leave their vbuffers stranded in the virtqueue, so the cache
still holds live objects when virtio_gpu_deinit() tears everything down.
This triggers a WARNING in virtio_gpu_free_vbufs:
BUG virtio-gpu-vbufs (Not tainted): Objects remaining in cache
on __kmem_cache_shutdown()
Drain any buffers still sitting in the control and cursor virtqueues in
virtio_gpu_deinit() after the device has been reset and before the
virtqueues are deleted, following the same pattern used by virtio_console's
remove_vqs(). Each reclaimed buffer is released with free_vbuf(), dropping
the reference on any GEM objects it holds. Pending RESOURCE_UNREF
commands are handled as well: their resp_cb_data still references a GEM
object, so it is cleaned up with virtio_gpu_cleanup_object() to avoid
leaking it on teardown.
Reported-by: syzbot+06f9b2a53ba4a5a47644@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=06f9b2a53ba4a5a47644
Signed-off-by: Anuj Bolewar <bolewara@gmail.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Link: https://patch.msgid.link/20260802-virtio-gpu-reclaim-vbufs-v2-1-5767fb860691@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
# Stable backport analysis: `drm/virtio: reclaim pending vbufs before
tearing down vqs`
Local tree: **6.12.111** (`p-6.12-3-gc504cf0baa3d8`). Verdict is for
this checkout only.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[drm/virtio] [reclaim] [drain leftover virtqueue buffers before
destroying vbufs cache]`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Reported-by: syzbot+06f9b2a53ba4a5a47644@syzkaller.appspotmail.com` —
fuzzer-found, reproducible
- `Closes: https://syzkaller.appspot.com/bug?extid=06f9b2a53ba4a5a47644`
- `Signed-off-by: Anuj Bolewar <bolewara@gmail.com>` (author)
- `Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>`
(virtio-gpu maintainer)
- `Link: https://patch.msgid.link/20260802-virtio-gpu-reclaim-
vbufs-v2-1-5767fb860691@gmail.com`
- No `Fixes:` tag (expected for this review pipeline)
- Ignore pipeline `Signed-off-by: Sasha Levin` / `[Upstream commit ...]`
if present
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `virtio_gpu_free_vbufs()` calls `kmem_cache_destroy()` after
VQs are already deleted. In-flight commands stay attached to the
virtqueue, so the `virtio-gpu-vbufs` cache still has live objects.
- **Symptom:** SLUB `BUG virtio-gpu-vbufs: Objects remaining on
__kmem_cache_shutdown()` plus `WARNING` in `virtio_gpu_free_vbufs` /
`kmem_cache_destroy`, on device removal.
- **Root cause:** `virtio_gpu_deinit()` does `virtio_reset_device()`
then `del_vqs()` without draining unused buffers.
- **Fix:** After reset, before `del_vqs()`, detach leftover buffers
(same idea as `virtio_console` `remove_vqs()`), `free_vbuf()` them,
drop GEM arrays, and `virtio_gpu_cleanup_object()` pending
`RESOURCE_UNREF` GEM objects.
- **Version info:** none in the message; code has been this way for
years.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. Explicit leak/WARNING fix on teardown. v2 also
closes a GEM leak for stranded `RESOURCE_UNREF` commands.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- `drivers/gpu/drm/virtio/virtgpu_drv.h`: +1 declaration
- `drivers/gpu/drm/virtio/virtgpu_kms.c`: +1 call in
`virtio_gpu_deinit()`
- `drivers/gpu/drm/virtio/virtgpu_vq.c`: +15, new
`virtio_gpu_reclaim_vbufs()`
- **Total: +17 / −0.** Single-subsystem surgical fix.
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** `virtio_gpu_deinit()` flushes work, resets the device,
deletes VQs. Later `virtio_gpu_release()` → `virtio_gpu_free_vbufs()`
→ `kmem_cache_destroy(vgdev->vbufs)` with any still-queued `vbuffer`s
live.
- **After:** Same sequence, plus `virtio_gpu_reclaim_vbufs()` between
reset and `del_vqs()`. Control queue: detach unused bufs, put GEM
arrays, cleanup `resp_cb_data`, `free_vbuf()`. Cursor queue: detach
and `free_vbuf()`.
- Path: device removal / unbind / probe-fail after successful
`virtio_gpu_init()`.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** resource leak + teardown WARNING (stranded slab objects;
GEM refs left held).
- **Mechanism:** `virtqueue_detach_unused_buf()` after reset, then
existing `free_vbuf()` / `virtio_gpu_array_put_free()` /
`virtio_gpu_cleanup_object()`.
- In this tree `resp_cb_data` is only set in
`virtio_gpu_cmd_unref_resource()` to a `struct virtio_gpu_object *`,
so the extra cleanup is specific and correct here.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: documented virtio shutdown helper; same pattern as
`virtio_console`.
- Minimal; no API/feature additions.
- Regression risk is low. After `drm_dev_unplug()`, `drm_dev_enter()`
fails, so `array_put_free()` → last GEM put →
`virtio_gpu_cmd_unref_resource()` takes the existing `-ENODEV` path
(`free_vbuf` + `virtio_gpu_cleanup_object`) instead of re-queuing.
Both VQs are always created in `virtio_gpu_init()`
(`vqs[0]`/`vqs[1]`), so no NULL `cursorq.vq` on the deinit path.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- `virtio_gpu_free_vbufs()` / `kmem_cache_destroy`: `f5985bf9cadd4e`
(Gerd Hoffmann, 2017-03-01, “drm: virtio: use kmem_cache”) — ancestor
of this HEAD.
- `virtio_gpu_deinit()` body: `dc5698e80cf724` (Dave Airlie, 2013) plus
later `virtio_reset_device()` (`d9679d0013a668`, 2021).
- Bug is old; present throughout 6.12.y.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Not applicable. Buggy teardown is in this tree.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Recent virtio GPU work here is unrelated (EDID bounds, deadlock,
fence, KMS removal). No reclaim helper already present. `cec7f1774605a`
is a different probe double-free. Standalone patch, not “X/Y” of a
larger series (cover said “Patch 1” of a 1-patch series).
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Anuj Bolewar has no virtio GPU commits in this tree. Dmitry
Osipenko is the virtio-gpu maintainer (SOB + “Applied to drm-misc-
fixes”).
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Uses `virtqueue_detach_unused_buf`, `free_vbuf`,
`virtio_gpu_array_put_free`, `virtio_gpu_cleanup_object` — all present.
No panic-path helpers required. Can apply standalone.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c <sha>` cannot match: commit is not in this tree (candidate
only).
- Lore search via lore.kernel.org returned 403.
- Recovered from Openwall/LKML:
- v1: drain only (`objs` + `free_vbuf`)
- v2: also `virtio_gpu_cleanup_object(resp_cb_data)`
- Applied version is v2
- Dmitry Osipenko, 2026-08-14: **“Applied to drm-misc-fixes, thanks!”**
— no NAK.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record: Recipients included Airlie, Hoffmann, Osipenko, Gurchetan Singh,
Chia-I Wu, dri-devel, virtualization, syzkaller-bugs. Maintainer applied
it. No NAK found.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record (syzkaller, extid `06f9b2a53ba4a5a47644`):
- Title: `WARNING in virtio_gpu_free_vbufs`
- Labels: `prio:high`
- Assessment: DenialOfService exploitable; VM guest and host trigger;
not unprivileged
- Stack: `kmem_cache_destroy` → `virtio_gpu_free_vbufs` →
`virtio_gpu_release` → `drm_dev_put` → `virtio_dev_remove` → PCI
`remove_store`
- 13 crashes; first ~2026-07-29
- Syzbot lists upstream fix `61d85f99b5a5` (that SHA is not in this
tree; `git merge-base --is-ancestor` was not used on an unknown remote
SHA)
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: 1-patch series. v2 is the applied leak-complete version.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable search 403. **UNVERIFIED** whether
stable@ already discussed it. Not used as a NO reason.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `virtio_gpu_reclaim_vbufs` (new), `virtio_gpu_deinit` (call
site), `free_vbuf`, `virtio_gpu_array_put_free`,
`virtio_gpu_cleanup_object`.
**Step 5.2: TRACE CALLERS**
Record:
- `virtio_gpu_deinit()` from `virtio_gpu_probe` error path after
`virtio_gpu_init()` (`err_deinit`) and from `virtio_gpu_remove()`
after `drm_dev_unplug()`.
- Teardown/unbind, not a hot path.
**Step 5.3: TRACE CALLEES**
Record: `virtqueue_detach_unused_buf` (documented for shutdown/reset),
GEM put, object cleanup, `kmem_cache_free`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Userspace/sysfs PCI remove (privileged) or guest virtio-gpu
unplug. Syzbot hit it from `remove_store`. Reachable on any system that
unbinds/unplugs virtio-gpu with commands still in the VQ (unresponsive
device, hot-unplug, fuzz).
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same drain exists in `virtio_console.c` `remove_vqs()`,
`virtio_net`, `virtio_crypto`, vsock, etc. virtio-gpu was missing it.
semcode: not available; used `git grep` / file reads.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.**
```283:293:drivers/gpu/drm/virtio/virtgpu_kms.c
void virtio_gpu_deinit(struct drm_device *dev)
{
...
virtio_reset_device(vgdev->vdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
}
```
```83:87:drivers/gpu/drm/virtio/virtgpu_vq.c
void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev)
{
kmem_cache_destroy(vgdev->vbufs);
vgdev->vbufs = NULL;
}
```
`virtio_gpu_reclaim_vbufs` is **not** in this tree. `git log
--grep='reclaim pending vbufs'` on those files: empty.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply expected.** Actual v2 context matches 6.12 (no
panic helpers required). Line numbers differ slightly; 3-line context
around reset/`del_vqs` and `free_vbuf`/`reclaim_vbufs` is the same.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No equivalent drain. KMS-disabled removal fix `38a5f891cda6d` is
unrelated (atomic shutdown).
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: SUBSYSTEM AND CRITICALITY**
Record: `drivers/gpu/drm/virtio` — **IMPORTANT** for VMs/QEMU guests
(common virtio-gpu), not core mm/VFS. Not staging.
**Step 7.2: SUBSYSTEM ACTIVITY**
Record: Mature driver (2013+), still receiving 6.12 stable fixes. Bug is
long-standing, not a brand-new subsystem.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: WHO IS AFFECTED**
Record: virtio-gpu users (`CONFIG_DRM_VIRTIO_GPU`) — QEMU/KVM/cloud VMs,
driver unbind, PCI remove, module unload.
**Step 8.2: TRIGGER CONDITIONS**
Record: Device teardown with commands still on the control/cursor VQ.
Syzbot: PCI `remove`. Privileged on the host; a VM guest can trigger it.
Not every boot, but a realistic hot-unplug/reset path.
**Step 8.3: FAILURE MODE SEVERITY**
Record:
- `kmem_cache_destroy()` **always** `WARN`s if objects remain
(`mm/slab_common.c` ~547), not only `CONFIG_SLUB_DEBUG`.
- With SLUB_DEBUG, additional `slab_bug` / `__slab_err`.
- If shutdown fails, the cache is **not** released (`if (err) return`) →
cache leak plus leaked vbufs/GEM.
- Syzbot: DoS, `prio:high`.
- Severity: **HIGH** (kernel WARNING/taint, resource leak on remove;
panic if `panic_on_warn`).
**Step 8.4: RISK-BENEFIT RATIO**
Record: **Benefit high** (stops WARNING + leak on a common VM GPU
driver). **Risk low** (17 lines, existing helpers, maintainer-acked).
Ratio favors backport.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: EVIDENCE**
**FOR:**
- Real, syzbot-reproduced teardown bug
- Unconditional `WARN` + slab/GEM leak
- virtio-gpu is widely used in VMs on 6.12 LTS
- Small, obviously correct drain matching other virtio drivers
- All callees exist here; bug exists here
- Maintainer applied v2 to drm-misc-fixes
**AGAINST:**
- Trigger is device removal, not a hot path
- Needs privilege / VM unplug (not a local unprivileged syscall crash)
- No `Fixes:` / stable Cc in the original (not a negative under these
rules)
**UNRESOLVED:**
- lore.kernel.org 403; stable@ discussion not fetched
- `b4 dig` could not resolve a SHA that is not in this tree
**Step 9.2: STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — v2, syzbot, maintainer
applied
2. Real user-visible bug? **PASS** — WARNING + leak on virtio-gpu remove
3. Important (crash/leak/DoS)? **PASS** — HIGH; syzbot prio:high / DoS
4. Small and contained? **PASS** — +17 lines, 3 files, one driver
5. No new features/APIs? **PASS** — internal helper only
6. Applies to local tree? **PASS** — buggy code present; helpers present
**Step 9.3: EXCEPTION CATEGORIES**
Record: Not ID/quirk/DT/docs. Ordinary important bugfix.
**Step 9.4: DECISION**
This tree has the bug and the dependencies. The fix is small, matches
established virtio teardown, and stops a syzbot-confirmed WARNING plus
object leak on virtio-gpu teardown. It should be backported to this 6.12
tree.
---
## What the commit solves for stable users
On virtio-gpu unbind/unplug, in-flight command buffers stay in the
virtqueues. Destroying the `virtio-gpu-vbufs` cache then WARNs and leaks
those buffers and any GEM objects they hold (including pending
`RESOURCE_UNREF`). 6.12 guests using virtio-gpu hit this on device
removal.
## Risk vs benefit
Benefit: no WARN/taint and no leaked GEM/vbufs on a common VM driver.
Risk: low; one drain after reset, before `del_vqs()`, using APIs already
in 6.12.
## Concerns
None that block backport. Header context in some expanded diffs mentions
later panic helpers; the real v2 hunk does not need them. Apply as the
17-line v2.
---
## Verification
- **[Phase 1]** Tags: syzbot Reported-by, Closes syzkaller URL, Osipenko
SOB
- **[Phase 2]** Diff: +17 across 3 virtio GPU files; drain after reset
- **[Phase 2]** `resp_cb_data` only assigned in
`virtio_gpu_cmd_unref_resource()` in this tree
- **[Phase 3]** `git describe HEAD` / Makefile: 6.12.111;
`p-6.12-3-gc504cf0baa3d8`
- **[Phase 3]** blame: `kmem_cache_destroy` in `virtio_gpu_free_vbufs`
since `f5985bf9cadd4e` (2017); deinit without drain since original
driver
- **[Phase 3]** `git log --grep='reclaim pending vbufs' --
drivers/gpu/drm/virtio/`: not present
- **[Phase 4]** Openwall v2 + LKML: Osipenko “Applied to drm-misc-fixes”
- **[Phase 4]** syzbot: WARNING in `virtio_gpu_free_vbufs`, prio:high,
PCI remove stack, 13 crashes
- **[Phase 4]** lore.kernel.org 403; `b4 dig` no local commit —
UNVERIFIED beyond Openwall/LKML/syzbot
- **[Phase 5]** Callers: `virtio_gpu_drv.c` probe fail +
`virtio_gpu_remove`
- **[Phase 5]** Both VQs assigned in `virtio_gpu_init()` lines 217–218
- **[Phase 6]** Local `virtio_gpu_deinit` / `virtio_gpu_free_vbufs`
match the buggy sequence
- **[Phase 6]** `virtqueue_detach_unused_buf`,
`virtio_gpu_array_put_free`, `virtio_gpu_cleanup_object` exist
- **[Phase 8]** `kmem_cache_destroy` WARNs if `__kmem_cache_shutdown`
returns nonzero (`mm/slab_common.c`); cache not released on error
- **UNVERIFIED:** whether this SHA is already in some other stable
series (irrelevant; not in this tree)
**YES**
drivers/gpu/drm/virtio/virtgpu_drv.h | 1 +
drivers/gpu/drm/virtio/virtgpu_kms.c | 1 +
drivers/gpu/drm/virtio/virtgpu_vq.c | 15 +++++++++++++++
3 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 51b78f8a8aa2e..d52f35a09a1b0 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -355,6 +355,7 @@ void virtio_gpu_array_put_free_work(struct work_struct *work);
/* virtgpu_vq.c */
int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev);
+void virtio_gpu_reclaim_vbufs(struct virtio_gpu_device *vgdev);
void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo,
struct virtio_gpu_object_params *params,
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 4a25347734fda..01ea5cd5ea7f1 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -289,6 +289,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
flush_work(&vgdev->cursorq.dequeue_work);
flush_work(&vgdev->config_changed_work);
virtio_reset_device(vgdev->vdev);
+ virtio_gpu_reclaim_vbufs(vgdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 244a791ab915e..f7e7d212fc67c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -207,6 +207,21 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
kmem_cache_free(vgdev->vbufs, vbuf);
}
+void virtio_gpu_reclaim_vbufs(struct virtio_gpu_device *vgdev)
+{
+ struct virtio_gpu_vbuffer *vbuf;
+
+ while ((vbuf = virtqueue_detach_unused_buf(vgdev->ctrlq.vq))) {
+ if (vbuf->objs)
+ virtio_gpu_array_put_free(vbuf->objs);
+ if (vbuf->resp_cb_data)
+ virtio_gpu_cleanup_object(vbuf->resp_cb_data);
+ free_vbuf(vgdev, vbuf);
+ }
+ while ((vbuf = virtqueue_detach_unused_buf(vgdev->cursorq.vq)))
+ free_vbuf(vgdev, vbuf);
+}
+
static void reclaim_vbufs(struct virtqueue *vq, struct list_head *reclaim_list)
{
struct virtio_gpu_vbuffer *vbuf;
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH AUTOSEL 6.18-6.1] drm/virtio: reclaim pending vbufs before tearing down vqs
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-6.1] drm/virtio: reclaim pending vbufs before tearing down vqs Sasha Levin
@ 2026-10-19 14:51 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-19 14:51 UTC (permalink / raw)
To: Sasha Levin; +Cc: dri-devel, virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] drm/virtio: missing pending_commands reset in virtio_gpu_reclaim_vbufs()
- [High] drm/virtio: race in virtio_gpu_deinit() flushing work before device reset
- [High] drm/virtio: uninterruptible hang in virtio_gpu_reclaim_vbufs() on imported dmabuf
--
commit f24f642758ec3795a40ce21fd3080771f4fdffb2
Author: Anuj Bolewar <bolewara@gmail.com>
drm/virtio: reclaim pending vbufs before tearing down vqs
This commit drains unused vbuffers from the control and cursor virtqueues in
virtio_gpu_deinit() after resetting the device and before deleting the
virtqueues, releasing any held GEM object references and pending
RESOURCE_UNREF objects before the vbufs kmem_cache is destroyed.
Link: https://patch.msgid.link/20260802-virtio-gpu-reclaim-vbufs-v2-1-5767fb860691@gmail.com
[ ... ]
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index 1c15cbf326b78..59173746c9494 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -286,6 +286,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
> flush_work(&vgdev->cursorq.dequeue_work);
> flush_work(&vgdev->config_changed_work);
> virtio_reset_device(vgdev->vdev);
> + virtio_gpu_reclaim_vbufs(vgdev);
> vgdev->vdev->config->del_vqs(vgdev->vdev);
> }
[Severity: High]
Could flushing the workqueues before virtio_reset_device() in
virtio_gpu_deinit() race with virtio_gpu_reclaim_vbufs() and del_vqs()?
In drivers/gpu/drm/virtio/virtgpu_kms.c:virtio_gpu_deinit():
flush_work(&vgdev->obj_free_work);
flush_work(&vgdev->ctrlq.dequeue_work);
flush_work(&vgdev->cursorq.dequeue_work);
flush_work(&vgdev->config_changed_work);
virtio_reset_device(vgdev->vdev);
virtio_gpu_reclaim_vbufs(vgdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
Until virtio_reset_device(vgdev->vdev) completes, the virtio device can still
raise virtqueue and config interrupts (virtio_gpu_ctrl_ack(),
virtio_gpu_cursor_ack(), and virtio_gpu_config_changed()), which call
schedule_work() and can re-queue ctrlq.dequeue_work, cursorq.dequeue_work, and
config_changed_work after flush_work() has already returned. In the
virtio_gpu_probe() -> err_deinit path, drm_dev_unplug() is not called, so
config_changed_work can also queue new buffers into ctrlq.vq.
Since virtio_reset_device() does not cancel or flush already-scheduled work,
virtio_gpu_dequeue_ctrl_func() and virtio_gpu_dequeue_cursor_func() can run
concurrently with virtio_gpu_reclaim_vbufs() and del_vqs():
spin_lock(&vgdev->ctrlq.qlock);
do {
virtqueue_disable_cb(vgdev->ctrlq.vq);
reclaim_vbufs(vgdev->ctrlq.vq, &reclaim_list);
} while (!virtqueue_enable_cb(vgdev->ctrlq.vq));
spin_unlock(&vgdev->ctrlq.qlock);
Because virtio_gpu_reclaim_vbufs() calls virtqueue_detach_unused_buf() without
holding ctrlq.qlock or cursorq.qlock while dequeue_work is calling
virtqueue_get_buf(), can this corrupt virtqueue descriptor state (such as
vq->split.desc_state[i].data, vq->free_head, and vq->vq.num_free), double-free
vbuffers, or access the virtqueues while del_vqs() is freeing them?
Additionally, flush_work(&vgdev->obj_free_work) runs before
flush_work(&vgdev->ctrlq.dequeue_work), even though
virtio_gpu_dequeue_ctrl_func() schedules vgdev->obj_free_work via
virtio_gpu_array_put_free_delayed(). Can virtio_gpu_array_put_free_work() end
up running concurrently with or after virtio_gpu_release() ->
virtio_gpu_free_vbufs() and touch vgdev->vbufs while or after the kmem_cache
is destroyed?
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 8181b22b9b46a..68cf7e4996884 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -207,6 +207,21 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
> kmem_cache_free(vgdev->vbufs, vbuf);
> }
>
> +void virtio_gpu_reclaim_vbufs(struct virtio_gpu_device *vgdev)
> +{
> + struct virtio_gpu_vbuffer *vbuf;
> +
> + while ((vbuf = virtqueue_detach_unused_buf(vgdev->ctrlq.vq))) {
> + if (vbuf->objs)
> + virtio_gpu_array_put_free(vbuf->objs);
> + if (vbuf->resp_cb_data)
> + virtio_gpu_cleanup_object(vbuf->resp_cb_data);
> + free_vbuf(vgdev, vbuf);
> + }
> + while ((vbuf = virtqueue_detach_unused_buf(vgdev->cursorq.vq)))
> + free_vbuf(vgdev, vbuf);
> +}
[Severity: High]
Should virtio_gpu_reclaim_vbufs() also reset vgdev->pending_commands to avoid
a use-after-free on vgdev->ctrlq.vq in virtio_gpu_notify()?
Whenever a control buffer is queued via virtio_gpu_queue_ctrl_sgs() (or
virtio_gpu_panic_queue_ctrl_sgs()), vgdev->pending_commands is incremented:
ret = virtqueue_add_sgs(vq, sgs, outcnt, incnt, vbuf, GFP_ATOMIC);
WARN_ON(ret);
vbuf->seqno = ++vgdev->ctrlq.seqno;
trace_virtio_gpu_cmd_queue(vq, virtio_gpu_vbuf_ctrl_hdr(vbuf), vbuf->seqno);
atomic_inc(&vgdev->pending_commands);
Several command submission paths (such as virtio_gpu_object_create(),
virtio_gpu_vram_create(), virtio_gpu_mode_dumb_create(),
virtio_gpu_resource_create_ioctl(), virtio_gpu_resource_create_blob_ioctl(),
virtio_gpu_resource_assign_uuid(), virtio_gpu_prepare_imported_obj(), and
virtio_gpu_create_context()) queue vbuffers into vgdev->ctrlq.vq without
immediately calling virtio_gpu_notify(vgdev), leaving vgdev->pending_commands
non-zero.
When virtio_gpu_reclaim_vbufs() detaches those vbuffers, it does not reset or
decrement vgdev->pending_commands, and virtio_gpu_deinit() immediately frees
vgdev->ctrlq.vq via vgdev->vdev->config->del_vqs(vgdev->vdev).
If an open drm_file later releases a GEM object or closes (calling
virtio_gpu_gem_object_close(), virtio_gpu_free_object(),
virtio_gpu_vram_free(), virtgpu_dma_buf_free_obj(), or
virtio_gpu_driver_postclose() -> virtio_gpu_notify()), virtio_gpu_notify()
has no drm_dev_enter() guard and only checks vgdev->pending_commands:
void virtio_gpu_notify(struct virtio_gpu_device *vgdev)
{
bool notify;
if (!atomic_read(&vgdev->pending_commands))
return;
spin_lock(&vgdev->ctrlq.qlock);
atomic_set(&vgdev->pending_commands, 0);
notify = virtqueue_kick_prepare(vgdev->ctrlq.vq);
spin_unlock(&vgdev->ctrlq.qlock);
if (notify)
virtqueue_notify(vgdev->ctrlq.vq);
}
Won't virtio_gpu_notify() see the stale non-zero vgdev->pending_commands and
call virtqueue_kick_prepare() and virtqueue_notify() on the freed
vgdev->ctrlq.vq? Also, if virtio_gpu_array_put_free(vbuf->objs) inside
virtio_gpu_reclaim_vbufs() drops the final reference to a created GEM object,
virtio_gpu_free_object() synchronously calls virtio_gpu_notify(vgdev), which
would kick vgdev->ctrlq.vq after virtio_reset_device() has already reset the
device while virtqueue_detach_unused_buf() is draining the ring.
[Severity: High]
Can calling virtio_gpu_array_put_free(vbuf->objs) from
virtio_gpu_reclaim_vbufs() hang uninterruptibly in dma_fence_wait() when
releasing an imported DMA-BUF GEM object?
When VIRTGPU_PARAM_RESOURCE_BLOB is enabled without virgl 3D
(vgdev->has_resource_blob && !vgdev->has_virgl_3d), importing a DMA-BUF via
virtgpu_gem_prime_import() creates a GEM object with
obj->funcs = &virtgpu_gem_dma_buf_funcs (.free = virtgpu_dma_buf_free_obj) and
bo->attached = true. If that object is used on the primary plane via
virtio_gpu_primary_plane_update() -> virtio_gpu_resource_flush() and the
50ms wait times out, the vbuf remains in ctrlq.vq holding a reference to the
imported GEM object in vbuf->objs.
During virtio_gpu_remove(), drm_dev_unplug(dev) marks the DRM device
unplugged, drm_atomic_helper_shutdown(dev) drops the plane state's framebuffer
reference, and virtio_gpu_deinit() resets the device and calls
virtio_gpu_reclaim_vbufs(). When virtio_gpu_array_put_free(vbuf->objs) drops
the last reference to the imported object, it triggers
virtgpu_dma_buf_free_obj() -> virtgpu_dma_buf_unmap() ->
virtio_gpu_detach_object_fenced(bo):
int virtio_gpu_detach_object_fenced(struct virtio_gpu_object *bo)
{
struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
struct virtio_gpu_fence *fence;
if (!bo->attached)
return 0;
fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0);
if (!fence)
return -ENOMEM;
virtio_gpu_object_detach(vgdev, bo, fence);
virtio_gpu_notify(vgdev);
dma_fence_wait(&fence->f, false);
dma_fence_put(&fence->f);
return 0;
}
Inside virtio_gpu_object_detach() -> virtio_gpu_cmd_resource_detach_backing()
-> virtio_gpu_queue_fenced_ctrl_buffer() -> virtio_gpu_queue_ctrl_sgs(),
drm_dev_enter() returns false because the device is unplugged:
if (!drm_dev_enter(vgdev->ddev, &idx)) {
if (fence && vbuf->objs)
virtio_gpu_array_unlock_resv(vbuf->objs);
free_vbuf(vgdev, vbuf);
return -ENODEV;
}
Since virtio_gpu_queue_ctrl_sgs() returns -ENODEV before
virtio_gpu_fence_emit() is called, and virtio_gpu_object_detach() ignores this
error, won't dma_fence_wait(&fence->f, false) in
virtio_gpu_detach_object_fenced() sleep forever waiting on a fence that was
never emitted and can never be signaled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-179-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH AUTOSEL 6.18-6.1] vdpa: alibaba: add missing MODULE_DEVICE_TABLE()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-6.1] drm/virtio: reclaim pending vbufs before tearing down vqs Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
2026-10-19 14:49 ` sashiko-bot
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] vdpa: octeon_ep: " Sasha Levin
` (5 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Michael S. Tsirkin, Sasha Levin, jasowangio,
eperezma, virtualization, linux-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit 7eeda5e2487276ed87960655fdae1c97d9779a0e ]
The driver has a match table for the pci bus wired into its driver
structure, but the table is not exported with MODULE_DEVICE_TABLE().
Add the missing MODULE_DEVICE_TABLE() entry so module alias information
is generated for automatic module loading.
This is a source-level fix. It does not claim dynamic hardware
reproduction; the evidence is the driver-owned match table, its use by
the driver registration structure, and the missing module alias
publication.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-ID: <20260704152732.55338-1-pengpeng@iscas.ac.cn>
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: `[vdpa: alibaba:] [add] [missing MODULE_DEVICE_TABLE() so PCI
aliases are generated for autoload]`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
From upstream commit `7eeda5e2487276ed87960655fdae1c97d9779a0e`
(verified with `git show`):
- `Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>` (author)
- `Signed-off-by: Michael S. Tsirkin <mst@redhat.com>` (virtio/vDPA
maintainer)
- `Message-ID: <20260704152732.55338-1-pengpeng@iscas.ac.cn>`
No `Fixes:`, no `Reported-by:`, no `Tested-by:`, no `Reviewed-by:`, no
`Acked-by:`, no `Cc: stable`, no `Link:` (besides Message-ID). Absence
of `Fixes:`/`Cc: stable` is expected for this review pipeline.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- Bug: `eni_pci_ids[]` is wired into `eni_vdpa_driver.id_table` but
never exported via `MODULE_DEVICE_TABLE()`.
- Symptom: `modpost`/`file2alias` never emits `pci:v...d...` aliases, so
depmod/udev will not autoload `alibaba-eni-vdpa` when the PCI device
appears.
- Version info: none in the message. Driver itself dates to v5.16-rc1.
- Root cause: missing alias publication, not a runtime logic error.
Author explicitly calls it a source-level fix (build-time module
metadata).
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a disguised cleanup. It is an autoload/hardware-enablement
bug: the match table exists and is used for probe matching, but is not
published for module loading. Same pattern as device-ID publication.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/vdpa/alibaba/eni_vdpa.c` (`+1` / `-0`)
- Function: none. Insert after `eni_pci_ids[]`, before
`eni_vdpa_driver`.
- Scope: single-file, one-line surgical change.
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record: Before, `eni_pci_ids` is only referenced by `.id_table` (in-
kernel PCI matching after the module is already loaded). After,
`MODULE_DEVICE_TABLE(pci, eni_pci_ids)` also creates
`__mod_pci__eni_pci_ids_device_table`, which `scripts/mod/file2alias.c`
(`do_pci_entry`) turns into PCI aliases. No probe/remove/runtime path
changes. Built-in (`!MODULE`) the macro is a no-op
(`include/linux/module.h`).
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: Category **(h) hardware enablement / missing device-ID
publication**. Mechanism: without `MODULE_DEVICE_TABLE`, udev never
loads the module on device discovery. Sibling PCI vDPA drivers (`ifcvf`,
`snet`) already export their tables.
**Step 2.4: ASSESS THE FIX QUALITY**
Record: Obviously correct; matches the established PCI-driver pattern.
Minimal. Regression risk is negligible: no locks, no APIs, no data-
structure changes, no runtime control-flow change. `linux/module.h` is
already included (line 12).
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record: `git blame -L 540,560` shows the ID table and `pci_driver` come
entirely from `e85087beedcae` (“eni_vdpa: add vDPA driver for Alibaba
ENI”, Wu Zongyong, 2021-10-29). Contained in `v5.16-rc1`. Present and
unchanged in this tree. Bug has existed for the entire life of the
driver.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. The introducing commit is `e85087beedcae`. `git
merge-base --is-ancestor e85087beedcae HEAD` succeeded. No later commit
in this tree added `MODULE_DEVICE_TABLE` for this driver.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Recent history is `e85087beedcae` → probe simplify → vDPA API
updates → probe error-path fix → `get_vq_size`. None of those touch the
PCI ID table. Standalone; not patch X/Y.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Pengpeng Hou is not the vDPA maintainer. Same day they sent an
identical `MODULE_DEVICE_TABLE` fix for `can: ctucanfd`, which **is
already in this 6.12 tree** as `877e99535dfb9` (`Cc: stable@kernel.org`,
`Fixes:`, Greg KH). Maintainer of this driver is Michael S. Tsirkin, who
applied the vDPA patch.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: No new helpers or structs. `MODULE_DEVICE_TABLE` has existed for
decades. Applies standalone. `git show
7eeda5e2487276ed87960655fdae1c97d9779a0e | git apply --check` succeeded
(exit 0).
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 7eeda5e2487276ed87960655fdae1c97d9779a0e` →
https://patch.msgid.link/20260704152732.55338-1-pengpeng@iscas.ac.cn
- `b4 dig -a`: single revision (v1 only).
- Thread (saved mbox): MST replied the same day asking what the “source-
level fix” boilerplate paragraph meant. No NAK, no requested code
change. He later SOB’d and applied it (CommitDate 2026-08-19). No
reviewer asked for `Cc: stable`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc were Michael S. Tsirkin, Jason Wang, Xuan
Zhuo, Eugenio Pérez, `virtualization@lists.linux.dev`, `linux-
kernel@vger.kernel.org`. Right maintainers. Only MST replied.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No `Reported-by`, no bugzilla/syzbot. Author found it by
inspecting the driver-owned match table vs missing alias export. No
evidence of a live user report.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Same-day batch of missing-`MODULE_DEVICE_TABLE` patches
(ctucanfd, octeon_ep vDPA, cadence PCI, this one). ctucanfd already
landed in this stable tree. octeon_ep in *this* checkout also still
lacks `MODULE_DEVICE_TABLE` (`octep_pci_vdpa_map` at
`drivers/vdpa/octeon_ep/octep_vdpa_main.c`); that is a separate
candidate and is not a dependency of this patch.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable searches returned 403; web search found
no stable-list discussion of this specific Alibaba patch. The related
ctucanfd patch *was* sent to stable and is in this tree. UNVERIFIED: no
positive evidence this exact SHA was discussed on stable@; no evidence
it was rejected either.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: No functions modified. Added `MODULE_DEVICE_TABLE(pci,
eni_pci_ids)` next to `eni_pci_ids[]` / `eni_vdpa_driver`.
**Step 5.2: TRACE CALLERS**
Record: `eni_pci_ids` is used as `.id_table` of `eni_vdpa_driver`,
registered via `module_pci_driver()`. PCI core matches on
enumeration/hotplug. The new alias is consumed at module-install time by
depmod, then at runtime by udev/systemd-udevd. Not an interrupt or
syscall path.
**Step 5.3: TRACE CALLEES**
Record: Macro only. Expands to an alias symbol for `file2alias.c`. No
allocations, locks, or I/O.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Reachable when `CONFIG_ALIBABA_ENI_VDPA=m` and a matching PCI
device appears (Red Hat Qumranet vendor `0x1af4`, transitional virtio-
net `VIRTIO_TRANS_ID_NET`, subsystem virtio-net). Userspace does not
need a special syscall; PCI discovery is enough. Built-in builds are
unaffected (macro is empty).
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: In-tree vDPA PCI drivers `ifcvf` and `snet` already have
`MODULE_DEVICE_TABLE(pci, ...)`. `eni_vdpa` and `octeon_ep` do not.
Systematic omission, not unique to Alibaba.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: Yes. Local tree is **6.12.111** (`git describe HEAD` →
`p-6.12-3-gc504cf0baa3d8`; `VERSION=6 PATCHLEVEL=12 SUBLEVEL=111`).
Current `eni_vdpa.c` has `eni_pci_ids` and `.id_table = eni_pci_ids`
with **no** `MODULE_DEVICE_TABLE`. `git merge-base --is-ancestor
7eeda5e2487276ed87960655fdae1c97d9779a0e HEAD` failed (fix not in this
tree). Bug introduced in `e85087beedcae` (v5.16-rc1), well before 6.12.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Surrounding lines match the upstream hunk exactly. `git apply
--check` clean. Expected difficulty: **clean apply**.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: `git log --grep='alibaba: add missing MODULE_DEVICE_TABLE'` on
this file is empty. No alternate autoload fix. ctucanfd’s analogous fix
is already here, which shows this class is accepted on 6.12.y.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: `drivers/vdpa/alibaba/` — vDPA PCI driver for Alibaba ENI.
**PERIPHERAL** (cloud-specific, `depends on PCI_MSI && X86`). Not core
mm/net, but a real distro-shipped driver.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: vDPA is actively maintained (recent ifcvf/mlx5/octeon_ep/vduse
fixes). The Alibaba file itself is mature; last functional commits are
API follow-ups plus a probe error-path fix. This is a long-standing
omission, not churn from new code.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Config- and driver-specific: `CONFIG_ALIBABA_ENI_VDPA=m` on x86
with Alibaba ENI (legacy virtio 0.9.5 transitional net PCI IDs). Built-
in (`=y`) already probes via the in-kernel `id_table`.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Trigger is ordinary PCI enumeration with the module not
preloaded. Common for distro modular kernels. Unprivileged users cannot
exploit it; the failure is “device never binds until someone
`modprobe`s”.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: Failure mode: driver never autoloads → vDPA/ENI NIC does not
come up. Not a panic, UAF, or corruption. Severity **MEDIUM** for
affected hardware (device non-functional until manual load). Matches the
device-ID/quirk exception, not a crash-class bug.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- Benefit: restores autoload for a driver that already exists in 6.12;
hardware that distros already ship a module for actually binds.
- Risk: **very low** (one line, build-time metadata only).
- Ratio: strongly favorable. Same class already taken into this tree
(`877e99535dfb9`).
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real functional bug: PCI match table present, aliases unpublished.
- One-line, obviously correct, applies cleanly to this 6.12 tree.
- Buggy code has been here since the driver was added (v5.16); the
upstream fix is not an ancestor of HEAD.
- Device-ID / autoload publication is an established stable exception.
- Identical pattern from the same author already backported here
(ctucanfd).
- Sibling vDPA PCI drivers already export `MODULE_DEVICE_TABLE`.
- Applied by the vDPA maintainer (MST) after seeing the patch.
- Zero runtime regression surface.
AGAINST:
- Not a crash, UAF, deadlock, or security issue.
- Niche (Alibaba ENI, x86, modular builds only).
- No user `Reported-by`.
- Author did not `Cc: stable` (expected for this review set; not a
negative).
- MST questioned the boilerplate wording, not the code.
UNRESOLVED:
- Could not fetch lore.kernel.org/stable HTML (403). No evidence of
rejection.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — one-line standard pattern;
maintainer applied it.
2. Fixes a real bug that affects users? **PASS** — modular autoload
fails for matching PCI devices.
3. Important issue? **PASS** under the device-ID / hardware-enablement
exception (severity MEDIUM, not crash-class).
4. Small and contained? **PASS** — +1 line, one file.
5. No new features or APIs? **PASS** — publishes IDs the driver already
matches on.
6. Can apply to the local tree? **PASS** — buggy code present; `git
apply --check` clean.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **Device ID / module alias publication for an existing driver.**
Equivalent to adding PCI IDs so hardware actually binds when built as a
module.
**Step 9.4: DECISION**
This should be backported to this 6.12 tree. The driver is already here,
the omission is real, the fix is trivial and has no runtime risk, and
this tree already accepted the same class of fix.
---
### What the commit solves and why it matters here
Without `MODULE_DEVICE_TABLE(pci, eni_pci_ids)`, `file2alias` never
emits PCI aliases for `alibaba-eni-vdpa`. On 6.12 systems that build
`CONFIG_ALIBABA_ENI_VDPA=m`, udev will not load the module when the ENI
virtio-net PCI device appears, so the vDPA device never probes unless
something else loads the module. The in-kernel `id_table` still works
after a manual `modprobe`; autoload is what is broken.
### Stable rules and risk
The change is one line, matches the PCI driver idiom used by
`ifcvf`/`snet`, and does not alter probe/remove. Risk is essentially
zero. Benefit is limited to Alibaba ENI modular users, but that is the
same justification used for PCI ID additions and for the ctucanfd
autoload fix already on this branch.
### Concerns
None material for this tree. No prerequisites. No backport rewrite
needed. octeon_ep has the same omission locally; that is independent.
---
Verification:
- [Phase 1] Parsed tags from `git show
7eeda5e2487276ed87960655fdae1c97d9779a0e`: SOB Hou, SOB Tsirkin,
Message-ID; no Fixes/Reported-by/Cc:stable
- [Phase 2] Diff: +1 line `MODULE_DEVICE_TABLE(pci, eni_pci_ids)` in
`eni_vdpa.c`; no function body change
- [Phase 2] Confirmed `MODULE_DEVICE_TABLE` expansion in
`include/linux/module.h` (alias for modules, empty if built-in)
- [Phase 2] Confirmed `do_pci_entry()` in `scripts/mod/file2alias.c`
generates `pci:` aliases from exported tables
- [Phase 3] `git blame -L 540,560`: ID table from `e85087beedcae`
(v5.16-rc1)
- [Phase 3] `git merge-base --is-ancestor e85087beedcae HEAD`: yes (bug
present)
- [Phase 3] `git merge-base --is-ancestor
7eeda5e2487276ed87960655fdae1c97d9779a0e HEAD`: no (fix absent)
- [Phase 3] Same-author ctucanfd fix `877e99535dfb9` already in this
tree
- [Phase 4] `b4 dig -c 7eeda5e…`:
https://patch.msgid.link/20260704152732.55338-1-pengpeng@iscas.ac.cn
- [Phase 4] `b4 dig -a`: v1 only
- [Phase 4] `b4 dig -w`: MST, Jason Wang, Xuan Zhuo, Eugenio Pérez,
virtualization@, lkml
- [Phase 4] mbox: MST asked about boilerplate wording; no NAK; later
applied
- [Phase 4] lore.kernel.org HTML fetches returned 403; used b4 mbox +
openwall copy instead
- [Phase 5] Callers: `module_pci_driver` → PCI probe; aliases consumed
by depmod/udev
- [Phase 5] Sibling drivers `ifcvf`/`snet` already have
`MODULE_DEVICE_TABLE`; `octeon_ep` still missing (independent)
- [Phase 6] `git describe HEAD` / `make kernelversion`: 6.12.111
(`p-6.12-3-gc504cf0baa3d8`)
- [Phase 6] Current `eni_vdpa.c` lines 540–559: table present,
`MODULE_DEVICE_TABLE` absent
- [Phase 6] `git apply --check` of the commit: success
- [Phase 7] Kconfig: `ALIBABA_ENI_VDPA` tristate, `depends on PCI_MSI &&
X86`
- [Phase 8] Failure mode: module autoload skip → device does not bind;
severity MEDIUM; risk very low
- UNVERIFIED: no stable@ thread found (lore/stable HTTP 403); did not
drive the decision
The fix is small, surgical, present as a real autoload bug in this 6.12
tree, and meets the device-ID/hardware-enablement exception.
**YES**
drivers/vdpa/alibaba/eni_vdpa.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c
index e476504db0c82..fd6fdba460945 100644
--- a/drivers/vdpa/alibaba/eni_vdpa.c
+++ b/drivers/vdpa/alibaba/eni_vdpa.c
@@ -545,6 +545,7 @@ static struct pci_device_id eni_pci_ids[] = {
VIRTIO_ID_NET) },
{ 0 },
};
+MODULE_DEVICE_TABLE(pci, eni_pci_ids);
static struct pci_driver eni_vdpa_driver = {
.name = "alibaba-eni-vdpa",
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH AUTOSEL 6.18-6.1] vdpa: alibaba: add missing MODULE_DEVICE_TABLE()
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.1] vdpa: alibaba: add missing MODULE_DEVICE_TABLE() Sasha Levin
@ 2026-10-19 14:49 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-19 14:49 UTC (permalink / raw)
To: Sasha Levin; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] vdpa: alibaba: exporting generic virtio-net PCI ID breaks virtio_pci binding
--
commit 56e367c85e2f18386c5a07775af8ff1e3da0457f
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>
vdpa: alibaba: add missing MODULE_DEVICE_TABLE()
This commit adds MODULE_DEVICE_TABLE(pci, eni_pci_ids) to the Alibaba ENI
vDPA driver so that PCI module alias entries are generated for automatic
module loading.
> diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c
> index e476504db0c82..fd6fdba460945 100644
> --- a/drivers/vdpa/alibaba/eni_vdpa.c
> +++ b/drivers/vdpa/alibaba/eni_vdpa.c
> @@ -545,6 +545,7 @@ static struct pci_device_id eni_pci_ids[] = {
> VIRTIO_ID_NET) },
> { 0 },
> };
> +MODULE_DEVICE_TABLE(pci, eni_pci_ids);
>
> static struct pci_driver eni_vdpa_driver = {
> .name = "alibaba-eni-vdpa",
[Severity: High]
Will exporting eni_pci_ids via MODULE_DEVICE_TABLE() cause eni_vdpa to
auto-load on standard non-Alibaba x86 VMs and race with virtio_pci for
generic transitional or legacy virtio-net PCI devices?
In drivers/vdpa/alibaba/eni_vdpa.c, eni_pci_ids matches the generic
Virtio 0.9.5 / transitional Virtio 1.0 network device IDs rather than an
Alibaba-specific PCI ID:
drivers/vdpa/alibaba/eni_vdpa.c:
static struct pci_device_id eni_pci_ids[] = {
{ PCI_DEVICE_SUB(PCI_VENDOR_ID_REDHAT_QUMRANET,
VIRTIO_TRANS_ID_NET,
PCI_SUBVENDOR_ID_REDHAT_QUMRANET,
VIRTIO_ID_NET) },
{ 0 },
};
These are the same PCI IDs (vendor 0x1af4, device 0x1000, subvendor
0x1af4, subdevice 0x0001) exposed by default virtio-net-pci devices in
QEMU/KVM and matched by virtio_pci_id_table in
drivers/virtio/virtio_pci_common.c:
static const struct pci_device_id virtio_pci_id_table[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_REDHAT_QUMRANET, PCI_ANY_ID) },
{ 0 }
};
MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
Previously, omitting MODULE_DEVICE_TABLE() prevented eni_vdpa from
publishing a modalias for 1af4:1000, so it was only loaded when explicitly
requested by userspace on Alibaba instances. With MODULE_DEVICE_TABLE()
added, udev/kmod will auto-load eni_vdpa on any x86 guest with a
transitional or legacy virtio-net-pci device when CONFIG_ALIBABA_ENI_VDPA=m
and CONFIG_VIRTIO_PCI=m.
In eni_vdpa_probe(), there is no hardware check distinguishing an Alibaba
ENI device from a standard QEMU virtio-net-pci device:
drivers/vdpa/alibaba/eni_vdpa.c:eni_vdpa_probe() {
...
ldev = &eni_vdpa->ldev;
ldev->pci_dev = pdev;
ret = vp_legacy_probe(ldev);
if (ret) {
ENI_ERR(pdev, "failed to probe legacy PCI device\n");
goto err;
}
...
}
If eni_vdpa probes the PCI device before virtio_pci, eni_vdpa_probe() ->
vp_legacy_probe() succeeds and claims the device into the vDPA subsystem
via vdpa_register_device(), preventing virtio_pci from binding and causing
the guest to lose its standard virtio_net network interface.
Was MODULE_DEVICE_TABLE() intentionally omitted here to avoid hijacking
generic legacy/transitional virtio-net PCI devices from virtio_pci?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-253-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH AUTOSEL 6.18-6.12] vdpa: octeon_ep: add missing MODULE_DEVICE_TABLE()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (3 preceding siblings ...)
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.1] vdpa: alibaba: add missing MODULE_DEVICE_TABLE() Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
2026-10-19 14:50 ` sashiko-bot
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] virtio_console: take a kref in find_port_by_vq() to fix port UAF Sasha Levin
` (4 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Michael S. Tsirkin, Sasha Levin, jasowangio,
eperezma, virtualization, linux-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit 346da62d9f9ccca81cf279c361137d1fe2fe0c7f ]
The driver has a match table for the pci bus wired into its driver
structure, but the table is not exported with MODULE_DEVICE_TABLE().
Add the missing MODULE_DEVICE_TABLE() entry so module alias information
is generated for automatic module loading.
This is a source-level fix. It does not claim dynamic hardware
reproduction; the evidence is the driver-owned match table, its use by
the driver registration structure, and the missing module alias
publication.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-ID: <20260705002546.85004-1-pengpeng@iscas.ac.cn>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.12.111** (`pending-6.12`). The one-line
`MODULE_DEVICE_TABLE()` addition belongs here: the Octeon vDPA PCI IDs
already exist, but they are not published for autoload.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[vdpa/octeon_ep] [add] [export the existing PCI match table
via MODULE_DEVICE_TABLE so userspace can autoload the module]**
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Verified from mainline `346da62d9f9cc` (`git log -1 --format=fuller`):
- `Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>` (author)
- `Signed-off-by: Michael S. Tsirkin <mst@redhat.com>` (virtio/vDPA
maintainer; applied the patch)
- `Message-ID: <20260705002546.85004-1-pengpeng@iscas.ac.cn>`
No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-by:`,
`Link:`, or `Cc: stable`. Absence of `Fixes:` / `Cc: stable` is expected
for this review and is not a negative.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `octep_pci_vdpa_map[]` is wired to `octep_pci_vdpa.id_table`,
but is not exported with `MODULE_DEVICE_TABLE()`.
- **Symptom:** `depmod`/`modpost` do not emit `pci:v0000177Dd0000...`
aliases, so udev/kmod will not autoload `octep_vdpa` when the
Cavium/Marvell DPU appears.
- **Version info:** none in the message. Driver introduced in this tree
by `8b6c724cdab85` (in v6.11; present in v6.12).
- **Root cause:** missing module-alias publication, not a runtime logic
error.
The “source-level fix / no dynamic hardware reproduction” paragraph is
in the original lore submission (not a stable-prep marker). MST asked
what it meant, then applied the patch unchanged.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not disguised.** It is an explicit autoload/hardware-binding
fix, same class as adding PCI IDs.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/vdpa/octeon_ep/octep_vdpa_main.c` (**+1 / -0**)
- Function/object: `octep_pci_vdpa_map[]` (table definition; no function
body change)
- Scope: **single-file surgical fix**
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** PCI IDs exist and are used for bind **if the module is
already loaded**. No `MODULE_DEVICE_TABLE`, so no `modules.alias`
entries.
- **After:** Same IDs plus `MODULE_DEVICE_TABLE(pci,
octep_pci_vdpa_map)`, which creates the
`__mod_pci__octep_pci_vdpa_map_device_table` alias consumed by
`scripts/mod/file2alias.c` (`do_pci_entry()`).
- Path: **module build / autoload**, not probe/error/init runtime logic.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Hardware enablement / missing device-ID publication]**
Mechanism: `MODULE_DEVICE_TABLE` is empty when `!MODULE`
(`include/linux/module.h`). This driver is **module-only** (`depends on
m`), so the missing export is always the live path. Sibling vDPA PCI
drivers already have it (`ifcvf_main.c`, `snet_main.c`).
**Step 2.4: ASSESS THE FIX QUALITY**
Record: **Obviously correct, one standard line, no API/locking/refcount
change.** Regression risk is **very low**. MST applied it to mainline.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record: Table and `pci_driver` come from **`8b6c724cdab85`** (`virtio:
vdpa: vDPA driver for Marvell OCTEON DPU devices`, Srujana Challa,
2024-06-14; committed 2024-07-09). **Never had `MODULE_DEVICE_TABLE`.**
Present since v6.11.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No `Fixes:` tag.** Buggy omission is the original driver
commit above. `git merge-base --is-ancestor 8b6c724cdab85 HEAD` →
**yes**.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record (this tree, `-- drivers/vdpa/octeon_ep/octep_vdpa_main.c`):
- `b3175ec5209b8` mailbox signature
- `6e950ebc7da41` PF→VF mailbox address
- `f655550322469` SR-IOV refcount leak
- `8b6c724cdab85` driver introduction
No later commit in this tree adds `MODULE_DEVICE_TABLE`. Standalone; not
patch X/Y.
**Step 3.4: CHECK THE AUTHOR’S OTHER COMMITS**
Record: Pengpeng Hou submitted a July 2026 batch of the same one-line
fix. Same pattern already **in this 6.12 tree**: `877e99535dfb9` `can:
ctucanfd: add missing MODULE_DEVICE_TABLE()` (upstream `d937bdb244a75`,
with `Cc: stable`). Mainline also has `7eeda5e248727` `vdpa: alibaba:
add missing MODULE_DEVICE_TABLE()`. Author is not the vDPA maintainer;
MST took the octeon_ep patch.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: **None.** Uses existing table name and `MODULE_DEVICE_TABLE()`.
`git show 346da62d9f9cc | git apply --check` → **CLEAN APPLY**. Later
mainline commits (`3e4cddec63db5`, `6519ca235131c`) do not touch this
site.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 346da62d9f9cc` →
https://patch.msgid.link/20260705002546.85004-1-pengpeng@iscas.ac.cn
- `b4 dig -a`: **v1 only** (2026-07-05)
- Thread: author patch + MST reply (“what does this paragraph mean?”
about the source-level boilerplate). **No NAK, no stable request, no
risk flags.** Applied 2026-08-19.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc included Marvell reviewers
`schalla@marvell.com`, `vattunuru@marvell.com`; MST; Jason Wang; Xuan
Zhuo; Eugenio Pérez; `virtualization@lists.linux.dev`. **Subsystem
maintainer applied it.**
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: **No user/syzbot/bugzilla report.** Evidence is the match table
vs missing export. Same class as PCI ID additions.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Same-day one-line series (cadence PCI, tegra CEC, ixp4xx,
most_usb, imx8qxp, ctucanfd, alibaba vdpa, this driver). Each is self-
contained.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: No octeon_ep-specific stable-list hit for this subject. Lore
HTTP fetch returned 403; search did not show a prior reject. Related
ctucanfd patch **did** go to this 6.12 tree with `Cc: stable`.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: No function body change. Objects: `octep_pci_vdpa_map`,
`octep_pci_vdpa`, `octep_vdpa_probe`.
**Step 5.2: TRACE CALLERS**
Record: `octep_vdpa_probe` is only the PCI `.probe` callback
(`module_pci_driver`). Invoked by PCI core on match **after** the module
is loaded. Callers of the *new* macro are `modpost`/`file2alias.c` at
build time.
**Step 5.3: TRACE CALLEES**
Record: Probe dispatches to `octep_vdpa_probe_vf` /
`octep_vdpa_probe_pf`. Unchanged.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: PCI enumeration of Cavium IDs
0xb900/0xb903/0xba00/0xba03/0xbd00/0xbd03 (`PCI_VENDOR_ID_CAVIUM`
0x177d). Userspace-visible via hotplug/boot. Autoload is the intended
udev path. Manual `modprobe octep_vdpa` still works without the patch.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: In-tree vDPA PCI drivers already export tables (`ifcvf`,
`snet`). This driver was the outlier.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `drivers/vdpa/octeon_ep/octep_vdpa_main.c:841-857` has
the table and `.id_table` and **no** `MODULE_DEVICE_TABLE`. Introduced
**v6.11**, present in this **6.12.111** checkout. `git merge-base --is-
ancestor 346da62d9f9cc HEAD` → **not in HEAD**.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply.** Surrounding lines match. No refactor of the
table.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: **No.** `git log --grep=MODULE_DEVICE_TABLE -- drivers/vdpa/` is
empty. Same-class ctucanfd fix is already in this tree, which shows the
pattern is accepted here.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **[drivers/vdpa/octeon_ep — Marvell Octeon DPU vDPA] [PERIPHERAL
/ IMPORTANT for that hardware]**
Not core mm/VFS. Real PCI devices; Kconfig is supported (MAINTAINERS:
MARVELL OCTEON ENDPOINT VIRTIO DATA PATH ACCELERATOR).
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Driver is young (2024) but present for the life of 6.12. Modest
churn; mailbox/SR-IOV fixes already in this tree.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-/hardware-specific** — hosts with Marvell/Cavium CN10x
DPU vDPA (`CONFIG_OCTEONEP_VDPA=m`). Not universal.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Trigger is **every boot/hotplug** of those PCI devices if the
module is not preloaded. Unprivileged users do not trigger a crash; they
just get a **non-bound device**. Kconfig `depends on m` means there is
**no built-in fallback**.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Failure mode: module does not autoload; vDPA offload does not
come up until manual modprobe.** **Severity: MEDIUM** (hardware non-
function, not oops/corruption). Same user-visible class as missing PCI
IDs.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** medium for Octeon DPU users (driver actually attaches on
device appearance).
- **Risk:** very low (one `MODULE_DEVICE_TABLE` line, no runtime
change).
- **Ratio:** **favorable.** Official rule: *“It must either fix a real
bug that bothers people or just add a device ID.”*
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real autoload bug: IDs exist, aliases are not generated.
- Module-only driver (`depends on m`) — autoload is the normal load
path.
- +1 line, standard macro, clean apply on 6.12.111.
- Same omission since driver introduction (in this tree).
- Sibling vDPA PCI drivers already export tables.
- Virtio maintainer applied it upstream.
- Same-class patch already in this 6.12 tree (`can: ctucanfd`).
- Matches the documented device-ID exception.
**AGAINST:**
- Not a crash/UAF/security issue.
- Niche DPU hardware; no distro bug report.
- Manual `modprobe` is a workaround.
- Kconfig notes firmware/emulation must be running first (PCI presence
still implies the device is enumerable).
**UNRESOLVED:** Direct lore.kernel.org HTTP was 403; thread recovered
via `b4 dig` mbox. No stable-list discussion found.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — one-line, maintainer-
applied.
2. Fixes a real bug that affects users? **PASS** — broken autoload for
existing PCI IDs.
3. Important issue? **PASS** under the **device-ID / hardware-
enablement** exception (not a crash).
4. Small and contained? **PASS** — +1 line, one file.
5. No new features or APIs? **PASS** — publishes IDs the driver already
matches.
6. Can apply to the local tree? **PASS** — buggy code present; `git
apply --check` clean.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **Device ID / module-alias publication for an existing driver**
(stable-rules “or just add a device ID”).
**Step 9.4: MAKE YOUR DECISION**
Backport. The local 6.12 tree has the driver without aliases; the fix is
the documented, low-risk hardware-enablement class.
---
### What the commit solves for this tree
On 6.12, `octep_vdpa` can bind only if something already loaded it. The
PCI table is complete; `MODULE_DEVICE_TABLE` is missing, so udev never
loads `octep_vdpa` for CN103K/CN105K/CN106K PF/VF IDs. Adding the macro
generates aliases at module build time and does not change probe logic.
### Risk vs benefit
Benefit is that those DPUs attach on boot/hotplug. Risk is negligible.
This is the same pattern already taken for `ctucanfd` in this tree.
### Concerns
None that block backport: no prerequisites, no conflicts, no new API. No
`Fixes:`/`Cc: stable` on the original, which is why it needs this
review.
---
### Verification
- **[Phase 1]** `git log -1 --format=fuller 346da62d9f9cc`: SOB Pengpeng
Hou + MST; Message-ID as given; no Fixes/Cc stable/Reported-by.
- **[Phase 1]** `git describe HEAD` / `make kernelversion` / Makefile:
**pending-6.12, 6.12.111**.
- **[Phase 2]** Diff is +1 `MODULE_DEVICE_TABLE(pci,
octep_pci_vdpa_map)` after the ID table.
- **[Phase 2]** `include/linux/module.h:251-256`: macro emits the alias
only for `MODULE`.
- **[Phase 2]** `scripts/mod/file2alias.c:441-459`: `do_pci_entry()`
builds `pci:vNdN...` aliases.
- **[Phase 2]** `drivers/vdpa/Kconfig:133-142`: `OCTEONEP_VDPA` is
`tristate` **`depends on m`**.
- **[Phase 3]** `git blame -L 841,863`: table from `8b6c724cdab85`;
never had MODULE_DEVICE_TABLE.
- **[Phase 3]** `git merge-base --is-ancestor 8b6c724cdab85 HEAD` → yes;
vs v6.11 and v6.12 → in both.
- **[Phase 3]** `git merge-base --is-ancestor 346da62d9f9cc HEAD` →
**not in this tree**.
- **[Phase 3]** `git log --oneline -20 --
drivers/vdpa/octeon_ep/octep_vdpa_main.c`: four commits; none add the
macro.
- **[Phase 3]** `git show 346da62d9f9cc | git apply --check` → **CLEAN
APPLY**.
- **[Phase 3]** `877e99535dfb9` in this tree: same-author
MODULE_DEVICE_TABLE pattern already backported.
- **[Phase 4]** `b4 dig -c 346da62d9f9cc`: lore URL; v1 only; recipients
include MST and Marvell; MST question only, then apply.
- **[Phase 4]** `/tmp/octep_mdt.mbox`: 2 messages; no NAK.
- **[Phase 4]** lore.kernel.org/stable search: no octeon_ep discussion
found (lore HTTP 403 on direct fetch).
- **[Phase 5]** `grep octep_vdpa_probe`: only PCI `.probe` in this file.
- **[Phase 5]** `ifcvf`/`snet` already have `MODULE_DEVICE_TABLE(pci,
...)`.
- **[Phase 6]** Current file lines 841–863: table + driver, **no**
MODULE_DEVICE_TABLE.
- **[Phase 6]** Later origin/master commits do not touch this hunk.
- **[Phase 7]** MAINTAINERS: `drivers/vdpa/octeon_ep/` listed; reviewers
schalla/vattunuru.
- **[Phase 8]** IDs in `octep_vdpa.h`:
0xb900/0xb903/0xba00/0xba03/0xbd00/0xbd03; vendor
`PCI_VENDOR_ID_CAVIUM` 0x177d.
- **[Phase 9]** `Documentation/process/stable-kernel-rules.rst:15`: “fix
a real bug … **or just add a device ID**”.
- **UNVERIFIED:** Live udev/hardware reproduction (author did not claim
it; source-level evidence is sufficient). Direct lore.kernel.org page
fetch (403); thread recovered via b4.
**YES**
drivers/vdpa/octeon_ep/octep_vdpa_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/vdpa/octeon_ep/octep_vdpa_main.c b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
index 9aac6dbdaf3aa..e5dbabff5e81b 100644
--- a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
+++ b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
@@ -898,6 +898,7 @@ static struct pci_device_id octep_pci_vdpa_map[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_VDPA_DEVID_CN103K_VF) },
{ 0 },
};
+MODULE_DEVICE_TABLE(pci, octep_pci_vdpa_map);
static struct pci_driver octep_pci_vdpa = {
.name = OCTEP_VDPA_DRIVER_NAME,
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH AUTOSEL 6.18-6.12] vdpa: octeon_ep: add missing MODULE_DEVICE_TABLE()
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] vdpa: octeon_ep: " Sasha Levin
@ 2026-10-19 14:50 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-19 14:50 UTC (permalink / raw)
To: Sasha Levin; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] vdpa: octeon_ep: MODULE_DEVICE_TABLE() conflicts with octeon_ep and Kconfig
--
commit 2202a6588d5755d6dbbded2b962a2580a97810a5
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>
vdpa: octeon_ep: add missing MODULE_DEVICE_TABLE()
This commit adds a MODULE_DEVICE_TABLE(pci, octep_pci_vdpa_map) entry to
drivers/vdpa/octeon_ep/octep_vdpa_main.c to generate PCI module aliases for
automatic module loading.
> diff --git a/drivers/vdpa/octeon_ep/octep_vdpa_main.c b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> index 9e8d07078606f..3c7245ff32507 100644
> --- a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> +++ b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> @@ -883,6 +883,7 @@ static struct pci_device_id octep_pci_vdpa_map[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_VDPA_DEVID_CN103K_VF) },
> { 0 },
> };
> +MODULE_DEVICE_TABLE(pci, octep_pci_vdpa_map);
[Severity: High]
Was MODULE_DEVICE_TABLE(pci, octep_pci_vdpa_map) intentionally omitted here?
In drivers/vdpa/Kconfig, config OCTEONEP_VDPA specifies:
config OCTEONEP_VDPA
tristate "vDPA driver for Octeon DPU devices"
depends on m
depends on PCI_MSI
help
This is a vDPA driver designed for Marvell's Octeon DPU devices.
This driver enables the offloading of the VirtIO dataplane to an
Octeon DPU device.
Please note that this driver must be built as a module and it
cannot be loaded until the Octeon emulation software is running.
Exporting octep_pci_vdpa_map via MODULE_DEVICE_TABLE() causes udev/kmod to
autoload octep_vdpa during boot or PCI enumeration before the Octeon
emulation software is running. If OCTEP_EPF_RINFO(0) is 0 because the
emulation software has not initialized the rings yet,
octep_vdpa_probe() -> octep_vdpa_probe_pf() -> octep_vdpa_pf_setup() fails
with -EINVAL:
drivers/vdpa/octeon_ep/octep_vdpa_main.c:octep_vdpa_pf_setup() {
...
addr = octpf->base[OCTEP_HW_MBOX_BAR];
val = readq(addr + OCTEP_EPF_RINFO(0));
if (val == 0) {
dev_err(&pdev->dev, "Invalid device configuration\n");
return -EINVAL;
}
...
}
In addition, all six PCI device IDs in octep_pci_vdpa_map (0x177d:0xb900,
0xb903, 0xba00, 0xba03, 0xbd00, and 0xbd03 with PCI_ANY_ID for subvendor,
subdevice, and class) are identical to the PCI device IDs already exported
via MODULE_DEVICE_TABLE() by the in-tree Ethernet PF and VF drivers:
drivers/net/ethernet/marvell/octeon_ep/octep_main.c:
static const struct pci_device_id octep_pci_id_tbl[] = {
...
{PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CN10KA_PF)},
{PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CNF10KA_PF)},
{PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CNF10KB_PF)},
{PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CN10KB_PF)},
{0, },
};
MODULE_DEVICE_TABLE(pci, octep_pci_id_tbl);
drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c:
static const struct pci_device_id octep_vf_pci_id_tbl[] = {
...
{PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CN10KA_VF)},
{PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CNF10KA_VF)},
{PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CNF10KB_VF)},
{PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CN10KB_VF)},
{0, },
};
MODULE_DEVICE_TABLE(pci, octep_vf_pci_id_tbl);
Neither octep_vdpa_probe_pf() nor octep_vdpa_probe_vf() checks whether the
device is running vDPA firmware rather than standard NIC firmware before
claiming the PCI device. In octep_vdpa_pf_setup(), OCTEP_EPF_RINFO(0) is
also non-zero in standard NIC mode, causing it to shrink BAR 4 via
octep_vdpa_pf_bar_shrink() and succeed, while octep_vdpa_probe_vf() queues
octep_vdpa_setup_task and returns 0 unconditionally:
drivers/vdpa/octeon_ep/octep_vdpa_main.c:octep_vdpa_probe_vf() {
...
INIT_WORK(&mgmt_dev->setup_task, octep_vdpa_setup_task);
schedule_work(&mgmt_dev->setup_task);
dev_info(&pdev->dev, "octep vdpa mgmt device setup task is queued\n");
return 0;
}
Could publishing MODULE_DEVICE_TABLE(pci, octep_pci_vdpa_map) cause
octep_vdpa to autoload prematurely and race with octeon_ep and
octeon_ep_vf to claim standard NIC PF and VF devices?
>
> static struct pci_driver octep_pci_vdpa = {
> .name = OCTEP_VDPA_DRIVER_NAME,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-265-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] virtio_console: take a kref in find_port_by_vq() to fix port UAF
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (4 preceding siblings ...)
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] vdpa: octeon_ep: " Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
2026-10-19 15:00 ` sashiko-bot
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] virtio_balloon: warn on failed buffer add in tell_host() Sasha Levin
` (3 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: Hari Mishal, Greg Kroah-Hartman, Sasha Levin, amit,
virtualization, linux-kernel
From: Hari Mishal <harimishal1@gmail.com>
[ Upstream commit 18dd0b4e489e4f0e3b748cf36ccb909c75a3efab ]
find_port_by_vq() returns a raw struct port pointer without taking a
reference on it, unlike find_port_by_devt_in_portdev() which does.
find_port_by_vq()'s only two callers, in_intr() and out_intr(), run as
virtqueue interrupt callbacks, entirely independent of and possibly
concurrently with unplug_port(), which itself runs from a workqueue when
the host sends a VIRTIO_CONSOLE_PORT_REMOVE control message.
unplug_port() removes the port from portdev->ports under ports_lock,
then later drops its last reference with kref_put(), freeing it via
remove_port(). find_port_by_vq() also walks portdev->ports under
ports_lock, so if it finds the port still on the list, the list removal,
and therefore the eventual kref_put(), has not happened yet, and taking
a reference at that point is always safe. Without doing so,
in_intr()/out_intr() can be left holding a pointer to a port that
unplug_port() frees on another core before they are done using it.
Both triggers are host-controlled as the host decides when to send the
PORT_REMOVE control message and when to kick the port's data vq. So a
malicious backend could race the two on purpose, without any guest side
cooperation. The freed object is a generic kmalloc allocation containing
a wait_queue_head_t, which in_intr()/out_intr() pass to
wake_up_interruptible() after touching the stale pointer.
wake_up_interruptible() invokes a function pointer read out of the wait
queue's entries. If the freed slab slot is reclaimed with attacker
influenced content before that call, then this is an arbitrary function
call primitive rather than just undefined behaviour.
Take a reference in find_port_by_vq() while still holding ports_lock,
matching find_port_by_devt_in_portdev(), and release it in in_intr() and
out_intr() once they are done with the port.
Signed-off-by: Hari Mishal <harimishal1@gmail.com>
Link: https://patch.msgid.link/20260717150622.23636-1-harimishal1@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.6.157** (`pending-6.6`). The virtio_console UAF is
in this tree, and the kref get/put fix is a small, self-contained match
for the existing `find_port_by_devt_in_portdev()` pattern.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[virtio_console] [take / fix] take a kref in
`find_port_by_vq()` to fix port UAF.** Subsystem is the virtio console
character driver. The subject names a use-after-free and the exact
missing refcount.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Signed-off-by:** Hari Mishal \<harimishal1@gmail.com\> (author)
- **Link:**
https://patch.msgid.link/20260717150622.23636-1-harimishal1@gmail.com
- **Signed-off-by:** Greg Kroah-Hartman \<gregkh@linuxfoundation.org\>
(char-misc application)
- **No Fixes:** tag (expected for this review)
- **No Cc: stable** (expected; ignored as a negative)
- **No Reported-by / Tested-by / Reviewed-by / Acked-by**
- Ignore pipeline **Signed-off-by: Sasha Levin** / **[Upstream commit
...]** if present
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `find_port_by_vq()` returns a raw `struct port *` with no
`kref_get()`, unlike `find_port_by_devt_in_portdev()`.
- **Race:** `in_intr()` / `out_intr()` (virtqueue callbacks) vs
`unplug_port()` (workqueue, `VIRTIO_CONSOLE_PORT_REMOVE`).
- **Mechanism:** both walk `portdev->ports` under `ports_lock`. Lookup
can succeed, drop the lock, then unplug `list_del()`s and later
`kref_put()` → `remove_port()` → `kfree(port)` while the IRQ path
still uses the pointer.
- **Symptom:** use-after-free of a kmalloc object that contains a
`wait_queue_head_t`. `wake_up_interruptible()` follows a function
pointer from wait-queue entries. If the slab is recycled with
attacker-controlled content, that is an **arbitrary function call**,
not just a crash.
- **Trigger:** host-controlled (PORT_REMOVE + data vq kick). A malicious
virtio backend can race this without guest help.
- **Fix:** `kref_get()` under `ports_lock` in `find_port_by_vq()`;
`kref_put(..., remove_port)` at the end of `in_intr()` / `out_intr()`.
- **Version info in message:** none.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** This is an explicit UAF/refcount fix, not
cleanup disguised as a fix.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `drivers/char/virtio_console.c` only
- **Scope:** ~14 insertions, 1 deletion (comment + `kref_get` + two
`kref_put`s)
- **Functions:** `find_port_by_vq()`, `out_intr()`, `in_intr()`
- **Classification:** single-file surgical refcount fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **`find_port_by_vq()`:** before, returned a list pointer after
dropping `ports_lock`. After, `kref_get(&port->kref)` while still on
the list and holding the lock; callers must put.
- **`out_intr()`:** still looks up, flushes if missing, then
`wake_up_interruptible(&port->waitqueue)`; now `kref_put()` after the
wake.
- **`in_intr()`:** same lookup/inbuf/sigio/wake/hvc path; now
`kref_put()` after the last use of `port`.
- **Early `if (!port)` returns:** no get was taken, so no put (correct).
- **Path:** virtqueue interrupt/callback vs hot-unplug workqueue.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** **use-after-free / reference counting** (UAF of `struct
port`).
- **What was broken:** lookup did not pin the object; unplug could free
it before IRQ handlers finished (waitqueue wake, inbuf,
`is_rproc_serial(port->portdev->vdev)`, `hvc_poll`).
- **How the fix works:** same lock-held get as
`find_port_by_devt_in_portdev()`. If the port is still on
`portdev->ports`, `unplug_port()` has not yet `list_del()`d, so it has
not yet done the matching `kref_put()`. `kref_get()` is therefore on a
live ref.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Obviously correct:** matches the 2013 open/close unplug fix
(`057b82be3ca3d0`) and the existing finder that already takes a kref.
- **Minimal:** no API, no structural change.
- **Regression risk:** low. `remove_port()` in this tree is only
`kfree(port)`, which is IRQ-safe, so a last `kref_put()` from
`in_intr()`/`out_intr()` is safe. Puts are at the end of both
handlers; 6.6 has no extra early return after a successful lookup, so
get/put is balanced.
- **Residual (not a reason to reject this patch):** `in_intr()` still
does `is_rproc_serial(port->portdev->vdev)` without a NULL `portdev`
check. That is a separate NPD if unplug already cleared `portdev`. It
already exists when a file holds a kref. Sibling series patch 3/4
addresses it; it is **not** required for kref accounting on this tree.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- `find_port_by_vq()` introduced in **`203baab8ba3195`** (“virtio:
console: Introduce function to hand off data from host to readers”,
2010-02-24), first in **v2.6.34-rc1**. Present in this 6.6 tree (`git
merge-base --is-ancestor` succeeded).
- `in_intr()` from **`17634ba25544d6`** (2009). `out_intr()` from
**`2770c5ea501be6`** (2011).
- **`kref_get` in `find_port_by_devt_in_portdev()`** from
**`057b82be3ca3d0`** (2013): “fix race with port unplug and
open/close”, **CC: stable**. Same bug class, same fix pattern, already
backported historically.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** The missing get was never added for this
finder; the bug is as old as `find_port_by_vq()` / unplug concurrency.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Recent 6.6 virtio_console commits are unrelated (control-out
buffers, cols/rows, probe). **This UAF fix is not in HEAD**
(`find_port_by_vq()` still has no `kref_get`). Standalone relative to
this tree’s `in_intr()` (no `if (!port->portdev)` early return).
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Hari Mishal is not the virtio-console maintainer (MAINTAINERS:
Amit Shah). Greg Kroah-Hartman applied the patch. Related series patches
1–2 touch virtio-mem/virtio_input, not this file.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Originally **patch 4/4** in “virtio: validate device-reported
values across drivers”. Patch 3/4 adds `if (!port->portdev)` in
`in_intr()`; lore diffs of 4/4 therefore had an extra `kref_put` on that
return. **This tree does not have that check** (`if (!port->portdev)`
only in `discard_port_data()` / `reclaim_consumed_buffers()`). The
candidate under review is the form without that hunk, which matches 6.6.
**Can apply standalone.** Patch 3 is complementary, not a build/semantic
prerequisite for this kref change here.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- **Lore / msgid:** `20260717150622.23636-1-harimishal1@gmail.com`
(standalone 2026-07-17 resubmit). Earlier: `[PATCH 4/4]` / `[PATCH v2
4/4]` 2026-07-15.
- **`b4 mbox` on that msgid:** **1 message in the thread** (no replies).
- **`b4 dig -c <sha>`:** not usable; this SHA is not in the 6.6
checkout.
- **WebFetch lore.kernel.org:** **403** (could not read lore HTML
directly).
- Openwall/LKML/Patchew confirm v1 → v2 (comment wording only) →
standalone resubmit. No NAK found in those archives.
- **UNVERIFIED:** full lore review thread beyond “no replies” from b4.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record: Recipients included Amit Shah, Arnd Bergmann, Greg Kroah-
Hartman, virtualization@, linux-kernel@. No public Reviewed-by. **Greg’s
SOB** is on the applied commit. Series v1 patches 1 and 3 carried
`Assisted-by: gkh_clanker:t1000`.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: **No syzbot/bugzilla/CVE found.** Analysis is code-level: host-
controlled race, waitqueue function-pointer UAF. Severity from the
author is crash / possible arbitrary call.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record:
- **3/4** `virtio_console: avoid NULL portdev dereference in in_intr()`
— NPD on `port->portdev->vdev` after unplug. **Not in this tree.**
Related, not required for this patch’s get/put balance on 6.6.
- **1/4, 2/4** — virtio-mem / virtio_input validation; independent.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: Web search of lore stable for this subject did **not** find a
prior stable accept/reject. No evidence it was already rejected for
stable.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
(semcode MCP not available; used grep + file reads.)
**Step 5.1: KEY FUNCTIONS**
Record: `find_port_by_vq`, `in_intr`, `out_intr`; callees `remove_port`,
`unplug_port`, `handle_control_message`.
**Step 5.2: TRACE CALLERS**
Record:
- `find_port_by_vq()`: **only** `in_intr()` and `out_intr()` (verified
by grep).
- Those are vq callbacks registered in `init_vqs()` (`io_callbacks[j] =
in_intr` / `out_intr`).
- `unplug_port()`: `handle_control_message()` on
`VIRTIO_CONSOLE_PORT_REMOVE`, and `virtcons_remove()`.
**Step 5.3: TRACE CALLEES**
Record: `in_intr` uses `get_inbuf`, `discard_port_data`,
`send_sigio_to_port`, `wake_up_interruptible`, `hvc_poll`/`hvc_kick`.
`unplug_port` does `list_del` under `ports_lock`, then inbuf/hvc/sysfs
teardown, then `kref_put`. `remove_port` is `kfree(port)` only.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Guest-visible: virtio-console `/dev/vport*` and hvc console.
Unplug is host-driven (control message). **Reachable** on any VM with
virtio-console and port hot-unplug, and **forcibly racable** by an
untrusted backend. Not a dead config path. `CONFIG_VIRTIO_CONSOLE` is a
normal guest option.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: `find_port_by_devt_in_portdev()` already `kref_get()`s under
`ports_lock`. `find_port_by_id()` does not; it is used from the control
work path that also calls `unplug_port()` (same workqueue for
PORT_REMOVE). Different concurrency than vq IRQs.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `drivers/char/virtio_console.c`: `find_port_by_vq()` at
309–323 has no `kref_get`; `out_intr` 1734–1745 and `in_intr` 1747–1790
use the raw pointer. Introduced ~v2.6.34. Local tree: **6.6.157**.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply expected.** 6.6 `in_intr()` matches the candidate
(no `if (!port->portdev)`). Lore 4/4 needed that extra put; the
candidate already dropped it. Same kref API
(`kref_get`/`kref_put`/`remove_port`).
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: 2013 open/close kref fix is present. **This vq-interrupt UAF fix
is not.** Patch 3/4 NPD guard is not present in `in_intr()`.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **virtio console** (`drivers/char/virtio_console.c`).
**IMPORTANT:** standard VM guest console/serial (`/dev/hvc`,
`/dev/vport*`), QEMU/KVM, clouds, rproc-serial. Not mm/core, but widely
deployed.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature driver (2009–); modest recent churn. Long-lived bug, not
a new-subsystem issue.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-specific / config-specific:** `CONFIG_VIRTIO_CONSOLE`
guests and rproc serial. Large population of VMs.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Concurrent **PORT_REMOVE** (or device remove → `unplug_port`)
and **data vq kick**. Legitimate hot-unplug can hit it; a malicious
backend can force it. Guest userspace is not required.
Interrupt/callback vs workqueue.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **UAF** of `struct port`; `wake_up_interruptible()` on a freed
waitqueue. **Severity: HIGH / security-relevant CRITICAL** (crash, or
arbitrary call if the slab is recycled). Host-triggered, so in a fully
trusted-hypervisor model the host is already privileged; it is still a
real guest kernel memory-safety bug and matters for untrusted/vhost-user
backends.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit: high** — pins the port for the IRQ path; closes a long-
standing UAF on a common VM driver.
- **Risk: very low** — ~3 executable lines, existing kref pattern,
`remove_port` is `kfree`.
- **Ratio:** strongly favors backport.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real UAF: vq IRQ vs `unplug_port()`, pointer used after possible
`kfree`.
- Security-relevant (waitqueue function pointer; host-controlled race).
- Same pattern as `057b82be3ca3d0`, which was **CC: stable**.
- Buggy code present since ~2010; definitely in 6.6.157.
- Small, obviously correct, get/put balanced on this tree.
- virtio-console is a common guest driver.
**AGAINST:**
- No Fixes/Cc-stable/Reported-by/syzbot (not a negative per review
rules).
- No public review replies on the 2026-07-17 thread.
- Sibling NPD fix (series 3/4) not included; remaining `portdev` NULL
deref possible. **Does not make this patch wrong.**
**UNRESOLVED:**
- lore.kernel.org HTML 403; b4 showed a 1-message thread only.
- No CVE id found.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — matches existing kref
finder; Greg applied it. No extra Tested-by, but the change is
locally verifiable.
2. Fixes a real bug that affects users? **PASS** — UAF on port unplug vs
data vq.
3. Important issue? **PASS** — UAF / possible arbitrary call;
HIGH/CRITICAL.
4. Small and contained? **PASS** — one file, ~14 lines.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — 6.6 has the buggy functions;
candidate already matches this `in_intr()`.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **Not an exception category.** Ordinary memory-safety bug fix
(the primary stable class).
**Step 9.4: MAKE YOUR DECISION**
Backport to this 6.6.157 tree. It fixes a real, old, host-racable port
UAF with a three-line kref pin that this driver already uses for the
open path.
===============================================================
What this solves for stable users
===============================================================
On 6.6, `find_port_by_vq()` still hands `in_intr()`/`out_intr()` an
unpinned `struct port`. `unplug_port()` can remove it from the list and
drop the last kref (`remove_port()` → `kfree`) while those callbacks
still wake `port->waitqueue`. That is a use-after-free. In VMs that hot-
unplug virtio-console ports, or against a hostile virtio backend, the
guest kernel can oops or worse.
The patch takes a kref under `ports_lock` (safe: still on the list ⇒
unplug has not put yet) and drops it after the last use. That is the
same design as the 2013 stable-bound open/close fix. It does not add
APIs. Residual `portdev == NULL` NPD in `in_intr()` is a separate, older
hole (open ports already hit it); series patch 3/4 would close it later
and is not required for this get/put to be correct here.
Risk is low: `remove_port()` is only `kfree`, so a final put from the vq
callback is fine.
===============================================================
Verification
===============================================================
- **[Phase 1]** Parsed subject/tags/body: UAF + kref; SOB Hari Mishal +
Greg KH; Link msgid `20260717150622.23636-1-harimishal1@gmail.com`; no
Fixes/Reported-by.
- **[Phase 2]** Diff: `kref_get` in `find_port_by_vq`; `kref_put` at end
of `in_intr`/`out_intr`; comment only otherwise.
- **[Phase 2]** Read 6.6 `in_intr`/`out_intr`/`unplug_port`/`remove_port
`/`handle_control_message`: race and `kfree`-only release confirmed.
- **[Phase 3]** `git describe HEAD` / Makefile: **6.6.157**, HEAD
`af85e5fc2f380`.
- **[Phase 3]** `git blame` `find_port_by_vq`: `203baab8ba3195`
(v2.6.34-rc1); `git merge-base --is-ancestor` true.
- **[Phase 3]** `git show 057b82be3ca3d0`: same unplug/kref pattern,
**CC: stable**.
- **[Phase 3]** grep: no `kref_get` in `find_port_by_vq` on HEAD; only
two callers.
- **[Phase 4]** Patchew/Openwall/LKML: series 0–4/4, v2 comment-only,
standalone 17 Jul 2026.
- **[Phase 4]** `b4 mbox` msgid: **1 message**, no replies.
- **[Phase 4]** WebFetch lore: **403**.
- **[Phase 5]** grep callers; `init_vqs` registers `in_intr`/`out_intr`;
PORT_REMOVE → `unplug_port`.
- **[Phase 6]** 6.6 `in_intr` has no `!port->portdev` early return;
candidate matches; complementary 3/4 not present.
- **[Phase 7]** Kconfig `VIRTIO_CONSOLE`; MAINTAINERS Amit Shah /
virtualization list.
- **[Phase 8]** Failure mode: port UAF via waitqueue; HIGH/CRITICAL.
- **UNVERIFIED:** complete lore review beyond b4’s empty thread; CVE
assignment.
**YES**
drivers/char/virtio_console.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 76f934b5d84be..750c15966d00e 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -304,6 +304,12 @@ static struct port *find_port_by_id(struct ports_device *portdev, u32 id)
return port;
}
+/*
+ * Finds a port by the virtqueue and returns a pointer to struct port
+ * with the reference count incremented.
+ *
+ * Callers MUST decrement it when finished.
+ */
static struct port *find_port_by_vq(struct ports_device *portdev,
struct virtqueue *vq)
{
@@ -312,8 +318,10 @@ static struct port *find_port_by_vq(struct ports_device *portdev,
spin_lock_irqsave(&portdev->ports_lock, flags);
list_for_each_entry(port, &portdev->ports, list)
- if (port->in_vq == vq || port->out_vq == vq)
+ if (port->in_vq == vq || port->out_vq == vq) {
+ kref_get(&port->kref);
goto out;
+ }
port = NULL;
out:
spin_unlock_irqrestore(&portdev->ports_lock, flags);
@@ -1706,6 +1714,7 @@ static void out_intr(struct virtqueue *vq)
}
wake_up_interruptible(&port->waitqueue);
+ kref_put(&port->kref, remove_port);
}
static void in_intr(struct virtqueue *vq)
@@ -1751,6 +1760,8 @@ static void in_intr(struct virtqueue *vq)
if (is_console_port(port) && hvc_poll(port->cons.hvc))
hvc_kick();
+
+ kref_put(&port->kref, remove_port);
}
static void control_intr(struct virtqueue *vq)
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH AUTOSEL 6.18-5.10] virtio_console: take a kref in find_port_by_vq() to fix port UAF
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] virtio_console: take a kref in find_port_by_vq() to fix port UAF Sasha Levin
@ 2026-10-19 15:00 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-19 15:00 UTC (permalink / raw)
To: Sasha Levin; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] virtio_console: use-after-free on port->cons.hvc in in_intr()
- [High] virtio_console: NULL portdev dereference and inbuf leak in in_intr()
--
commit 30bde48929beace6a7511551cb2600c5af9d8ca1
Author: Hari Mishal <harimishal1@gmail.com>
virtio_console: take a kref in find_port_by_vq() to fix port UAF
This commit takes a kref reference on struct port inside find_port_by_vq()
under ports_lock and releases it at the end of in_intr() and out_intr() to
prevent a use-after-free when racing with unplug_port().
Link: https://patch.msgid.link/20260717150622.23636-1-harimishal1@gmail.com
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 088182e54debd..14ca24b6daf2c 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -304,6 +304,12 @@ static struct port *find_port_by_id(struct ports_device *portdev, u32 id)
> return port;
> }
>
> +/*
> + * Finds a port by the virtqueue and returns a pointer to struct port
> + * with the reference count incremented.
> + *
> + * Callers MUST decrement it when finished.
> + */
> static struct port *find_port_by_vq(struct ports_device *portdev,
> struct virtqueue *vq)
> {
> @@ -312,8 +318,10 @@ static struct port *find_port_by_vq(struct ports_device *portdev,
>
> spin_lock_irqsave(&portdev->ports_lock, flags);
> list_for_each_entry(port, &portdev->ports, list)
> - if (port->in_vq == vq || port->out_vq == vq)
> + if (port->in_vq == vq || port->out_vq == vq) {
> + kref_get(&port->kref);
> goto out;
> + }
> port = NULL;
> out:
> spin_unlock_irqrestore(&portdev->ports_lock, flags);
[ ... ]
> @@ -1751,6 +1760,8 @@ static void in_intr(struct virtqueue *vq)
>
> if (is_console_port(port) && hvc_poll(port->cons.hvc))
> hvc_kick();
> +
> + kref_put(&port->kref, remove_port);
> }
>
> static void control_intr(struct virtqueue *vq)
[Severity: High]
When in_intr() races with unplug_port() after find_port_by_vq() drops
ports_lock, can in_intr() dereference a NULL port->portdev pointer and
leak port->inbuf?
With the kref keeping struct port alive while unplug_port() executes on
another CPU, unplug_port() clears guest_connected, drains the port data,
and sets port->portdev to NULL:
drivers/char/virtio_console.c:unplug_port() {
...
spin_lock_irq(&port->inbuf_lock);
if (port->guest_connected) {
...
port->guest_connected = false;
port->host_connected = false;
...
}
spin_unlock_irq(&port->inbuf_lock);
...
remove_port_data(port);
port->portdev = NULL;
...
kref_put(&port->kref, remove_port);
}
If in_intr() acquires port->inbuf_lock after unplug_port() has run, it
pops a buffer into port->inbuf and then checks port->portdev->vdev while
port->guest_connected is false:
drivers/char/virtio_console.c:in_intr() {
...
spin_lock_irqsave(&port->inbuf_lock, flags);
port->inbuf = get_inbuf(port);
...
if (!port->guest_connected && !is_rproc_serial(port->portdev->vdev))
discard_port_data(port);
...
}
Because port->portdev is now NULL, evaluating port->portdev->vdev
dereferences NULL in interrupt context.
Additionally, if port->portdev becomes NULL just before discard_port_data()
runs (or on an rproc_serial port), discard_port_data() returns early
without freeing port->inbuf:
drivers/char/virtio_console.c:discard_port_data() {
if (!port->portdev) {
/* Device has been unplugged. vqs are already gone. */
return;
}
...
}
When in_intr() then drops the final reference via kref_put(&port->kref,
remove_port), remove_port() only calls kfree(port), leaking the struct
port_buffer stored in port->inbuf.
[Severity: Critical]
In the same race between in_intr() and unplug_port() on a console port,
can in_intr() pass a freed port->cons.hvc pointer to hvc_poll()?
While in_intr() holds a kref on struct port after find_port_by_vq() drops
ports_lock, unplug_port() tears down the HVC console without clearing
port->cons.hvc or synchronizing with in_intr():
drivers/char/virtio_console.c:unplug_port() {
...
if (is_console_port(port)) {
spin_lock_irq(&pdrvdata_lock);
list_del(&port->cons.list);
spin_unlock_irq(&pdrvdata_lock);
hvc_remove(port->cons.hvc);
ida_free(&vtermno_ida, port->cons.vtermno);
}
...
}
hvc_remove() drops the tty_port reference, which frees the struct
hvc_struct in hvc_port_destruct() if no tty holds an open reference. When
in_intr() then reaches:
if (is_console_port(port) && hvc_poll(port->cons.hvc))
hvc_kick();
is_console_port(port) still sees a non-NULL port->cons.hvc and passes the
dangling pointer to hvc_poll().
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-319-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] virtio_balloon: warn on failed buffer add in tell_host()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (5 preceding siblings ...)
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] virtio_console: take a kref in find_port_by_vq() to fix port UAF Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
2026-10-19 14:59 ` sashiko-bot
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-5.10] virtio_console: fix control queue race during restore Sasha Levin
` (2 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: Denis V. Lunev, David Hildenbrand, Michael S. Tsirkin,
Sasha Levin, jasowangio, eperezma, virtualization, linux-kernel
From: "Denis V. Lunev" <den@openvz.org>
[ Upstream commit 198eda395067b56ee605de076f0519620f4e5d90 ]
tell_host() ignores the return value of virtqueue_add_outbuf() and goes
on to kick the queue and wait_event() for the host's ack. The comment
claims "We should always be able to add one buffer to an empty queue",
but that does not hold once the virtqueue has been broken (e.g. on
device shutdown): the add then fails with -EIO and the following
wait_event() would block forever on a buffer the host can never return.
Warn and bail out on failure, mirroring virtballoon_free_page_report().
Suggested-by: David Hildenbrand <david@kernel.org>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-ID: <20260624140846.2616797-5-den@openvz.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This tree is **6.12.111** (`pending-6.12`, `git describe`
`p-6.12-3-gc504cf0baa3d8`). The verdict is only for this checkout.
The candidate is `198eda395067b` (`virtio_balloon: warn on failed buffer
add in tell_host()`). It is **not** an ancestor of `HEAD`. The buggy
`tell_host()` is present and the patch applies cleanly.
---
## Phase 1: Commit message forensics
**Step 1.1 — Subject**
Record: **[virtio_balloon] [warn] Check `virtqueue_add_outbuf()` in
`tell_host()` and return instead of waiting forever**
**Step 1.2 — Tags**
Record:
- Suggested-by: David Hildenbrand `<david@kernel.org>`
- Signed-off-by: Denis V. Lunev `<den@openvz.org>` (author)
- Signed-off-by: Michael S. Tsirkin `<mst@redhat.com>` (virtio
maintainer)
- Message-ID: `<20260624140846.2616797-5-den@openvz.org>` (v2 4/4)
- No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Cc: stable, or
Link: on the committed message (absence is expected)
**Step 1.3 — Body**
Record:
- **Bug:** `tell_host()` ignores `virtqueue_add_outbuf()`. The comment
assumes an empty queue can always take one buffer.
- **Symptom:** After a broken virtqueue (`-EIO` on device shutdown /
already-broken vq), `wait_event(vb->acked, virtqueue_get_buf(...))`
waits forever because the host will never return a buffer that was
never queued.
- **Root cause:** Broken-queue add failure is not treated as fatal; kick
+ wait still run.
- **Fix:** `WARN_ON_ONCE(err)` and return, same as
`virtballoon_free_page_report()`.
- **Version note:** Message cites “device shutdown”. That specific
trigger is `8bd2fa086a04` (2025-02-24), which is **not** in this 6.12
tree. Other broken-queue triggers **are** in this tree (below).
**Step 1.4 — Hidden bug fix?**
Record: Not hidden. It is an explicit hang fix. It also finishes the
2014 change that removed `BUG()` on add failure but left the wait.
---
## Phase 2: Diff analysis
**Step 2.1 — Inventory**
Record:
- File: `drivers/virtio/virtio_balloon.c` only (~+4 / −2)
- Function: `tell_host()`
- Scope: single-function surgical check
**Step 2.2 — Flow**
Record:
- **Before:** `virtqueue_add_outbuf()`; ignore result;
`virtqueue_kick()`; `wait_event` until `virtqueue_get_buf()` is non-
NULL.
- **After:** same, but if add returns non-zero, warn once and return (no
kick, no wait).
- Path: inflate/deflate notify to host (normal and remove/leak paths).
**Step 2.3 — Mechanism**
Record:
- **Category:** logic / correctness — ignored error leading to infinite
wait
- **Broken:** add on `vq->broken` returns `-EIO` (`virtqueue_add` in
`virtio_ring.c`). `virtqueue_get_buf_*` returns NULL on broken.
`wait_event` never completes.
- **Fix:** abort when add fails.
**Step 2.4 — Quality**
Record:
- Obviously correct; same pattern already in
`virtballoon_free_page_report()` in this file.
- Regression risk is low. Early return from `fill_balloon()` can leave
pages ballooned without a host ack if add failed for a reason other
than a dead device; on a broken vq that is the intended abort.
`leak_balloon()` still runs `release_pages_balloon()` after
`tell_host()`, so surprise-remove still returns pages.
- `WARN_ON_ONCE` + `panic_on_warn` could panic instead of hang. That is
the existing reporting-path policy. In **this** tree, shutdown does
not break virtio devices, so this does not add reboot-time WARNs.
---
## Phase 3: Git history investigation
**Step 3.1 — Blame**
Record:
- `tell_host()`: `6b35e40767c6c1` (2008, original balloon driver)
- Ignored add return: `4951cc90831470` (2014, Rusty Russell, “don't
crash if virtqueue is broken”) — replaced `if (add < 0) BUG()` with an
unchecked add. That avoided a crash and left a hang.
- Reporting-path WARN: `b0c504f1547189` (2020)
**Step 3.2 — Fixes: tag**
Record: None. Natural parent is `4951cc90831470` (present in this tree).
**Step 3.3 — File history**
Record: Recent balloon work here is stats/wakeup/indirect-desc, not
`tell_host()`. No competing fix for this hang. Not already applied.
**Step 3.4 — Author**
Record: Denis V. Lunev is a long-time balloon contributor
(`997e120843e82`, etc.). Applied by MST. Suggested by David Hildenbrand.
**Step 3.5 — Dependencies**
Record: Standalone. v2 series is 4/4:
1. `virtio_device_shutdown()` helper — needs `virtio_dev_shutdown()`
from `8bd2fa086a04` (not here)
2. `virtballoon_quiesce()` factor-out
3. `.shutdown` quiesce (Fixes: `8bd2fa086a04`)
4. **This patch** — no new helpers, applies to current `tell_host()`
This commit does not need 1–3.
---
## Phase 4: Mailing list and external research
**Step 4.1 — Original thread**
Record:
- Lore: https://patch.msgid.link/20260624140846.2616797-5-den@openvz.org
- Cover: `[PATCH v2 0/4] virtio_balloon: quiesce balloon work on device
shutdown`
- Cover URL: https://lore-
kernel.gnuweeb.org/lkml/20260624140846.2616797-1-den@openvz.org/
- v1 (2026-06-22) was 2 patches (quiesce only). v2 added this tell_host
check (Hildenbrand) and kept it separate (Tsirkin).
- Field report in the cover: kexec/cluster upgrade + `panic_on_warn` on
the **reporting** WARN after `8bd2fa086a04`. That trigger is **not**
in 6.12. Cover also states inflate/deflate `wait_event` would block
forever on a broken queue.
**Step 4.2 — Recipients (b4 dig -w)**
Record: `mst@redhat.com`, `david@kernel.org`,
`virtualization@lists.linux.dev`, `linux-kernel@vger.kernel.org`. Right
maintainers.
**Step 4.3 — Review**
Record: David Hildenbrand asked about `stats_handle_request()` (no
`wait_event`; sibling `d62fb5cc8a432`, not required). No NAK. No
reviewer `Cc: stable`.
**Step 4.4 — Series**
Record: This hunk is independent. 1–3 fix post-6.12 shutdown-break. They
cannot and need not go in with this one.
**Step 4.5 — Stable list**
Record: gnuweeb search for `virtio_balloon tell_host stable` found old
balloon stable backports, not this patch. lore.kernel.org/stable
returned 403.
---
## Phase 5: Code semantic analysis
**Step 5.1 — Functions**
Record: `tell_host()` only.
**Step 5.2 — Callers (this tree)**
Record:
- `fill_balloon()` → `update_balloon_size_func()` work; inflate
- `leak_balloon()` → size work, `virtio_balloon_oom_notify()`,
**`remove_common()`**
- `virtballoon_migratepage()` (compaction)
- `remove_common()` ← `virtballoon_remove()`, `virtballoon_freeze()`
**Step 5.3 — Callees**
Record: `sg_init_one`, `virtqueue_add_outbuf` → `-EIO` if `vq->broken`,
`virtqueue_kick`, `wait_event` + `virtqueue_get_buf` (NULL if broken).
**Step 5.4 — Reachability**
Record: Userspace/hypervisor: balloon size change, OOM deflate,
compaction, unplug/freeze. Host/PCI/CCW can mark the device broken
**before** `remove_common()`:
```754:759:drivers/virtio/virtio_pci_common.c
- Device is marked broken on surprise removal so that virtio upper
- layers can abort any ongoing operation.
*/
if (!pci_device_is_present(pci_dev))
virtio_break_device(&vp_dev->vdev);
```
Same idea on s390: `virtio_ccw_remove` / `virtio_ccw_offline` call
`virtio_break_device()` when `device_lost` before
`unregister_virtio_device()`.
Then `remove_common()` → `leak_balloon()` → `tell_host()` on a broken
deflate vq → infinite wait without this patch.
**Step 5.5 — Similar patterns**
Record: `virtballoon_free_page_report()` already WARNs and returns.
`stats_handle_request()` still ignores add (no hang). 2014 commit
removed `BUG()` in both `tell_host` and stats.
---
## Phase 6: Cross-referencing this tree
**Step 6.1 — Buggy code here?**
Record: Yes. `tell_host()` at `drivers/virtio/virtio_balloon.c:184-198`
still ignores add and waits. Bug since 2008/2014. `8bd2fa086a04` is
**not** an ancestor of `HEAD` (`git merge-base --is-ancestor` = no).
`virtio_dev_shutdown` / `virtballoon_quiesce` are absent. Surprise-break
(`43bb40c5b92659`, 2021) **is** here.
**Step 6.2 — Backport difficulty**
Record: Clean apply. `tell_host()` matches the hunk.
`finish_wakeup_event` is already adjacent context.
**Step 6.3 — Related fixes already here?**
Record: No tell_host add-check. Reporting path already has the WARN. No
duplicate.
---
## Phase 7: Subsystem and maintainer context
**Step 7.1**
Record: **virtio balloon** (`drivers/virtio/`). **IMPORTANT** for VMs
(KVM/QEMU, s390 ccw), not core mm/vfs.
**Step 7.2**
Record: Mature driver; balloon core is old. Recent 6.12 backports are
stats/wakeup/indirect-desc, not a rewrite of `tell_host()`.
---
## Phase 8: Impact and risk
**Step 8.1 — Who**
Record: Guests with `CONFIG_VIRTIO_BALLOON` (very common). Triggered
when the vq is already broken, then inflate/deflate/`remove_common`
runs.
**Step 8.2 — Trigger**
Record:
- **In this 6.12 tree:** surprise PCI remove (`!pci_device_is_present`)
or ccw `device_lost` → break → `leak_balloon`/`tell_host` hang. Also
any prior `virtio_break_device()` / `vq->broken` before the next
`tell_host()`.
- **Not in this tree:** `device_shutdown()` break/reset (`8bd2fa086a04`)
— the series’ kexec/`panic_on_warn` story.
- Not a normal unprivileged syscall. Host/device loss or hot-unplug.
**Step 8.3 — Failure mode**
Record: Uninterruptible `wait_event` in balloon work or `remove_common`.
Device unbind / `unregister_virtio_device` / `cancel_work_sync` can
stick. **Severity: HIGH** (hang), not a UAF/corruption.
This patch does **not** unstick a wait that already started after a
successful add (notify then fail / break while waiting). It stops the
**next** `tell_host()` after the vq is already broken — the remove/leak
path after surprise-break.
**Step 8.4 — Risk vs benefit**
Record:
- Benefit: high for the hang path; medium overall (surprise-loss is less
common than the post-6.13 shutdown path, but the hang is real and the
driver is supposed to abort).
- Risk: very low (4 lines, established pattern).
- Ratio: favor backport.
---
## Phase 9: Final synthesis
**Step 9.1 — Evidence**
For:
- Real infinite `wait_event` if add fails (verified: add `-EIO`, get_buf
NULL on broken).
- `remove_common()` always leaks remaining pages via `tell_host()` after
surprise-break in this tree.
- Completes 2014 “don’t BUG” change that left a hang.
- Tiny, obviously correct, same as existing reporting helper.
- Applies cleanly; no series 1–3 needed.
- Maintainer-applied; suggested by balloon reviewer.
Against:
- Main series story (`8bd2fa086a04` kexec/`panic_on_warn`) is not in
6.12.
- No Fixes:/Cc: stable (not a negative per rules).
- Surprise-remove is less common than graceful unplug.
- `WARN_ON_ONCE` + `panic_on_warn` trades hang for panic if add fails;
same as reporting path; shutdown in 6.12 does not break vqs, so no new
reboot WARNs.
Unresolved: lore.kernel.org/stable HTTP 403. gnuweeb search did not show
a prior rejection of this patch.
**Step 9.2 — Stable rules**
1. Obviously correct and tested? **PASS** — 4-line check; reviewed; MST
applied; series tested for the later shutdown bug.
2. Real user-visible bug? **PASS** — hang on broken vq / surprise
remove.
3. Important (hang/deadlock)? **PASS** — unbounded wait in
remove/inflate/deflate.
4. Small and contained? **PASS**
5. No new features/APIs? **PASS**
6. Applies to this tree? **PASS**
**Step 9.3 — Exception category**
Record: None (not an ID/quirk/DT/build/doc exception). Qualifies as a
hang fix.
**Step 9.4 — Decision**
The missing `8bd2fa086a04` path does not make this a no-op here. The
unchecked add + `wait_event` is in this tree, and surprise PCI/CCW
break-then-remove is the designed “upper layer must abort” path.
Backporting this patch alone does not require the quiesce series and
does not introduce shutdown WARNs on 6.12.
---
## What it solves for 6.12 users
On a broken balloon virtqueue, `tell_host()` currently kicks and waits
for an ack that cannot arrive. After surprise PCI/CCW removal this tree
already marks the device broken, then `remove_common()` →
`leak_balloon()` → `tell_host()`, so unbind/remove can hang. The patch
warns and returns, matching the reporting helper.
It meets stable rules: real hang, small, no new API, clean apply, no
missing prerequisites in this tree.
Risk is low versus an unbounded wait.
---
## Verification
- **[Phase 1]** Parsed message/tags from the supplied commit and `git
log -1` of `198eda395067b` on `all-next`.
- **[Phase 1]** Cover letter: kexec/`panic_on_warn` after
`8bd2fa086a04`; also documents inflate/deflate `wait_event` hang.
- **[Phase 2]** Diff: 4 lines in `tell_host()` only; local file matches.
- **[Phase 2]** `virtqueue_add` returns `-EIO` if `vq->broken`
(`virtio_ring.c:565-568`). `virtqueue_get_buf_*` returns NULL if
broken (`834-836`, `1684-1686`).
- **[Phase 3]** Blame: `tell_host` `6b35e40767c6c1` (2008); unchecked
add `4951cc90831470` (2014, removed `BUG()`).
- **[Phase 3]** `git show 4951cc90831470`: confirmed BUG → ignore.
- **[Phase 3]** `git merge-base --is-ancestor 8bd2fa086a04 HEAD` →
**NO**. `198eda395067b` → **NO**.
- **[Phase 3]** `virtio_device_shutdown` / `virtballoon_quiesce` not in
this tree.
- **[Phase 4]** `b4 dig -c 198eda395067b`: msgid
`20260624140846.2616797-5-den@openvz.org`.
- **[Phase 4]** `b4 dig -a`: v1 quiesce-only; v2 added this patch.
- **[Phase 4]** `b4 dig -w`: MST + Hildenbrand + virtualization list.
- **[Phase 4]** Hildenbrand reply: asked about stats path; no NAK.
- **[Phase 4]** lore.kernel.org/stable: 403. gnuweeb: no discussion of
this patch for stable.
- **[Phase 5]** Callers: `fill_balloon`, `leak_balloon`, migratepage;
`remove_common` from remove/freeze.
- **[Phase 5]** `virtio_pci_remove` surprise-break: `43bb40c5b92659`
(2021), present. `virtio_ccw` break on `device_lost`, present.
- **[Phase 6]** `git describe HEAD` → `p-6.12-3-gc504cf0baa3d8`; `make
kernelversion` → 6.12.111.
- **[Phase 6]** Local `tell_host()` still unchecked; patch context
present; not already patched.
- **[Phase 8]** Failure mode: infinite `wait_event` / stuck remove.
Severity HIGH.
- **UNVERIFIED:** Whether any 6.12.y discussion already rejected this
(stable list fetch failed). Not used to decide.
The hang is real in this tree, the fix is small and self-contained, and
it should be backported.
**YES**
drivers/virtio/virtio_balloon.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index a3995e77a652c..0386e7b459bb1 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -185,16 +185,18 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
{
struct scatterlist sg;
unsigned int len;
+ int err;
sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
/* We should always be able to add one buffer to an empty queue. */
- virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
+ err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
+ if (WARN_ON_ONCE(err))
+ return;
virtqueue_kick(vq);
/* When host has read buffer, this completes via balloon_ack */
wait_event(vb->acked, virtqueue_get_buf(vq, &len));
-
}
static int virtballoon_free_page_report(struct page_reporting_dev_info *pr_dev_info,
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH AUTOSEL 6.18-5.10] virtio_balloon: warn on failed buffer add in tell_host()
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] virtio_balloon: warn on failed buffer add in tell_host() Sasha Levin
@ 2026-10-19 14:59 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-19 14:59 UTC (permalink / raw)
To: Sasha Levin; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> From: "Denis V. Lunev" <den@openvz.org>
>
> [ Upstream commit 198eda395067b56ee605de076f0519620f4e5d90 ]
>
> tell_host() ignores the return value of virtqueue_add_outbuf() and goes
> on to kick the queue and wait_event() for the host's ack. The comment
> claims "We should always be able to add one buffer to an empty queue",
> but that does not hold once the virtqueue has been broken (e.g. on
> device shutdown): the add then fails with -EIO and the following
> wait_event() would block forever on a buffer the host can never return.
>
> Warn and bail out on failure, mirroring virtballoon_free_page_report().
>
> Suggested-by: David Hildenbrand <david@kernel.org>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Message-ID: <20260624140846.2616797-5-den@openvz.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-356-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] virtio_console: fix control queue race during restore
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (6 preceding siblings ...)
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] virtio_balloon: warn on failed buffer add in tell_host() Sasha Levin
@ 2026-10-19 13:51 ` Sasha Levin
2026-10-19 15:11 ` sashiko-bot
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18] drm/virtio: abort virtqueue wait on device removal to avoid hung task Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-6.12] vdpa/mlx5: roll back MR update after VQ setup failure Sasha Levin
9 siblings, 1 reply; 20+ messages in thread
From: Sasha Levin @ 2026-10-19 13:51 UTC (permalink / raw)
To: patches, stable
Cc: Sungho Bae, Greg Kroah-Hartman, Sasha Levin, amit, virtualization,
linux-kernel
From: Sungho Bae <baver.bae@lge.com>
[ Upstream commit 952261953cf79820aa4350679882db0d4f71f493 ]
In virtcons_restore(), after virtio_device_ready() sets DRIVER_OK, the
device becomes active. If the control receive queue (c_ivq) is populated
immediately, the host can instantly deliver pending control messages
(e.g., VIRTIO_CONSOLE_PORT_REMOVE).
This triggers the control_work_handler(), which can modify the
portdev->ports list concurrently with the unprotected list_for_each_entry
loop in virtcons_restore(), leading to list corruption or Use-After-Free.
Fix this by deferring the population of the control receive queue
(fill_queue for c_ivq) until after the list iteration is complete. This
ensures the host cannot inject control messages during the vulnerable
window.
Signed-off-by: Sungho Bae <baver.bae@lge.com>
Link: https://patch.msgid.link/20260603183757.21587-4-baver.bae@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.6.157** (`pending-6.6`). The buggy restore path is
present, the patch applies cleanly, and it is a self-contained UAF/list-
corruption fix.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[virtio_console] [fix] [control queue race during restore]**
- Subsystem: `virtio_console` (guest virtio-serial/console driver)
- Action verb: **fix** — explicit bug fix
- Claimed intent: close a restore-time race between filling the control
receive queue and iterating `portdev->ports`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Fixes:** none (expected for this review pipeline)
- **Reported-by:** none
- **Tested-by:** none on the commit itself; the v3 cover letter says it
was runtime-tested on arm64 S4 with `no_console_suspend`
- **Reviewed-by / Acked-by:** none
- **Link:**
`https://patch.msgid.link/20260603183757.21587-4-baver.bae@gmail.com`
(v3 3/4 of a 4-patch series)
- **Cc: stable:** none (expected; not a negative)
- **Signed-off-by:** Sungho Bae `<baver.bae@lge.com>` (author); Greg
Kroah-Hartman (char-misc maintainer). Ignore any Sasha Levin SOB from
candidate prep.
- Mainline SHA (from `fixes-next`, not from candidate markers):
`952261953cf79820aa4350679882db0d4f71f493`
- AuthorDate: 2026-06-04; CommitDate: 2026-07-31
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** After `virtio_device_ready()` sets `DRIVER_OK`, immediately
`fill_queue(c_ivq)` lets the host deliver control packets (e.g.
`VIRTIO_CONSOLE_PORT_REMOVE`) while `virtcons_restore()` is still
walking `portdev->ports` with unlocked `list_for_each_entry`.
- **Symptom / failure mode:** `control_work_handler()` →
`handle_control_message()` → `unplug_port()` does `list_del` +
`kref_put` concurrently with that walk → **list corruption or use-
after-free**.
- **Version info:** none in the message. The restore path dates to 2011;
`virtio_device_ready()` before queue use dates to 2014 (see Phase 3).
- **Root cause (author):** host can inject control messages as soon as
`c_ivq` has buffers after `DRIVER_OK`.
- **Fix:** move `fill_queue(portdev->c_ivq)` until after the list walk.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. Subject and body describe a UAF/list-corruption
race. The extra comment is documentation of that race, not a cleanup
disguised as a fix.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/char/virtio_console.c` only
- Scope: **+12 / −3** (net +9, one function)
- Function: `virtcons_restore()`
- Classification: **single-file surgical fix**
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record: **Before** (this tree, `virtcons_restore` at lines 2201–2235):
1. `init_vqs(portdev)`
2. `virtio_device_ready()` → `DRIVER_OK`, notifications enabled
3. `fill_queue(c_ivq)` if multiport → host can write control packets
4. Unlocked `list_for_each_entry` over `portdev->ports`: rebind vqs,
`fill_queue(in_vq)`, `PORT_READY` / `PORT_OPEN`
**After:** steps 3 and 4 swap. Control IN buffers are added only after
the walk finishes.
Affected path: **PM restore** (`CONFIG_PM_SLEEP`), not probe. Probe
already fills `c_ivq` before ports exist, so it does not have this walk.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Race / UAF / list corruption]**
Verified chain in this tree:
- `control_intr()` (`drivers/char/virtio_console.c:1792`) →
`schedule_work(&portdev->control_work)`
- `control_work_handler()` (`:1696`) → `handle_control_message()`
- `VIRTIO_CONSOLE_PORT_REMOVE` (`:1597`) → `unplug_port()`
- `unplug_port()` (`:1507`) takes `ports_lock`, `list_del(&port->list)`,
later `kref_put`
- `PORT_ADD` (`:1580`) can `add_port()` → `list_add_tail` under
`ports_lock` (`:1445`)
- `virtcons_restore()` walks the same list **without** `ports_lock`
(`:2218`)
Other walkers (`find_port_by_id`, `find_port_by_devt_in_portdev`) do
take `ports_lock`. Restore is the unprotected one.
Without `c_ivq` buffers, the host cannot complete used descriptors on
the control IN queue, so `control_intr` cannot fire. Deferring
`fill_queue(c_ivq)` closes the window. `send_control_msg()` uses `c_ovq`
only (`__send_control_msg` at `:541`), so PORT_READY/OPEN still go out
during the loop; host replies wait until IN buffers exist.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Obviously correct:** same two calls, safer order. Matches how virtio
queues work (no IN buffers ⇒ no host-to-guest control).
- **Minimal:** no API, no new fields, no locking change.
- **Regression risk:** low. Host replies to PORT_READY are delayed until
`c_ivq` is filled; that is normal virtio backpressure. Probe already
sends DEVICE_READY only after `fill_queue(c_ivq)` (`:2089–2118`);
restore now fills `c_ivq` after PORT_READY, which is slightly
different timing but still valid.
- Residual: the restore walk still does not take `ports_lock`. After
this patch the only concurrent list mutator on this path is host
control, which cannot run until `c_ivq` is filled. Device remove is
serialized with PM restore via the driver/device lock. That is enough
for this bug.
- Patch 4 of the same series (`pm_freezing`) is a **different**
freeze/TX race and is not required for this reorder.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Unlocked restore loop and `fill_queue(c_ivq)` before it:
**`2b8f41d846990c`** (Amit Shah, 2011-12-22) “Add freeze and restore
handlers to support S4”. Present in this tree (`git merge-base --is-
ancestor` succeeded).
- `virtio_device_ready()` immediately before `fill_queue(c_ivq)`:
**`401bbdc901b268`** (Michael S. Tsirkin, 2014-10-15) “enable VQs
early on restore”. Also an ancestor of HEAD. That is when the host can
legally deliver as soon as buffers are posted.
- Race has existed in this form since ~2014; this is old code, not a
recent-mainline-only bug.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. The enabling commit is `401bbdc901b268`, which
is in this tree. No intermediate fix of this same race on
`drivers/char/virtio_console.c` in this checkout (`git log
--grep="control queue race"` empty on HEAD).
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Recent local history (`git log --oneline -20 --
drivers/char/virtio_console.c`) is unrelated (control-out free,
endian/cols-rows, probe bugs). This commit is **not** in HEAD (`git
merge-base --is-ancestor 952261953cf79 HEAD` failed).
On `fixes-next` the series is:
1. `fc220d6be3c7e` refactor `__send_to_port()` buffer ownership
2. `a616ad344d9f7` hot-unplug races in TX paths
3. **`952261953cf79` this commit**
4. `b187532ac6dba` hvc `put_chars` vs freeze teardown (`pm_freezing`)
Patches 1–2 do not touch `virtcons_restore`. Patch 4 adds `pm_freezing`
clear after the moved `fill_queue`; it does not change this race fix.
**Standalone.**
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Sungho Bae has no commits on this file in the 6.6 tree. On
`fixes-next` he authored this 4-patch virtio_console PM/hot-unplug set.
Maintainer of record is Amit Shah; the patch was To: amit@kernel.org,
arnd, gregkh and applied by Greg.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Diff uses only `virtio_device_ready`, `use_multiport`,
`fill_queue`, `c_ivq` / `c_ivq_lock`, all present. `git show
952261953cf79 | git apply --check` **applies cleanly**. No prerequisite.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 952261953cf79`: match at
https://patch.msgid.link/20260603183757.21587-4-baver.bae@gmail.com
- `b4 dig -a`: only **v3** of `[PATCH v3 0/4] virtio_console: fix
suspend/resume and hot-unplug races`
- v3 change note: split a monolithic patch for grouping/bisectability
- v1
(https://patchew.org/linux/20260515225259.1054-1-baver.bae@gmail.com/)
was only the freeze/`pm_freezing` fix; **this restore reorder is new
in the split v3**
- Saved mbox: five messages (0/4–4/4), **no replies, no NAK, no Cc:
stable**
- Greg applied it 2026-07-31
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc Amit Shah, Arnd Bergmann, Greg Kroah-
Hartman, `virtualization@lists.linux.dev`, `linux-kernel`. Appropriate
lists/maintainers. No public Reviewed-by; Greg’s SOB is the merge.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot, bugzilla, or user crash report. Cover letter: found
while investigating the freeze/`put_chars` race; runtime-tested on arm64
S4 with `no_console_suspend`. Mechanism is verified in code (Phase 2/5),
not only asserted.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Series of 4; this is 3/4 and is self-contained. Patch 4 is a
separate freeze BUG_ON and does not need to ride along. Later `fixes-
next` commits (`allocate the port_buffer with the caller's gfp`, `do not
free control-out buffers on remove`) do not revert this reorder.
`virtcons_restore` on `fixes-next` still has fill-`c_ivq`-after-loop.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: `lore.kernel.org/stable/` fetch returned 403. No stable-thread
evidence in the v3 mbox. **UNVERIFIED** whether anyone already asked for
or rejected this on stable@. Absence of that discussion is not a reason
to skip a UAF fix that applies here.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `virtcons_restore()` only. Related callees: `fill_queue`,
`use_multiport`, `send_control_msg`, `virtio_device_ready`. Related
concurrent path: `control_work_handler` / `unplug_port` / `add_port`.
**Step 5.2: TRACE CALLERS**
Record: `virtcons_restore` is `.restore` of `struct virtio_driver
virtio_console` (`:2247–2249`), compiled under `CONFIG_PM_SLEEP`.
Invoked from virtio PM restore (guest S3/S4 / hibernation restore of the
virtio-console device). Not a syscall hot path; **reachable on any guest
that suspends/hibernates with this driver**.
**Step 5.3: TRACE CALLEES**
Record:
- `init_vqs` registers `control_intr` for the control IN vq (`:1876`)
- `virtio_device_ready` sets `VIRTIO_CONFIG_S_DRIVER_OK`
(`include/linux/virtio_config.h:270–297`)
- `fill_queue` allocates PAGE_SIZE IN buffers and `add_inbuf` under the
given lock (`:1323`)
- `send_control_msg` → `__send_control_msg` kicks **c_ovq** only
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Guest suspend/resume or hibernation of a VM using multiport
virtio-console/serial. Host can have pending PORT_REMOVE (port gone
while guest was down) or reply to PORT_READY with PORT_ADD/REMOVE/NAME.
Userspace in the guest does not need to issue a special syscall; PM
restore plus host control is enough. Unprivileged guest users do not
directly trigger it; the **hypervisor/host** does. Severity is still
guest kernel UAF.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: `virtcons_freeze()` also walks `portdev->ports` unlocked
(`:2186`), but first `virtio_reset_device` +
`virtqueue_disable_cb(c_ivq)` + `cancel_work_sync(&control_work)`
(`:2173–2184`), so control work cannot run. Probe fills `c_ivq` before
any ports are on the list. Restore was the inconsistent path.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `drivers/char/virtio_console.c:2213–2234` still does
`virtio_device_ready` → `fill_queue(c_ivq)` → unlocked
`list_for_each_entry`. Tree: `pending-6.6`, `make kernelversion`
**6.6.157**, `git describe` `p-6.6-4-gaf85e5fc2f380`. Bug introduced
~2014 (`401bbdc901b268`), long before 6.6.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply** (`git apply --check` on `952261953cf79`
succeeded). Surrounding `virtcons_restore` matches the pre-image. No
refactor gap.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This SHA is not an ancestor of HEAD. No other “control queue
race” commit on this file in this tree. Freeze-path callback disable is
present and unrelated.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **virtio_console** (`drivers/char/virtio_console.c`,
`CONFIG_VIRTIO_CONSOLE`). **IMPORTANT** for KVM/QEMU/cloud/embedded
guests (virtio-serial, guest console). Not mm/core, but not
staging/obscure. Maintainers: Amit Shah; virtio core: MST / Jason Wang.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature. Freeze/restore since 2011; little churn on this function
in 6.6. Long-lived bug, not a new-subsystem mismatch.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-specific + config-specific:** `CONFIG_VIRTIO_CONSOLE` +
`CONFIG_PM_SLEEP` + multiport (`VIRTIO_CONSOLE_F_MULTIPORT`, normal for
virtio-serial). Guest suspend/hibernate/restore users.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Guest PM restore while host delivers control messages (pending
PORT_REMOVE, or replies to PORT_READY). Window includes
`fill_queue(c_ivq)` (many PAGE_SIZE allocations) plus the port loop. Not
every boot; **realistic on S3/S4 of VMs with virtio-serial**. Guest
userspace need not be malicious; the host is the other party (trusted in
the virtio model, but still a crash in the guest).
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Use-after-free and list corruption** → oops/panic, possible
memory corruption. **Severity: HIGH** (CRITICAL if the UAF is hit). Not
a WARN-only issue.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** HIGH — prevents UAF/list corruption on a common VM driver
during restore; code has been wrong for years in this tree.
- **Risk:** VERY LOW — 9-line reorder, same helpers, applies cleanly, no
new API/locking.
- **Ratio:** strongly favors backport.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real race: unlocked list walk vs `unplug_port`/`add_port` from control
work
- Failure is UAF / list corruption (stable-class bug)
- Buggy code is in this 6.6 tree and has been since ~2014
- Fix is a 9-line reorder; `git apply --check` clean
- No dependency on series patches 1/2/4
- Applied by Greg to char-misc; later `fixes-next` still has this order
- Author tested related PM paths on arm64 S4
**AGAINST:**
- No `Reported-by` / syzbot; found during investigation
- Restore-only (S3/S4), not every boot
- No public review comments in the v3 mbox
- Part of a 4-patch series (but this hunk is independent)
**UNRESOLVED:**
- Stable@ discussion (lore/stable 403)
- No independent crash log attached to this specific UAF (mechanism
still verified in tree)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — reorder is locally
checkable; series was S4-tested; merged by Greg
2. Fixes a real bug that affects users? **PASS** — restore + multiport
virtio-console is a real guest path
3. Important issue? **PASS** — UAF / list corruption, HIGH
4. Small and contained? **PASS** — one function, +12/−3
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — clean apply; all symbols
exist
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. It qualifies as a
normal **small race/UAF fix**.
**Step 9.4: MAKE YOUR DECISION**
This should be backported to **this** 6.6 tree. The bug exists here, the
fix is surgical and independent of the rest of the series, and the
downside is a mild delay of host control replies until after ports are
rebound — which is the intended barrier.
------------------------------------------------
What it solves for stable users
------------------------------------------------
On guest suspend/resume or hibernation restore, `virtcons_restore()`
currently posts control IN buffers while still walking `portdev->ports`
without `ports_lock`. A host `PORT_REMOVE`/`PORT_ADD` can then free or
insert list nodes under that walk. 6.6 guests using virtio-serial (the
usual QEMU multiport setup) can oops or corrupt kernel memory. Moving
`fill_queue(c_ivq)` after the loop removes that window without changing
APIs.
Risk vs benefit: benefit is avoiding a restore-time UAF; risk is a
small, obviously correct reorder that already applies.
------------------------------------------------
Verification
------------------------------------------------
- [Phase 1] Parsed subject/tags: fix, Link msgid
`20260603183757.21587-4`, SOB Sungho Bae + Greg; no Fixes/Reported-
by/stable Cc
- [Phase 1] Mainline SHA from `fixes-next`:
`952261953cf79820aa4350679882db0d4f71f493` (not treated as “already
selected”)
- [Phase 2] Diff: only `virtcons_restore()`; move `fill_queue(c_ivq)`
after unlocked `list_for_each_entry`
- [Phase 2] `control_intr` → `control_work_handler` → `PORT_REMOVE` →
`unplug_port` `list_del`; restore walk has no `ports_lock` (lines
1507–1511, 1597–1598, 2218)
- [Phase 2] `__send_control_msg` uses `c_ovq` only (line 551)
- [Phase 3] Blame: loop/`fill_queue` from `2b8f41d846990c` (2011);
`virtio_device_ready` from `401bbdc901b268` (2014); both ancestors of
HEAD
- [Phase 3] `git merge-base --is-ancestor 952261953cf79 HEAD` → NOT in
this tree
- [Phase 3] Series 1–2 do not touch `virtcons_restore`; patch 4 is a
separate `pm_freezing` freeze race
- [Phase 4] `b4 dig -c 952261953cf79` → v3 3/4 lore URL; `-a` only v3;
`-w` amit/arnd/gregkh/virtualization; mbox has no replies
- [Phase 4] v1 patchew: freeze-only; this reorder appeared in v3
- [Phase 4] UNVERIFIED: stable@ archives (HTTP 403)
- [Phase 5] `virtio_device_ready` sets `DRIVER_OK`
(`include/linux/virtio_config.h:297`); `init_vqs` registers
`control_intr`
- [Phase 6] `git describe HEAD` `p-6.6-4-gaf85e5fc2f380`; `make
kernelversion` 6.6.157; buggy order at lines 2213–2234
- [Phase 6] `git show 952261953cf79 | git apply --check` → APPLIES
CLEANLY
- [Phase 8] Failure mode: UAF/list corruption on PM restore; severity
HIGH
- UNVERIFIED: no captured crash log for this exact UAF (code path still
verified)
The patch is small, surgical, and meets the stable criteria for this
tree.
**YES**
drivers/char/virtio_console.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 750c15966d00e..cfda31d5e86ec 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2172,9 +2172,6 @@ static int virtcons_restore(struct virtio_device *vdev)
virtio_device_ready(portdev->vdev);
- if (use_multiport(portdev))
- fill_queue(portdev->c_ivq, &portdev->c_ivq_lock);
-
list_for_each_entry(port, &portdev->ports, list) {
port->in_vq = portdev->in_vqs[port->id];
port->out_vq = portdev->out_vqs[port->id];
@@ -2191,6 +2188,18 @@ static int virtcons_restore(struct virtio_device *vdev)
if (port->guest_connected)
send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
}
+
+ /*
+ * Populate the control receive queue only after the list iteration
+ * is complete. If we fill this queue before iterating, the host could
+ * immediately deliver a VIRTIO_CONSOLE_PORT_REMOVE message.
+ * This would trigger the control workqueue, which modifies the
+ * portdev->ports list concurrently with the unprotected loop above,
+ * leading to a Use-After-Free and list corruption.
+ */
+ if (use_multiport(portdev))
+ fill_queue(portdev->c_ivq, &portdev->c_ivq_lock);
+
return 0;
}
#endif
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH AUTOSEL 6.18-5.10] virtio_console: fix control queue race during restore
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-5.10] virtio_console: fix control queue race during restore Sasha Levin
@ 2026-10-19 15:11 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-19 15:11 UTC (permalink / raw)
To: Sasha Levin; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] virtio_console: dropped host control replies in virtcons_restore()
--
commit 896c2558dbc3180e4d84cf033ee9bf699250ed69
Author: Sungho Bae <baver.bae@lge.com>
virtio_console: fix control queue race during restore
This commit defers populating the control receive queue (portdev->c_ivq) in
virtcons_restore() until after iterating over portdev->ports to prevent
concurrent list modification from control_work_handler().
Link: https://patch.msgid.link/20260603183757.21587-4-baver.bae@gmail.com
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 088182e54debd..6dd0cf78bf839 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -2136,9 +2136,6 @@ static int virtcons_restore(struct virtio_device *vdev)
>
> virtio_device_ready(portdev->vdev);
>
> - if (use_multiport(portdev))
> - fill_queue(portdev->c_ivq, &portdev->c_ivq_lock);
> -
> list_for_each_entry(port, &portdev->ports, list) {
> port->in_vq = portdev->in_vqs[port->id];
> port->out_vq = portdev->out_vqs[port->id];
> @@ -2155,6 +2152,18 @@ static int virtcons_restore(struct virtio_device *vdev)
> if (port->guest_connected)
> send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
> }
> +
> + /*
> + * Populate the control receive queue only after the list iteration
> + * is complete. If we fill this queue before iterating, the host could
> + * immediately deliver a VIRTIO_CONSOLE_PORT_REMOVE message.
> + * This would trigger the control workqueue, which modifies the
> + * portdev->ports list concurrently with the unprotected loop above,
> + * leading to a Use-After-Free and list corruption.
> + */
> + if (use_multiport(portdev))
> + fill_queue(portdev->c_ivq, &portdev->c_ivq_lock);
> +
> return 0;
> }
> #endif
[Severity: High]
Won't deferring fill_queue(portdev->c_ivq, &portdev->c_ivq_lock) until after
the loop cause the host's control replies to be dropped during restore?
During freeze, virtcons_freeze() clears port->host_connected for every port
and destroys all virtqueues:
drivers/char/virtio_console.c:virtcons_freeze() {
...
list_for_each_entry(port, &portdev->ports, list) {
virtqueue_disable_cb(port->in_vq);
virtqueue_disable_cb(port->out_vq);
/*
* We'll ask the host later if the new invocation has
* the port opened or closed.
*/
port->host_connected = false;
remove_port_data(port);
}
remove_vqs(portdev);
...
}
In virtcons_restore(), the list_for_each_entry() loop queries the host for
each port's open/close status via send_control_msg() -> __send_control_msg():
drivers/char/virtio_console.c:virtcons_restore() {
...
list_for_each_entry(port, &portdev->ports, list) {
...
/* Get port open/close status on the host */
send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
...
}
}
Because __send_control_msg() kicks portdev->c_ovq and busy-waits
synchronously until the host consumes the VIRTIO_CONSOLE_PORT_READY request:
drivers/char/virtio_console.c:__send_control_msg() {
...
if (virtqueue_add_outbuf(vq, sg, 1, &portdev->cpkt, GFP_ATOMIC) == 0) {
virtqueue_kick(vq);\
while (!virtqueue_get_buf(vq, &len)
&& !virtqueue_is_broken(vq))
cpu_relax();
}
...
}
the host processes VIRTIO_CONSOLE_PORT_READY and immediately attempts to send
back VIRTIO_CONSOLE_PORT_OPEN (along with VIRTIO_CONSOLE_CONSOLE_PORT and
VIRTIO_CONSOLE_PORT_NAME) on portdev->c_ivq before __send_control_msg()
returns.
If fill_queue(portdev->c_ivq, &portdev->c_ivq_lock) has not been called yet,
portdev->c_ivq has zero receive buffers when the host replies, causing those
VIRTIO_CONSOLE_PORT_OPEN replies to be dropped and leaving
port->host_connected permanently stuck at false after restore (which causes
port_fops_write() to block or fail with -EAGAIN, port_fops_read() to return
EOF, and port_fops_poll() to report EPOLLHUP).
Would it work to keep fill_queue(portdev->c_ivq, &portdev->c_ivq_lock) before
the loop, but disable the c_ivq callback or defer scheduling control_work
until the loop finishes so the host has receive buffers available while list
traversal is still protected from concurrent control_work_handler() runs?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-390-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH AUTOSEL 6.18] drm/virtio: abort virtqueue wait on device removal to avoid hung task
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (7 preceding siblings ...)
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-5.10] virtio_console: fix control queue race during restore Sasha Levin
@ 2026-10-19 13:51 ` Sasha Levin
2026-10-19 15:24 ` sashiko-bot
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-6.12] vdpa/mlx5: roll back MR update after VQ setup failure Sasha Levin
9 siblings, 1 reply; 20+ messages in thread
From: Sasha Levin @ 2026-10-19 13:51 UTC (permalink / raw)
To: patches, stable
Cc: Ryosuke Yasuoka, Dmitry Osipenko, Sasha Levin, airlied, kraxel,
maarten.lankhorst, mripard, tzimmermann, simona, dri-devel,
virtualization, linux-kernel
From: Ryosuke Yasuoka <ryasuoka@redhat.com>
[ Upstream commit a46991b334f663fa0e0e9c051e50587ef977ac8b ]
virtio_gpu_queue_ctrl_sgs() and virtio_gpu_queue_cursor() use
wait_event() without any abort condition when waiting for virtqueue
space. If the host device stops processing commands, these waits block
indefinitely inside a drm_dev_enter/exit() critical section. Since
drm_dev_unplug(), which is called in device removal and system shutdown
call path, blocks on synchronize_srcu() until all critical sections
complete, device removal and system shutdown also hang.
Add a vqs_released flag to virtio_gpu_device and include it in the
wait_event() condition. Set the flag and wake up both queues in a new
virtio_gpu_release_vqs() helper, called before drm_dev_unplug() in both
virtio_gpu_remove() and virtio_gpu_shutdown(). When the flag is set, the
wait returns immediately and the command is aborted, following the same
cleanup path as drm_dev_enter() failure.
Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Link: https://patch.msgid.link/20260601-virtio-gpu_wait_event-v3-1-89530517a98a@redhat.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1 — Subject**
Record: `[drm/virtio] [abort] abort virtqueue wait on device removal to
avoid hung task`
**Step 1.2 — Tags**
Record:
- `Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>` (author)
- `Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>`
(virtio-gpu maintainer)
- `Link: https://patch.msgid.link/20260601-virtio-
gpu_wait_event-v3-1-89530517a98a@redhat.com`
- No `Fixes:`, `Reported-by:`, `Cc: stable` on the committed v3 (absence
is expected here)
v2 originally had two syzbot `Reported-by`/`Closes` tags. v3 changelog:
*“Remove Reported-by and Closes tag from commit msg because they are not
related to this fix.”* Those syzbot hung-task reports are a different
probe-time deadlock, already fixed in this tree by `89890a5fcefad`.
**Step 1.3 — Body**
Record:
- **Bug:** `wait_event()` on ctrl/cursor virtqueue space has no abort.
If the host stops processing commands, the waiter stays in D-state
inside `drm_dev_enter()`/`drm_dev_exit()`.
- **Symptom:** `drm_dev_unplug()` (remove and shutdown) blocks forever
on `synchronize_srcu()`, so device removal and system shutdown hang
(`INFO: task hung`).
- **Root cause:** uninterruptible wait holds an SRCU critical section
that unplug must drain.
- **Fix:** `vqs_released` flag + `wake_up_all()` before unplug; abort
with the same cleanup as `drm_dev_enter()` failure (`-ENODEV` / free
vbuf).
- **Versions:** none named in the message. The wait + `drm_dev_enter`
wrapping has been in this driver since 2020.
**Step 1.4 — Hidden bug fix?**
Record: Not hidden. Subject and body describe a hang/deadlock. It is a
real teardown hang, not cleanup-as-style.
---
## Phase 2: Diff analysis
**Step 2.1 — Inventory**
Record:
- `virtgpu_drv.c`: +15 — new `virtio_gpu_release_vqs()`, called from
`virtio_gpu_remove()` and `virtio_gpu_shutdown()`
- `virtgpu_drv.h`: +1 — `bool vqs_released` on `struct
virtio_gpu_device`
- `virtgpu_vq.c`: +21/−2 — abort conditions in
`virtio_gpu_queue_ctrl_sgs()` and `virtio_gpu_queue_cursor()`
- Scope: ~37 insertions, 2 deletions, three files, one driver. Surgical.
**Step 2.2 — Flow**
Record:
- **Before:** if `vq->num_free` is short, unlock, notify host,
`wait_event(ack_queue, num_free >= needed)` with no timeout/abort,
still inside `drm_dev_enter()`.
- **After:** wait also succeeds when `vqs_released`. Then unlock resv if
needed, `free_vbuf()`, `drm_dev_exit()`, return `-ENODEV` (ctrl) or
return (cursor). Same path as `drm_dev_enter()` failure.
- **Teardown:** set flag, wake both ack queues, *then*
`drm_dev_unplug()`.
- Path affected: virtqueue-full wait during remove/shutdown, not the
happy path.
**Step 2.3 — Bug mechanism**
Record: **Deadlock / hung-task (uninterruptible wait blocking SRCU
unplug).** `wait_event()` is `TASK_UNINTERRUPTIBLE`. `drm_dev_unplug()`
→ `drm_dev_synchronize_unplug()` → `synchronize_srcu(&drm_unplug_srcu)`
cannot finish while that section is held. Flag + `wake_up_all()` is a
standard abort.
**Step 2.4 — Fix quality**
Record: Obviously correct and minimal. Abort cleanup matches existing
`drm_dev_enter()` failure. Fence emit happens *after* the wait, so abort
does not leak an emitted fence. `vgdev` is `drmm_kzalloc()`’d so
`vqs_released` starts false. Flag is set only on remove/shutdown.
Regression risk is low: happy-path wait is unchanged; abort only when
tearing down. Remaining `wait_event(resp_wq)` in prime/vram is a
separate issue (author and maintainer agreed to leave it).
---
## Phase 3: Git history
**Step 3.1 — Blame (v6.18.53)**
Record:
- Ctrl `wait_event`: `6ebe8661ebf27a` (2020, Gerd/Chia-I) — `drm/virtio:
move locking into virtio_gpu_queue_ctrl_sgs`
- Wrapped in `drm_dev_enter`: `b1df3a2b24a917` (2020) — `drm/virtio: add
drm_driver.release callback`
- Cursor `wait_event`: `d02d270014f70d` (2018) / original queue from
`dc5698e80cf724` (2013)
- Bug has been in this driver for years; it is in 6.18.y.
**Step 3.2 — Fixes: tag**
Record: None. Introducing commits above exist in this tree.
**Step 3.3 — File history**
Record: Related in-tree: `89890a5fcefad` (probe deadlock / syzbot hung
task — different bug, already in v6.18.53). `2507789a724d6` implements
`virtio_gpu_shutdown` (in v6.18.53). Later candidate `3cdc509eb471e`
reclaims leftover vbufs on teardown — complementary, not a prerequisite
for this abort.
**Step 3.4 — Author**
Record: Ryosuke Yasuoka is a regular virtio-gpu contributor
(`drm_panic`, the display_info deadlock fix). Signed off by maintainer
Dmitry Osipenko.
**Step 3.5 — Dependencies**
Record: Standalone. Needs `drm_dev_enter` around queueing,
`virtio_gpu_remove`/`virtio_gpu_shutdown` calling `drm_dev_unplug`, and
`ctrlq`/`cursorq` ack waitqueues — all present at v6.18.53. No new APIs.
---
## Phase 4: Mailing list / external
**Step 4.1 — b4 dig**
Record:
- v2: https://patch.msgid.link/20260521-virtio-
gpu_wait_event-v2-1-5796b3a71d03@redhat.com
- v3 (applied): https://patch.msgid.link/20260601-virtio-
gpu_wait_event-v3-1-89530517a98a@redhat.com
- v1 used `wait_event_timeout()` (5s). v2/v3 switched to an explicit
`vqs_released` flag (no arbitrary timeout). Applied version is v3.
**Step 4.2 — Recipients (`b4 dig -w`)**
Record: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
Chia-I Wu, drm-misc maintainers, `dri-devel`,
`virtualization@lists.linux.dev`. Right people.
**Step 4.3 — Review**
Record (from saved v2 mbox):
- Dmitry: “What about other `wait_event`?” Author: those are `resp_wq`,
not inside `drm_dev_enter`, different bug. Dmitry: leave for later.
- Dmitry could not reproduce on normal systemd shutdown and noted the
syzbot stacks were not remove/shutdown. Later: *“the patch looks okay,
but I'm not convinced that it fixes the reported issue.”*
- Author agreed syzbot was a probe deadlock (`clientlist_mutex` vs
dequeue worker), dropped those tags, and described the real trigger:
host stops processing → guest waits in D-state inside `drm_dev_enter`
→ unplug’s `synchronize_srcu` never completes. systemd cannot kill
D-state tasks.
- v3 dropped the unrelated tags; Dmitry still signed off.
**Step 4.4 — Series**
Record: Single-patch series. `resp_wq` abort and interruptible waits
were explicitly deferred.
**Step 4.5 — stable@**
Record: lore.kernel.org/stable returned HTTP 403. UNVERIFIED whether it
was already discussed on stable@.
Syzbot URLs from v2: hung in `virtio_gpu_queue_fenced_ctrl_buffer`
during **probe** (`virtio_gpu_probe` → fbcon), not removal. Those are
the deadlock already closed by `89890a5fcefad` in this tree. They do
**not** prove this particular hang, and they are not used as a YES
signal for this commit.
---
## Phase 5: Semantic / call graph
**Step 5.1 — Functions**
Record: `virtio_gpu_release_vqs` (new), `virtio_gpu_remove`,
`virtio_gpu_shutdown`, `virtio_gpu_queue_ctrl_sgs`,
`virtio_gpu_queue_cursor`.
**Step 5.2 — Callers**
Record:
- `virtio_gpu_queue_ctrl_sgs` ← `virtio_gpu_queue_fenced_ctrl_buffer` ←
nearly every `virtio_gpu_cmd_*` (ioctls, plane update/flush, GEM,
etc.). Userspace and KMS both reach it.
- `virtio_gpu_queue_cursor` ← cursor plane updates.
- `virtio_gpu_remove` / `virtio_gpu_shutdown` are virtio `.remove` /
`.shutdown`.
**Step 5.3 — Callees**
Record: `drm_dev_enter`/`exit`, `wait_event`, `wake_up_all`,
`free_vbuf`, `virtio_gpu_array_unlock_resv`, `virtio_gpu_notify`. No new
helpers outside this driver.
**Step 5.4 — Reachability**
Record: Yes. Any guest GPU submit that fills the virtqueue waits here.
Unplug/shutdown is virtio remove or poweroff. Host-side virtio-gpu/virgl
stall is the trigger.
**Step 5.5 — Similar patterns**
Record: `wait_event(vgdev->resp_wq)` in `virtgpu_prime.c` and
`virtgpu_vram.c` still unbounded; not inside `drm_dev_enter`, so they do
not block unplug. Same author deferred that.
---
## Phase 6: This tree (6.18.y)
**Step 6.1 — Buggy code present?**
Record: **Yes at v6.18.53.**
```
v6.18.53:virtgpu_vq.c:398: wait_event(vgdev->ctrlq.ack_queue,
vq->num_free >= elemcnt);
v6.18.53:virtgpu_vq.c:568: wait_event(vgdev->cursorq.ack_queue,
vq->num_free >= outcnt);
```
`vqs_released` is **not** in v6.18.53. `virtio_gpu_remove`/`shutdown`
call `drm_dev_unplug()` with no prior wake.
`drm_dev_synchronize_unplug()` does `synchronize_srcu`. Bug introduced
~2020; 6.18 branched 2025-11-30.
Local tree: `git describe HEAD` → `v6.18.53-5049-g0f0bdc2b0dffb`;
Makefile `6.18.53`. Verdict is for **this 6.18.y tree only**.
**Step 6.2 — Backport difficulty**
Record: `git apply --check` of this patch onto v6.18.53 virtio files:
**clean apply**. Context differs slightly (`DRIVER_ATOMIC` guard around
`drm_atomic_helper_shutdown` from `19a6a00ff50c2`, already in v6.18.53);
insertion is still immediately before `drm_dev_unplug()`.
**Step 6.3 — Related fix already here?**
Record: Probe deadlock `89890a5fcefad` is in v6.18.53 and does **not**
abort virtqueue waits on removal. No equivalent abort exists. This is
not a duplicate.
---
## Phase 7: Subsystem
**Step 7.1**
Record: `drivers/gpu/drm/virtio` — virtio-gpu DRM. **IMPORTANT** for VMs
(QEMU/KVM, cloud, CrosVM), not core mm/VFS.
**Step 7.2**
Record: Mature driver, still getting targeted fixes. Not staging.
---
## Phase 8: Impact and risk
**Step 8.1 — Who**
Record: virtio-gpu guests (`CONFIG_DRM_VIRTIO_GPU`). Common in VMs, not
every bare-metal box.
**Step 8.2 — Trigger**
Record: Host stops completing virtqueue commands (host renderer crash,
hung device, hypervisor bug) **and** a guest command waits for ring
space **and** then remove/shutdown. Not every boot. Cannot be signal-
killed (`TASK_UNINTERRUPTIBLE`). Module unload, virtio reset, and
poweroff all hit `drm_dev_unplug()`.
**Step 8.3 — Failure mode**
Record: **Hung task / shutdown or unplug deadlock.** Severity: **HIGH**
(system cannot finish shutdown/remove; D-state). Not a UAF or silent
corruption.
**Step 8.4 — Risk vs benefit**
Record:
- Benefit: HIGH for virtio-gpu VMs that lose the host GPU and then
reboot/unplug.
- Risk: LOW — ~35 lines, teardown-only flag, abort path copies existing
`-ENODEV` cleanup.
- Ratio: favorable.
---
## Phase 9: Synthesis
**Step 9.1 — Evidence**
**For:**
- Real hang: uninterruptible wait inside `drm_dev_enter` blocks
`drm_dev_unplug()`/`synchronize_srcu`.
- Confirmed in v6.18.53 source (`wait_event` without abort; unplug with
no wake).
- Small, contained, no new API.
- Abort cleanup matches existing `drm_dev_enter()` failure.
- Maintainer signed off after review (v1 timeout → v3 flag).
- Applies cleanly to this tree’s virtio files.
- virtio-gpu is widely used; host GPU process failure is a realistic VM
event.
**Against:**
- Trigger needs a stuck host plus a full virtqueue plus teardown — not a
hot path.
- Syzbot tags were wrongly attached in v2 and correctly dropped; do not
treat this as a syzbot backport.
- Incomplete vs all `wait_event`s; `resp_wq` still unbounded
(intentionally separate).
- Maintainer initially could not reproduce on a normal systemd shutdown.
**Unresolved:** stable@ archive 403; whether this hang has a public user
report besides the author’s analysis.
**Step 9.2 — Stable rules**
1. Obviously correct and tested — **PASS** (standard wait/wake abort;
maintainer SOB; v3 after review).
2. Real user-visible bug — **PASS** (remove/shutdown hang when host
stops the vq).
3. Important (hang/deadlock) — **PASS** (HIGH: D-state blocks
unplug/shutdown).
4. Small and contained — **PASS** (~37 lines, one driver).
5. No new features/APIs — **PASS**.
6. Applies to this tree — **PASS** (clean apply to v6.18.53 virtio
files; all helpers present).
**Step 9.3 — Exceptions**
Record: None (not a device ID/quirk/DT/build/doc exception). Qualifies
as a normal hang fix.
**Step 9.4 — Decision**
This belongs on 6.18.y. The bug is in this tree, the fix is surgical,
and a stuck virtqueue otherwise makes guest unplug/shutdown hang
forever.
---
## What the commit solves for this tree
On 6.18.y, virtio-gpu waits for virtqueue space with `wait_event()`
inside `drm_dev_enter()`. If the host stops processing, that wait never
ends, `drm_dev_unplug()` never gets past `synchronize_srcu()`, and
remove/shutdown hang in D-state. The patch sets `vqs_released`, wakes
both queues before unplug, and aborts the command like a failed
`drm_dev_enter()`.
That is a real hang, not a feature. Risk is low; the only extra behavior
is on teardown.
---
## Verification
- **[Phase 1]** Parsed v3 message/tags; no Fixes/Reported-by on
committed v3; v3 changelog documents dropping unrelated syzbot tags.
- **[Phase 2]** Diff: +37/−2 across three virtio-gpu files; abort copies
`drm_dev_enter()` failure cleanup; fence emit is after the wait.
- **[Phase 3]** `git blame` on v6.18.53: wait from `6ebe8661ebf27a`
(2020) / cursor from 2013–2018; `drm_dev_enter` from `b1df3a2b24a917`
(2020).
- **[Phase 3]** `git merge-base --is-ancestor 0ffdc5889ef64 v6.18.53` →
**not** in v6.18.53; buggy wait **is** in v6.18.53.
- **[Phase 3]** Probe deadlock `89890a5fcefad` **is** in v6.18.53 and
closes the syzbot hung-task IDs; it does not abort vq waits on
removal.
- **[Phase 3]** `virtio_gpu_shutdown` (`2507789a724d6`) **is** in
v6.18.53.
- **[Phase 3]** `vgdev` allocated with `drmm_kzalloc` in `virtgpu_kms.c`
— `vqs_released` starts false.
- **[Phase 4]** `b4 dig -c a46991b334f663fa0e0e9c051e50587ef977ac8b`
matched v2/v3 lore; `b4 dig -a` showed v2 then v3; `b4 dig -w` listed
virtio-gpu maintainers.
- **[Phase 4]** Mbox: Dmitry review; author explained syzbot mismatch
and the real remove/shutdown hang; v3 dropped tags; Dmitry SOB on the
commit.
- **[Phase 4]** WebFetch syzbot bugs `d6dd6f86…` / `908bd910…`: hung
during **probe/fbcon**, not unplug — not this bug.
- **[Phase 4]** lore.kernel.org and lore.kernel.org/stable: HTTP 403.
UNVERIFIED for stable@ discussion. Used gitlab.freedesktop.org +
openwall + mbox instead.
- **[Phase 5]** `git grep`: `virtio_gpu_queue_ctrl_sgs` is on
ioctl/plane/GEM paths; dequeue `wake_up(&ctrlq.ack_queue)` only after
host responses — if host is dead, waiters never wake without this
patch.
- **[Phase 6]** `git describe HEAD` / Makefile: **6.18.53**. `git apply
--check` of this patch onto archived v6.18.53 virtio files: **APPLY
CLEAN**.
- **[Phase 6]** `drm_dev_unplug` at v6.18.53 calls
`drm_dev_synchronize_unplug()` → `synchronize_srcu`.
- **[Phase 6]** `wait_event()` in `include/linux/wait.h` uses
`TASK_UNINTERRUPTIBLE`.
- **[Phase 8]** Failure mode from code + commit: hung unplug/shutdown,
severity HIGH.
- **UNVERIFIED:** public non-syzbot user report of this exact remove
hang; stable@ archive (403).
The hang is in this 6.18.y tree, the patch is small and correct, and it
should be backported.
**YES**
drivers/gpu/drm/virtio/virtgpu_drv.c | 15 +++++++++++++++
drivers/gpu/drm/virtio/virtgpu_drv.h | 1 +
drivers/gpu/drm/virtio/virtgpu_vq.c | 23 +++++++++++++++++++++--
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index d9556e1b67b10..78cc388b434d9 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -118,10 +118,24 @@ static int virtio_gpu_probe(struct virtio_device *vdev)
return ret;
}
+/*
+ * Release pending virtqueue waits so the drm_dev_enter/exit() critical
+ * sections complete before drm_dev_unplug() blocks on synchronize_srcu().
+ */
+static void virtio_gpu_release_vqs(struct drm_device *dev)
+{
+ struct virtio_gpu_device *vgdev = dev->dev_private;
+
+ vgdev->vqs_released = true;
+ wake_up_all(&vgdev->ctrlq.ack_queue);
+ wake_up_all(&vgdev->cursorq.ack_queue);
+}
+
static void virtio_gpu_remove(struct virtio_device *vdev)
{
struct drm_device *dev = vdev->priv;
+ virtio_gpu_release_vqs(dev);
drm_dev_unplug(dev);
if (drm_core_check_feature(dev, DRIVER_ATOMIC))
@@ -135,6 +149,7 @@ static void virtio_gpu_shutdown(struct virtio_device *vdev)
{
struct drm_device *dev = vdev->priv;
+ virtio_gpu_release_vqs(dev);
/* stop talking to the device */
drm_dev_unplug(dev);
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 8575c7787fc36..51b78f8a8aa2e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -260,6 +260,7 @@ struct virtio_gpu_device {
struct virtio_gpu_queue ctrlq;
struct virtio_gpu_queue cursorq;
+ bool vqs_released;
struct kmem_cache *vbufs;
atomic_t pending_commands;
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 592ad4f9b100d..244a791ab915e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -422,7 +422,19 @@ static int virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
if (vq->num_free < elemcnt) {
spin_unlock(&vgdev->ctrlq.qlock);
virtio_gpu_notify(vgdev);
- wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= elemcnt);
+ wait_event(vgdev->ctrlq.ack_queue,
+ vq->num_free >= elemcnt || vgdev->vqs_released);
+ /*
+ * Set by virtio_gpu_release_vqs() to unblock
+ * synchronize_srcu() wait in drm_dev_unplug().
+ */
+ if (vgdev->vqs_released) {
+ if (fence && vbuf->objs)
+ virtio_gpu_array_unlock_resv(vbuf->objs);
+ free_vbuf(vgdev, vbuf);
+ drm_dev_exit(idx);
+ return -ENODEV;
+ }
goto again;
}
@@ -592,7 +604,14 @@ static void virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
ret = virtqueue_add_sgs(vq, sgs, outcnt, 0, vbuf, GFP_ATOMIC);
if (ret == -ENOSPC) {
spin_unlock(&vgdev->cursorq.qlock);
- wait_event(vgdev->cursorq.ack_queue, vq->num_free >= outcnt);
+ wait_event(vgdev->cursorq.ack_queue,
+ vq->num_free >= outcnt || vgdev->vqs_released);
+ /* See comment in virtio_gpu_queue_ctrl_sgs(). */
+ if (vgdev->vqs_released) {
+ free_vbuf(vgdev, vbuf);
+ drm_dev_exit(idx);
+ return;
+ }
spin_lock(&vgdev->cursorq.qlock);
goto retry;
} else {
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH AUTOSEL 6.18] drm/virtio: abort virtqueue wait on device removal to avoid hung task
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18] drm/virtio: abort virtqueue wait on device removal to avoid hung task Sasha Levin
@ 2026-10-19 15:24 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-19 15:24 UTC (permalink / raw)
To: Sasha Levin; +Cc: dri-devel, virtualization, Michael S. Tsirkin, Eugenio Perez
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] drm/virtio: un-emitted fence on abort in virtio_gpu_queue_ctrl_sgs()
- [Medium] drm/virtio: leak of vbuf->objs on abort in virtio_gpu_queue_ctrl_sgs()
- [Medium] drm/virtio: unreclaimed vbufs in virtio_gpu_deinit() on device removal
--
commit 81841bb63e995940f01c6fe877305de441ff0e80
Author: Ryosuke Yasuoka <ryasuoka@redhat.com>
drm/virtio: abort virtqueue wait on device removal to avoid hung task
This commit adds a vqs_released flag to struct virtio_gpu_device and wakes
up the control and cursor virtqueue wait queues during virtio_gpu_remove()
and virtio_gpu_shutdown() so that threads waiting for virtqueue space exit
their drm_dev_enter() critical sections and unblock drm_dev_unplug().
Link: https://patch.msgid.link/20260601-virtio-gpu_wait_event-v3-1-89530517a98a@redhat.com
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
> index d9556e1b67b10..78cc388b434d9 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> @@ -118,10 +118,24 @@ static int virtio_gpu_probe(struct virtio_device *vdev)
> return ret;
> }
>
> +/*
> + * Release pending virtqueue waits so the drm_dev_enter/exit() critical
> + * sections complete before drm_dev_unplug() blocks on synchronize_srcu().
> + */
> +static void virtio_gpu_release_vqs(struct drm_device *dev)
> +{
> + struct virtio_gpu_device *vgdev = dev->dev_private;
> +
> + vgdev->vqs_released = true;
> + wake_up_all(&vgdev->ctrlq.ack_queue);
> + wake_up_all(&vgdev->cursorq.ack_queue);
> +}
> +
> static void virtio_gpu_remove(struct virtio_device *vdev)
> {
> struct drm_device *dev = vdev->priv;
>
> + virtio_gpu_release_vqs(dev);
> drm_dev_unplug(dev);
>
> if (drm_core_check_feature(dev, DRIVER_ATOMIC))
[Severity: Medium]
When virtio_gpu_release_vqs() wakes a thread blocked on a full virtqueue,
the virtqueue itself still contains all of the previously queued
struct virtio_gpu_vbuffer entries that the host stopped processing.
Once drm_dev_unplug() unblocks, virtio_gpu_remove() proceeds to
virtio_gpu_deinit() and drm_dev_put() -> virtio_gpu_release():
drivers/gpu/drm/virtio/virtgpu_kms.c:virtio_gpu_deinit() {
flush_work(&vgdev->obj_free_work);
flush_work(&vgdev->ctrlq.dequeue_work);
flush_work(&vgdev->cursorq.dequeue_work);
flush_work(&vgdev->config_changed_work);
virtio_reset_device(vgdev->vdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
}
drivers/gpu/drm/virtio/virtgpu_kms.c:virtio_gpu_release() {
...
virtio_gpu_modeset_fini(vgdev);
virtio_gpu_free_vbufs(vgdev);
...
}
Because flush_work() on dequeue_work only reclaims buffers that the host
placed into the used ring via virtqueue_get_buf(), the unconsumed vbufs
left in ctrlq.vq and cursorq.vq are never detached via
virtqueue_detach_unused_buf() and freed before del_vqs() and
virtio_gpu_free_vbufs() -> kmem_cache_destroy(vgdev->vbufs).
Will this leak those in-flight vbufs (along with their resp_buf, data_buf,
and vbuf->objs references) and trigger a "Objects remaining on
__kmem_cache_shutdown()" warning in kmem_cache_destroy()?
[ ... ]
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 592ad4f9b100d..244a791ab915e 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -422,7 +422,19 @@ static int virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
> if (vq->num_free < elemcnt) {
> spin_unlock(&vgdev->ctrlq.qlock);
> virtio_gpu_notify(vgdev);
> - wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= elemcnt);
> + wait_event(vgdev->ctrlq.ack_queue,
> + vq->num_free >= elemcnt || vgdev->vqs_released);
> + /*
> + * Set by virtio_gpu_release_vqs() to unblock
> + * synchronize_srcu() wait in drm_dev_unplug().
> + */
> + if (vgdev->vqs_released) {
> + if (fence && vbuf->objs)
> + virtio_gpu_array_unlock_resv(vbuf->objs);
> + free_vbuf(vgdev, vbuf);
> + drm_dev_exit(idx);
> + return -ENODEV;
> + }
> goto again;
> }
[Severity: Medium]
Does this abort path leak vbuf->objs and the GEM object references it
holds?
Callers that attach a struct virtio_gpu_object_array to vbuf->objs (such
as virtio_gpu_cmd_submit(), virtio_gpu_cmd_transfer_to_host_2d(),
virtio_gpu_cmd_transfer_to_host_3d(), virtio_gpu_cmd_resource_flush(), and
virtio_gpu_cmd_create_resource()) transfer ownership of objs to the vbuf
and ignore the return value of virtio_gpu_queue_fenced_ctrl_buffer() and
virtio_gpu_queue_ctrl_buffer().
On normal completion in virtio_gpu_dequeue_ctrl_func(), vbuf->objs is
released before free_vbuf():
drivers/gpu/drm/virtio/virtgpu_vq.c:virtio_gpu_dequeue_ctrl_func() {
...
list_for_each_entry_safe(entry, tmp, &reclaim_list, list) {
if (entry->objs)
virtio_gpu_array_put_free_delayed(vgdev, entry->objs);
list_del(&entry->list);
free_vbuf(vgdev, entry);
}
...
}
However, free_vbuf() only frees vbuf->resp_buf, vbuf->data_buf, and vbuf:
drivers/gpu/drm/virtio/virtgpu_vq.c:free_vbuf() {
if (vbuf->resp_size > MAX_INLINE_RESP_SIZE)
kfree(vbuf->resp_buf);
kvfree(vbuf->data_buf);
kmem_cache_free(vgdev->vbufs, vbuf);
}
When vgdev->vqs_released is true, virtio_gpu_queue_ctrl_sgs() unlocks the
reservations on vbuf->objs when fenced, but never calls
virtio_gpu_array_put_free() or virtio_gpu_array_put_free_delayed() on
vbuf->objs.
[Severity: High]
What happens to callers that pass a non-NULL fence when
virtio_gpu_queue_ctrl_sgs() aborts here before calling
virtio_gpu_fence_emit()?
Because virtio_gpu_queue_fenced_ctrl_buffer() and its callers (such as
virtio_gpu_cmd_resource_detach_backing(),
virtio_gpu_cmd_transfer_to_host_2d(), and virtio_gpu_cmd_submit()) have
void return types and drop the -ENODEV error, the caller is not aware that
virtio_gpu_fence_emit() was skipped, leaving fence->f.seqno == 0 and the
fence unqueued in vgdev->fence_drv.fences.
In virtio_gpu_detach_object_fenced(), the caller immediately waits
uninterruptibly on the fence:
drivers/gpu/drm/virtio/virtgpu_object.c:virtio_gpu_detach_object_fenced() {
...
virtio_gpu_object_detach(vgdev, bo, fence);
virtio_gpu_notify(vgdev);
dma_fence_wait(&fence->f, false);
dma_fence_put(&fence->f);
}
Similarly, virtio_gpu_cursor_plane_update() calls
dma_fence_wait(&vgplane_st->fence->f, true) right after
virtio_gpu_cmd_transfer_to_host_2d(). Since the fence was never emitted
and will never be signaled, won't dma_fence_wait() in
virtio_gpu_detach_object_fenced() sleep forever in TASK_UNINTERRUPTIBLE
and cause another hung task?
In addition, in virtio_gpu_execbuffer_ioctl():
drivers/gpu/drm/virtio/virtgpu_submit.c:virtio_gpu_execbuffer_ioctl() {
...
virtio_gpu_submit(&submit);
virtio_gpu_install_out_fence_fd(&submit);
virtio_gpu_process_post_deps(&submit);
virtio_gpu_complete_submit(&submit);
cleanup:
if (ret && submit.out_fence && submit.out_fence->e) {
drm_event_cancel_free(dev, &submit.out_fence->e->base);
submit.out_fence->e = NULL;
}
virtio_gpu_cleanup_submit(&submit);
return ret;
}
Because virtio_gpu_submit() does not propagate the abort error to ret:
1. submit.out_fence (with f.seqno == 0) is exported to userspace sync_file
or syncobjs, which triggers WARN_ON_ONCE(f->seqno == 0) in
virtio_gpu_fence_signaled() when queried.
2. drm_event_cancel_free() at cleanup is skipped because ret is 0, and
because submit.out_fence is not in vgdev->fence_drv.fences,
submit.out_fence->e is never sent or freed.
3. virtio_gpu_complete_submit() clears submit.buflist = NULL, so
virtio_gpu_cleanup_submit() skips freeing submit.buflist (vbuf->objs).
[ ... ]
>
>
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-417-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH AUTOSEL 6.18-6.12] vdpa/mlx5: roll back MR update after VQ setup failure
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (8 preceding siblings ...)
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18] drm/virtio: abort virtqueue wait on device removal to avoid hung task Sasha Levin
@ 2026-10-19 13:52 ` Sasha Levin
2026-10-19 15:17 ` sashiko-bot
9 siblings, 1 reply; 20+ messages in thread
From: Sasha Levin @ 2026-10-19 13:52 UTC (permalink / raw)
To: patches, stable
Cc: Weimin Xiong, Michael S. Tsirkin, Sasha Levin, dtatulea,
jasowangio, eperezma, virtualization, linux-kernel
From: Weimin Xiong <xiongwm2026@163.com>
[ Upstream commit 11f79e2780043ad94424a8db02a7a9022ba93252 ]
mlx5_vdpa_change_map() must install the new MR before rebuilding or
resuming virtqueues, because both paths read the MR keys from
mvdev->mres.mr[].
If rebuilding the virtqueue resources fails, the new MR must not remain
installed after its reference is released. Keep an extra reference to
the old MR before replacing it. On setup failure, restore the old MR;
the saved reference then becomes the map reference, while replacing the
new MR drops its map reference.
Make mlx5_vdpa_change_map() consume new_mr on all error paths so that
set_map_data() does not release an MR already released during rollback.
v2:
- Keep the new MR installed while virtqueues are rebuilt.
- Restore the old MR only after setup_vq_resources() fails.
Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-ID: <20260716054353.155805-1-xiongwm2026@163.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Local tree is **6.12.111** (`pending-6.12`). The buggy
`mlx5_vdpa_change_map()` path is present, the patch applies cleanly, and
this is a real use-after-free on the map-update error path.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[vdpa/mlx5] [roll back] [restore the previous memory region if
virtqueue rebuild fails after installing a new MR]**
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Fixes:** none (expected for this review)
- **Reported-by / Tested-by / Reviewed-by / Acked-by:** none
- **Link:** none in the commit; Message-ID
`20260716054353.155805-1-xiongwm2026@163.com`
- **Cc: stable:** none (expected)
- **Signed-off-by:** Weimin Xiong (author); Michael S. Tsirkin (vhost
maintainer, applied the patch)
- Ignore any pipeline SOBs.
Notable: standalone v2 patch, not `N/M`. Maintainer SOB is present; no
syzbot tag.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `mlx5_vdpa_change_map()` installs `new_mr` into
`mvdev->mres.mr[]` before rebuilding virtqueues. If
`setup_vq_resources()` fails, the caller still drops `new_mr` while
that pointer remains installed.
- **Symptom:** dangling MR pointer / use-after-free after a failed live
map update. GC can free the MR ~2s later (`MLX5_VDPA_MR_GC_TRIGGER_MS`
is 2000).
- **Version info:** none in the message. Introduced by the v6.7 MR-
update flow, present in this 6.12 tree.
- **Root cause:** missing rollback of the map slot and split ownership
of `new_mr` between `change_map()` and `set_map_data()`.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not a disguised cleanup.** This is an explicit error-path
refcount/UAF fix. v1 wrongly deferred the MR install until after VQ
setup; v2 keeps the new MR installed during rebuild and rolls back only
on failure.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/vdpa/mlx5/net/mlx5_vnet.c` — **+15 / -8** (23 lines)
- Functions: `mlx5_vdpa_change_map()`, `set_map_data()`
- Scope: **single-file surgical error-path fix**
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
1. `save_channels_info()` failure: now `mlx5_vdpa_put_mr(new_mr)` inside
`change_map()` (caller no longer puts it).
2. Before `mlx5_vdpa_update_mr()`: extra `get` on `old_mr` so it is not
GC'd after the map slot is replaced.
3. `setup_vq_resources()` failure: `mlx5_vdpa_update_mr(old_mr)`
restores the old map; extra get becomes the map ref; installing
`old_mr` drops `new_mr`'s map ref.
4. Success / early `!DRIVER_OK` / suspended: `goto out` and drop the
extra old-MR ref.
5. `set_map_data()`: remove `out_err` put of `new_mr` so rollback cannot
double-free.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[UAF / dangling pointer + refcount] [map slot updated, then
`new_mr` released on error while `mres.mr[asid]` still points at it]**
Verified in this tree:
```731:743:drivers/vdpa/mlx5/core/mr.c
void mlx5_vdpa_update_mr(struct mlx5_vdpa_dev *mvdev,
struct mlx5_vdpa_mr *new_mr,
unsigned int asid)
{
struct mlx5_vdpa_mr *old_mr = mvdev->mres.mr[asid];
mutex_lock(&mvdev->mres.lock);
_mlx5_vdpa_put_mr(mvdev, old_mr);
mvdev->mres.mr[asid] = new_mr;
mutex_unlock(&mvdev->mres.lock);
}
```
`create_virtqueue()` / `modify_virtqueue` read `mvdev->mres.mr[]` for
mkeys, so the new MR **must** be installed before rebuild/resume. v1 was
incorrect; v2 matches that constraint.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** refcount flow is consistent: extra get on `old_mr`;
failure converts it into the map ref; success drops it; `new_mr` is
consumed on every `change_map()` error path.
- **Regression risk:** low. No new locks, APIs, or data structures.
`put_mr(NULL)` is a no-op. Residual issue: after a failed rebuild, VQs
stay torn down (pre-existing); this patch only keeps the MR pointer
valid.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- `mlx5_vdpa_update_mr()` before setup: **625e4b59a923d2** (“vdpa/mlx5:
Improve mr update flow”), **v6.7-rc1**.
- Teardown/rebuild via `setup_vq_resources()`: **f756dd3e2a4c70**,
**v6.8-rc1**.
- Caller `out_err` put: **625e4b59a923d2** / **a06bd11b18fdf2**.
- Function itself dates to **1a86b377aa2147** (2020).
The dangling-pointer pattern exists since the v6.7 MR-update refactor:
setup failure returns without restoring the map, and the caller puts
`new_mr`.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Buggy pattern is in this tree via 625e4b59 /
f756dd3e. `git merge-base --is-ancestor 11f79e2780043 HEAD` is false —
fix is **not** already here.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Related: **62111654481d5** “Postpone MR deletion” (**v6.12-rc1**) —
delayed GC, so a dangling MR is freed ~2s later rather than
immediately.
- No prior rollback of `change_map()` in this tree.
- Standalone; not `N/M`.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Weimin Xiong has **no other mlx5 vdpa commits in this tree**.
Subsystem maintainer is Dragos Tatulea; the patch was applied by Michael
Tsirkin. Recipients included Tatulea, Tsirkin, Jason Wang, Eugenio
Perez.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Uses existing `mlx5_vdpa_get_mr` / `put_mr` / `update_mr`. `git
apply --check` of 11f79e27 against this tree: **clean**. **Standalone.**
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 11f79e2780043`:
https://patch.msgid.link/20260716054353.155805-1-xiongwm2026@163.com
- `b4 dig -a`: **only v2** matched by patch-id
- v1 existed with a different subject/diff: “fix MR state corruption on
setup_vq_resources failure” — it moved `update_mr` **after** setup,
which would program VQs with the **old** mkey. v2 changelog documents
that correction.
- Saved mbox: **patch only, no public replies**. lore.kernel.org fetches
returned 403; no NAK found in the saved thread.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc Dragos Tatulea, Michael Tsirkin, Jason Wang,
Xuan Zhuo, Eugenio Perez, virtualization@lists.linux.dev. Applied with
Tsirkin SOB. No public Reviewed-by.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: **No syzbot, bugzilla, or user crash report.** Bug is from code
inspection of the error path. Failure mode is still verified from the
current sources.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: **Standalone v2.** No later `mlx5_vnet.c` commits on `all-next`
after 11f79e27. No follow-up reverting or replacing this fix.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable search returned 403. **UNVERIFIED**
whether stable@ already discussed this. That does not change the
technical verdict.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `mlx5_vdpa_change_map()`, `set_map_data()`
**Step 5.2: TRACE CALLERS**
Record:
- `set_map_data()` → `mlx5_vdpa_change_map()` when an ASID already has
an MR
- `mlx5_vdpa_set_map()` (vdpa `.set_map`) → `set_map_data()` under
`reslock`
- vhost-vdpa calls `.set_map` from `vhost_vdpa_map()` / unmap / batch
IOTLB updates (`drivers/vhost/vdpa.c`)
**Step 5.3: TRACE CALLEES**
Record: `suspend_vqs`, `save_channels_info`, `teardown_vq_resources`,
`mlx5_vdpa_get_mr`/`update_mr`/`put_mr`, `setup_vq_resources`,
`resume_vqs`. `setup_vq_resources()` can fail on umem, virtqueue create,
RQT/TIR, steering (firmware/alloc errors). `_mlx5_vdpa_put_mr()` queues
GC when refcount hits 0.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Userspace/QEMU vhost-vdpa IOTLB updates → `VHOST_IOTLB_*` →
`ops->set_map` → this path. **Reachable from VM memory map changes**
(boot, live migration, hotplug, vIOMMU). The UAF arm additionally needs
DRIVER_OK, non-resumable firmware (`freeze_to_rdy_supported` missing →
`.resume = NULL` → teardown/rebuild), and `setup_vq_resources()`
failure.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Other `mlx5_vdpa_update_mr()` sites (`mlx5_vdpa_clean_mrs`,
first-map in `set_map_data`, reset) do not rebuild VQs then drop the new
MR while leaving it installed. This error path is unique to
`change_map()`.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `mlx5_vdpa_change_map()` at
`drivers/vdpa/mlx5/net/mlx5_vnet.c:3044` still updates the MR then
returns on `setup_vq_resources()` failure; `set_map_data()` still `put`s
`new_mr` on that error. Bug since **v6.7**; delayed GC since
**v6.12-rc1**. Local: **6.12.111**.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply** (`git apply --check` exit 0). Surrounding code
matches the diff hunks.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: **No** rollback/`roll back MR` commit in this tree. 11f79e27 is
not an ancestor of HEAD.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **[drivers/vdpa/mlx5] [IMPORTANT]** — ConnectX6+ virtio-net
datapath offload (`CONFIG_MLX5_VDPA_NET`). Not core MM/VFS, but
production virtualization (OpenStack/K8s HW-offloaded virtio).
Maintainer: Dragos Tatulea.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature, still receiving error-path and MR-lifetime fixes
(postpone MR deletion, probe error paths). Not staging.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **driver-specific / config-specific** — `CONFIG_MLX5_VDPA_NET`
users, mainly when firmware lacks `freeze_to_rdy_supported`
(teardown/rebuild on every map change).
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Live `.set_map` while DRIVER_OK, teardown path, then VQ resource
setup fails. Not every boot; map updates are normal for vDPA VMs.
Unprivileged guest does not call `.set_map` directly; the VMM/host
process does. Still a host kernel UAF.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **[dangling `mres.mr[]` + GC free → UAF] [HIGH]** — later VQ
modify/create or another map op can dereference a freed MR. Crash or
memory corruption in the host kernel.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** high for mlx5 vDPA — prevents UAF after a failed map
change.
- **Risk:** low — ~15 lines, local refcount, applies cleanly.
- **Ratio:** **favor backport.**
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real UAF: map slot holds `new_mr` after the caller drops its last map
ref.
- Delayed GC in this 6.12 tree makes the free asynchronous and easier to
hit.
- Small, contained, obviously correct refcount rollback.
- Applies cleanly; no missing helpers.
- v2 respects that VQ setup/resume must see the new MR keys.
- Applied by the vhost maintainer.
AGAINST:
- Error-path only (setup failure after teardown).
- mlx5 vDPA only, not all 6.12 users.
- No syzbot/user report, no Fixes/Cc:stable (not a negative per the
review rules).
- No public review comments in the saved mbox.
UNRESOLVED:
- Full lore/stable@ threads (HTTP 403).
- No Tested-by from hardware.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — refcount accounting is
consistent; maintainer-applied. Hardware Tested-by absent but the
logic is verifiable from the code.
2. Fixes a real bug that affects users? **PASS** — dangling MR after
failed `.set_map`.
3. Important issue? **PASS** — UAF / host crash, HIGH.
4. Small and contained? **PASS** — one file, +15/-8.
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — buggy code present; clean
apply.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **Not an exception category** (not an ID/quirk/DT/build/doc
add). It qualifies as a normal UAF/refcount bug fix.
**Step 9.4: MAKE YOUR DECISION**
This tree has the bug. The patch is a small, correct rollback that
prevents a host UAF on a reachable vDPA map-update error path. Backport
it.
===============================================================
What the commit solves, and why it matters here
===============================================================
On 6.12, a live mlx5 vDPA map change with non-resumable firmware tears
down virtqueues, installs a new MR, then rebuilds queues. If rebuild
fails, `set_map_data()` still `put`s that new MR while `mres.mr[asid]`
points at it. The MR GC workqueue can free it two seconds later. Later
mkey reads are a use-after-free in the host kernel.
The fix keeps an extra ref on the old MR, installs the new one for the
rebuild (required, because create/modify read `mres.mr[]`), and on setup
failure restores the old MR so the extra ref becomes the map ref.
`change_map()` owns `new_mr` on all error paths, so the caller does not
double-put.
Risk vs benefit: low risk, high severity. No prerequisites missing in
6.12.111.
===============================================================
Verification
===============================================================
- [Phase 1] Parsed tags: SOB Weimin Xiong + Michael Tsirkin; Message-ID
present; no Fixes/Reported-by/Cc:stable
- [Phase 2] Diff: +15/-8 in `mlx5_vnet.c`; extra get on old MR; rollback
via `update_mr`; move `new_mr` put into `change_map()`
- [Phase 2] `mlx5_vdpa_update_mr()` puts old and assigns new without get
on new (mr.c:731)
- [Phase 2] `mlx5_vdpa_create_mr()` starts refcount at 1 (mr.c:832)
- [Phase 2] create_virtqueue reads `mvdev->mres.mr[]` for mkeys
(mlx5_vnet.c:944)
- [Phase 3] blame: update-before-setup from 625e4b59 (v6.7-rc1);
teardown/rebuild from f756dd3e (v6.8-rc1)
- [Phase 3] `git merge-base --is-ancestor 11f79e2780043 HEAD` → not in
this tree
- [Phase 3] Postpone-MR-deletion 62111654481d5 is in this tree
(v6.12-rc1); GC delay 2000ms (mr.c:664)
- [Phase 4] `b4 dig -c 11f79e27` →
patch.msgid.link/20260716054353.155805-1-xiongwm2026@163.com
- [Phase 4] `b4 dig -a`: only v2; v1 was a different (incorrect) patch-
id
- [Phase 4] `b4 dig -w`: Tatulea, Tsirkin, Wang, Perez CC'd
- [Phase 4] mbox: submission only, no replies; lore HTTP 403
- [Phase 5] callers: `mlx5_vdpa_set_map` ← vhost-vdpa `.set_map`
- [Phase 5] teardown path when `.resume` is NULL
(`freeze_to_rdy_supported` missing, mlx5_vnet.c:4107)
- [Phase 6] `git describe HEAD` → p-6.12-3-gc504cf0baa3d8; `make
kernelversion` → 6.12.111
- [Phase 6] current `mlx5_vdpa_change_map()` matches the pre-patch code;
`git apply --check` clean
- [Phase 8] Failure mode: dangling MR → GC → UAF, severity HIGH
- UNVERIFIED: stable@ mailing-list discussion (lore 403); no hardware
Tested-by
**YES**
drivers/vdpa/mlx5/net/mlx5_vnet.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 44062e9d68f00..c49167d760a62 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -3044,18 +3044,24 @@ static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev,
unsigned int asid)
{
struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
+ struct mlx5_vdpa_mr *old_mr;
bool teardown = !is_resumable(ndev);
int err;
suspend_vqs(ndev, 0, ndev->cur_num_vqs);
if (teardown) {
err = save_channels_info(ndev);
- if (err)
+ if (err) {
+ mlx5_vdpa_put_mr(mvdev, new_mr);
return err;
+ }
teardown_vq_resources(ndev);
}
+ /* Keep the old MR alive in case rebuilding the VQs fails. */
+ old_mr = mvdev->mres.mr[asid];
+ mlx5_vdpa_get_mr(mvdev, old_mr);
mlx5_vdpa_update_mr(mvdev, new_mr, asid);
for (int i = 0; i < mvdev->max_vqs; i++)
@@ -3063,17 +3069,22 @@ static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev,
MLX5_VIRTQ_MODIFY_MASK_DESC_GROUP_MKEY;
if (!(mvdev->status & VIRTIO_CONFIG_S_DRIVER_OK) || mvdev->suspended)
- return 0;
+ goto out;
if (teardown) {
restore_channels_info(ndev);
err = setup_vq_resources(ndev, true);
- if (err)
+ if (err) {
+ /* The saved reference becomes the restored map reference. */
+ mlx5_vdpa_update_mr(mvdev, old_mr, asid);
return err;
+ }
}
resume_vqs(ndev, 0, ndev->cur_num_vqs);
+out:
+ mlx5_vdpa_put_mr(mvdev, old_mr);
return 0;
}
@@ -3357,15 +3368,11 @@ static int set_map_data(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb,
err = mlx5_vdpa_change_map(mvdev, new_mr, asid);
if (err) {
mlx5_vdpa_err(mvdev, "change map failed(%d)\n", err);
- goto out_err;
+ return err;
}
}
return mlx5_vdpa_update_cvq_iotlb(mvdev, iotlb, asid);
-
-out_err:
- mlx5_vdpa_put_mr(mvdev, new_mr);
- return err;
}
static int mlx5_vdpa_set_map(struct vdpa_device *vdev, unsigned int asid,
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH AUTOSEL 6.18-6.12] vdpa/mlx5: roll back MR update after VQ setup failure
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-6.12] vdpa/mlx5: roll back MR update after VQ setup failure Sasha Levin
@ 2026-10-19 15:17 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-19 15:17 UTC (permalink / raw)
To: Sasha Levin; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez
> From: Weimin Xiong <xiongwm2026@163.com>
>
> [ Upstream commit 11f79e2780043ad94424a8db02a7a9022ba93252 ]
>
> mlx5_vdpa_change_map() must install the new MR before rebuilding or
> resuming virtqueues, because both paths read the MR keys from
> mvdev->mres.mr[].
>
> If rebuilding the virtqueue resources fails, the new MR must not remain
> installed after its reference is released. Keep an extra reference to
> the old MR before replacing it. On setup failure, restore the old MR;
> the saved reference then becomes the map reference, while replacing the
> new MR drops its map reference.
>
> Make mlx5_vdpa_change_map() consume new_mr on all error paths so that
> set_map_data() does not release an MR already released during rollback.
>
> v2:
> - Keep the new MR installed while virtqueues are rebuilt.
> - Restore the old MR only after setup_vq_resources() fails.
>
> Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> Message-ID: <20260716054353.155805-1-xiongwm2026@163.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-442-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread