Skip to content

Fix potential tear-down bug - #68

Open
lsgunth wants to merge 1 commit into
SamsungDS:mainfrom
Eideticom:fix-queue-delete-ordering
Open

Fix potential tear-down bug#68
lsgunth wants to merge 1 commit into
SamsungDS:mainfrom
Eideticom:fix-queue-delete-ordering

Conversation

@lsgunth

@lsgunth lsgunth commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

We've been using Claude a bit on this code base and it found this real, though somewhat niche bug: if a device is torn down while there is still work in the queue the hardware may write to memory that has already been freed.

In most cases I expect the process will soon be excited when this happens so there's likely not much harm, but I thought it was worth sending Claude's solution for it so the code is correct here.

nvme_delete_iosq()/nvme_delete_iocq() freed a queue's host-side
resources (with nvme_discard_sq()/nvme_discard_cq()) before sending
the NVMe command that actually asks the device to delete it. A command
still outstanding on the queue at that point has its request tracker
freed before the device has actually aborted it, leaving any caller
holding a `struct nvme_rq *` for that queue with a dangling pointer.

Deleting a queue aborts whatever is still outstanding on it, so send the
delete command first and only discard its resources once the device
confirms it's gone. nvme_delete_ioqpair() already deletes the submission
queue before the completion queue; this changes how each individual
delete interleaves with its own cleanup, not that ordering.

If the delete command itself fails, that confirmation never happened --
the queue and whatever was outstanding on it might still be alive -- so
skip the discard rather than free resources the device may still be
using and leave the queue leaked instead. Every caller already treats a
failed delete as fatal, so the leak only lives as long as the process
does.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Logan Gunthorpe <logan.gunthorpe@eideticom.com>
iomartin added a commit to Eideticom/libvfn-pub that referenced this pull request Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant