Re: [PATCH] nvme: reject completions for requests that are not in flight

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] nvme: unratelimited dev_err() in nvme_find_rq() on completion irq path
--

commit 2696cd6e7dfab32aa750573cdd163f831092e123
Author: Chao Shi <coshi036@gmail.com>

nvme: reject completions for requests that are not in flight

This commit checks that a request looked up in nvme_find_rq() is in the
MQ_RQ_IN_FLIGHT state before returning it for completion, avoiding a NULL
pointer dereference on rq->mq_hctx when a spurious completion matches an
idle request.

Link: https://lore.kernel.org/linux-nvme/20260522153034.2168862-1-coshi036@gmail.com/
Link: https://lore.kernel.org/linux-nvme/20260715005105.1115958-1-coshi036@gmail.com/

> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 2d8f681079e85..3ba0666da7be6 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -708,6 +708,11 @@ static inline struct request *nvme_find_rq(struct blk_mq_tags *tags,
>  			tag, genctr, nvme_genctr_mask(nvme_req(rq)->genctr));
>  		return NULL;
>  	}
> +	if (unlikely(blk_mq_rq_state(rq) != MQ_RQ_IN_FLIGHT)) {
> +		dev_err(nvme_req(rq)->ctrl->device,
> +			"completion for request %#x not in flight\n", tag);
> +		return NULL;
> +	}

[Severity: Medium]
Should this use dev_err_ratelimited() instead of dev_err()?

Commit c05c86681170 ("nvme: ratelimit the completion-path messages driven
by device data") switched the existing error messages in nvme_find_rq() and
nvme_handle_cqe() to ratelimited variants because device-supplied command
IDs are processed directly in interrupt context via
nvme_irq()->nvme_poll_cq()->nvme_handle_cqe()->nvme_find_rq() (as well as
in the apple, rdma, tcp, and loop completion handlers):

drivers/nvme/host/nvme.h:nvme_find_rq() {
	...
	rq = blk_mq_tag_to_rq(tags, tag);
	if (unlikely(!rq)) {
		pr_err_ratelimited("could not locate request for tag %#x\n",
				   tag);
		return NULL;
	}
	if (unlikely(nvme_genctr_mask(nvme_req(rq)->genctr) != genctr)) {
		dev_err_ratelimited(nvme_req(rq)->ctrl->device,
			"request %#x genctr mismatch (got %#x expected %#x)\n",
			tag, genctr, nvme_genctr_mask(nvme_req(rq)->genctr));
		return NULL;
	}
}

Because nvme_find_rq() returns NULL before nvme_try_complete_req()
increments nvme_req(rq)->genctr, a malfunctioning controller or remote
Fabrics target sending repeated completions for an idle request with a
matching tag and generation counter will hit this path on every completion
and flood the kernel log from interrupt context.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-nvme-double-cmpl-uaf-blk-mq-v1-1-b10fea474423@igalia.com?part=1