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

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

[Mauricio Faria de Oliveira]

From: Chao Shi <coshi036@gmail.com>

There are a few kernel crash reports from the Steam Deck where incorrect
completions received by the nvme driver have a command id/tag associated
with a freed request (rq->mq_hctx = NULL), dereferencing a NULL pointer.

  nvme_handle_cqe()
  -> nvme_try_complete_req(req, ...)
     -> blk_mq_complete_request_remote(req)
        -> if ((rq->mq_hctx->nr_ctx == 1 &&   /* Oops! */

Although the usual suspect is the nvme device firmware, one report shows
incorrect completions also received by the ath11k wireless driver at the
same time, which suggests the underlying issue might be something else.

  [ 2585.223155] ath11k_pci 0000:03:00.0: tx completion for unknown msdu_id 0
  [ 2585.223163] ath11k_pci 0000:03:00.0: tx completion for unknown msdu_id 0
  ...
  [ 2585.228077] nvme nvme0: request 0xec genctr mismatch (got 0x2 expected 0xa)
  [ 2585.228079] nvme nvme0: invalid id 8428 completed on queue 3
  [ 2585.228205] nvme nvme0: request 0xe2 genctr mismatch (got 0xe expected 0x2)
  [ 2585.228208] nvme nvme0: invalid id 57570 completed on queue 3
  ...
  [ 2585.228726] BUG: kernel NULL pointer dereference, address: 00000000000000fe
  ...
  [ 2585.228752] RIP: 0010:blk_mq_complete_request_remote+0x1c/0x160
  ...
  [ 2585.228787] Call Trace:
  [ 2585.228790]  <IRQ>
  [ 2585.228828]  nvme_poll_cq+0x14b/0x300
  [ 2585.228838]  nvme_irq+0x41/0x80
  [ 2585.228845]  __handle_irq_event_percpu+0x4a/0x180
  [ 2585.228850]  handle_irq_event+0x38/0x90
  [ 2585.228853]  handle_edge_irq+0x8b/0x230
  [ 2585.228858]  __common_interrupt+0x3e/0x90
  [ 2585.228862]  common_interrupt+0x80/0xa0
  [ 2585.228866]  </IRQ>
  [ 2585.228868]  <TASK>
  ...

However, even when the nvme device firmware is at fault and need an update,
it is interesting for the kernel not to crash, so that it is even possible
to run the firmware update where the issue happens frequently or early on,
and while waiting for the update to be made available by the device vendor.

The fix is to check that a request is actually in-flight before completing
it. This has been previously reported and proposed by Chao Shi, found with
fuzzing, but without further feedback since July. I happened to learn of it
only after root-cause analysis and reaching a similar conclusion, checking
for prior work/feedback, thus keeping them as Author/Signed-off-by, adding
myself as Co-developed-by for the commit message and root cause analysis.

In Chao Shi's patch, there's been feedback/question of whether this should
be done in blk-mq, as this is not specific to nvme. I'd say no, at first:
- Some blk-mq calls for completion have no return value (e.g., completion
  on other CPUs), which suggests maybe an API change and updates to many
  callers, all for an edge case.
- An alternative available currently might be blk_mq_ops.complete(), but
  that is handled in the driver anyway, with risk of hitting an error in
  blk-mq (like this) as the code changes, and seems semantically wrong,
  i.e., completing a request that should not have been completed.
- Keeping in mind a small fix for stable, for now, should this be merged.
- But happy to take suggestions if blk-mq is indeed the way to go.

This has been tested with a synthetic reproducer produced by an LLM.

Originally-by: Chao Shi <coshi036@gmail.com>
Signed-off-by: Chao Shi <coshi036@gmail.com>
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/
Co-developed-by: Mauricio Faria de Oliveira <mfo@igalia.com>
Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
---
 drivers/nvme/host/nvme.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 2d8f681079e85601a92624d196e77f1ba3ace34a..3ba0666da7be6a3de2e5f0fd50155af115b4e1f6 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;
+	}
 	return rq;
 }
 

---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261102-nvme-double-cmpl-uaf-blk-mq-e3d28f74cdaf

Best regards,
-- 
Mauricio Faria de Oliveira <mfo@igalia.com>

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

[K]

Thanks, pls let me take a look, it looks like all AI generated. May I
know what kind of test you run or LLM run?

Regards,
Chao

On Tue, Nov 3, 2026 at 2:17 PM Mauricio Faria de Oliveira
<mfo@igalia.com> wrote:
>
> From: Chao Shi <coshi036@gmail.com>
>
> There are a few kernel crash reports from the Steam Deck where incorrect
> completions received by the nvme driver have a command id/tag associated
> with a freed request (rq->mq_hctx = NULL), dereferencing a NULL pointer.
>
>   nvme_handle_cqe()
>   -> nvme_try_complete_req(req, ...)
>      -> blk_mq_complete_request_remote(req)
>         -> if ((rq->mq_hctx->nr_ctx == 1 &&   /* Oops! */
>
> Although the usual suspect is the nvme device firmware, one report shows
> incorrect completions also received by the ath11k wireless driver at the
> same time, which suggests the underlying issue might be something else.
>
>   [ 2585.223155] ath11k_pci 0000:03:00.0: tx completion for unknown msdu_id 0
>   [ 2585.223163] ath11k_pci 0000:03:00.0: tx completion for unknown msdu_id 0
>   ...
>   [ 2585.228077] nvme nvme0: request 0xec genctr mismatch (got 0x2 expected 0xa)
>   [ 2585.228079] nvme nvme0: invalid id 8428 completed on queue 3
>   [ 2585.228205] nvme nvme0: request 0xe2 genctr mismatch (got 0xe expected 0x2)
>   [ 2585.228208] nvme nvme0: invalid id 57570 completed on queue 3
>   ...
>   [ 2585.228726] BUG: kernel NULL pointer dereference, address: 00000000000000fe
>   ...
>   [ 2585.228752] RIP: 0010:blk_mq_complete_request_remote+0x1c/0x160
>   ...
>   [ 2585.228787] Call Trace:
>   [ 2585.228790]  <IRQ>
>   [ 2585.228828]  nvme_poll_cq+0x14b/0x300
>   [ 2585.228838]  nvme_irq+0x41/0x80
>   [ 2585.228845]  __handle_irq_event_percpu+0x4a/0x180
>   [ 2585.228850]  handle_irq_event+0x38/0x90
>   [ 2585.228853]  handle_edge_irq+0x8b/0x230
>   [ 2585.228858]  __common_interrupt+0x3e/0x90
>   [ 2585.228862]  common_interrupt+0x80/0xa0
>   [ 2585.228866]  </IRQ>
>   [ 2585.228868]  <TASK>
>   ...
>
> However, even when the nvme device firmware is at fault and need an update,
> it is interesting for the kernel not to crash, so that it is even possible
> to run the firmware update where the issue happens frequently or early on,
> and while waiting for the update to be made available by the device vendor.
>
> The fix is to check that a request is actually in-flight before completing
> it. This has been previously reported and proposed by Chao Shi, found with
> fuzzing, but without further feedback since July. I happened to learn of it
> only after root-cause analysis and reaching a similar conclusion, checking
> for prior work/feedback, thus keeping them as Author/Signed-off-by, adding
> myself as Co-developed-by for the commit message and root cause analysis.
>
> In Chao Shi's patch, there's been feedback/question of whether this should
> be done in blk-mq, as this is not specific to nvme. I'd say no, at first:
> - Some blk-mq calls for completion have no return value (e.g., completion
>   on other CPUs), which suggests maybe an API change and updates to many
>   callers, all for an edge case.
> - An alternative available currently might be blk_mq_ops.complete(), but
>   that is handled in the driver anyway, with risk of hitting an error in
>   blk-mq (like this) as the code changes, and seems semantically wrong,
>   i.e., completing a request that should not have been completed.
> - Keeping in mind a small fix for stable, for now, should this be merged.
> - But happy to take suggestions if blk-mq is indeed the way to go.
>
> This has been tested with a synthetic reproducer produced by an LLM.
>
> Originally-by: Chao Shi <coshi036@gmail.com>
> Signed-off-by: Chao Shi <coshi036@gmail.com>
> 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/
> Co-developed-by: Mauricio Faria de Oliveira <mfo@igalia.com>
> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
> ---
>  drivers/nvme/host/nvme.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 2d8f681079e85601a92624d196e77f1ba3ace34a..3ba0666da7be6a3de2e5f0fd50155af115b4e1f6 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;
> +       }
>         return rq;
>  }
>
>
> ---
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> change-id: 20261102-nvme-double-cmpl-uaf-blk-mq-e3d28f74cdaf
>
> Best regards,
> --
> Mauricio Faria de Oliveira <mfo@igalia.com>
>

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

[Mauricio Faria de Oliveira]

On 2026-11-03 16:21, K wrote:
> Thanks, pls let me take a look, it looks like all AI generated. May I

AI has not been used for analysis or the commit message; just the
reproducer, as mentioned.
Please check git log for previous contribution history.

If you're not comfortable with the commit message, I can certainly
change the authorship.
Please just let me know your preference (e.g., Originally-by,
Co-developed-by, name in the commit message, or none).

> know what kind of test you run or LLM run?

Sure. It's a synthetic reproducer that runs the completion handler
again.

Code: https://termbin.com/40tl

In a VM with 1 vCPU, (so nvme_handle_cqe() -> nvme_try_complete_req() ->
blk_mq_complete_request_remote() returns false),
echo mq-deadline >/sys/block/nvme0n1/queue/scheduler # (so
nvme_handle_cqe() -> blk_mq_add_to_batch() also returns false),
  # with both returning false, nvme_handle_cqe() ->
nvme_pci_complete_rq() is taken, reaching __blk_mq_free_request(), which
does rq->mq_hctx = NULL
echo 1 >/sys/module/nvme/parameters/repro_arm # prepare reproducer
dd if=/dev/nvme0n1 of=/dev/null bs=4k count=1 iflag=direct # oops

cheers,

> 
> Regards,
> Chao
> 
> On Tue, Nov 3, 2026 at 2:17 PM Mauricio Faria de Oliveira
> <mfo@igalia.com> wrote:
>>
>> From: Chao Shi <coshi036@gmail.com>
>>
>> There are a few kernel crash reports from the Steam Deck where incorrect
>> completions received by the nvme driver have a command id/tag associated
>> with a freed request (rq->mq_hctx = NULL), dereferencing a NULL pointer.
>>
>>   nvme_handle_cqe()
>>   -> nvme_try_complete_req(req, ...)
>>      -> blk_mq_complete_request_remote(req)
>>         -> if ((rq->mq_hctx->nr_ctx == 1 &&   /* Oops! */
>>
>> Although the usual suspect is the nvme device firmware, one report shows
>> incorrect completions also received by the ath11k wireless driver at the
>> same time, which suggests the underlying issue might be something else.
>>
>>   [ 2585.223155] ath11k_pci 0000:03:00.0: tx completion for unknown msdu_id 0
>>   [ 2585.223163] ath11k_pci 0000:03:00.0: tx completion for unknown msdu_id 0
>>   ...
>>   [ 2585.228077] nvme nvme0: request 0xec genctr mismatch (got 0x2 expected 0xa)
>>   [ 2585.228079] nvme nvme0: invalid id 8428 completed on queue 3
>>   [ 2585.228205] nvme nvme0: request 0xe2 genctr mismatch (got 0xe expected 0x2)
>>   [ 2585.228208] nvme nvme0: invalid id 57570 completed on queue 3
>>   ...
>>   [ 2585.228726] BUG: kernel NULL pointer dereference, address: 00000000000000fe
>>   ...
>>   [ 2585.228752] RIP: 0010:blk_mq_complete_request_remote+0x1c/0x160
>>   ...
>>   [ 2585.228787] Call Trace:
>>   [ 2585.228790]  <IRQ>
>>   [ 2585.228828]  nvme_poll_cq+0x14b/0x300
>>   [ 2585.228838]  nvme_irq+0x41/0x80
>>   [ 2585.228845]  __handle_irq_event_percpu+0x4a/0x180
>>   [ 2585.228850]  handle_irq_event+0x38/0x90
>>   [ 2585.228853]  handle_edge_irq+0x8b/0x230
>>   [ 2585.228858]  __common_interrupt+0x3e/0x90
>>   [ 2585.228862]  common_interrupt+0x80/0xa0
>>   [ 2585.228866]  </IRQ>
>>   [ 2585.228868]  <TASK>
>>   ...
>>
>> However, even when the nvme device firmware is at fault and need an update,
>> it is interesting for the kernel not to crash, so that it is even possible
>> to run the firmware update where the issue happens frequently or early on,
>> and while waiting for the update to be made available by the device vendor.
>>
>> The fix is to check that a request is actually in-flight before completing
>> it. This has been previously reported and proposed by Chao Shi, found with
>> fuzzing, but without further feedback since July. I happened to learn of it
>> only after root-cause analysis and reaching a similar conclusion, checking
>> for prior work/feedback, thus keeping them as Author/Signed-off-by, adding
>> myself as Co-developed-by for the commit message and root cause analysis.
>>
>> In Chao Shi's patch, there's been feedback/question of whether this should
>> be done in blk-mq, as this is not specific to nvme. I'd say no, at first:
>> - Some blk-mq calls for completion have no return value (e.g., completion
>>   on other CPUs), which suggests maybe an API change and updates to many
>>   callers, all for an edge case.
>> - An alternative available currently might be blk_mq_ops.complete(), but
>>   that is handled in the driver anyway, with risk of hitting an error in
>>   blk-mq (like this) as the code changes, and seems semantically wrong,
>>   i.e., completing a request that should not have been completed.
>> - Keeping in mind a small fix for stable, for now, should this be merged.
>> - But happy to take suggestions if blk-mq is indeed the way to go.
>>
>> This has been tested with a synthetic reproducer produced by an LLM.
>>
>> Originally-by: Chao Shi <coshi036@gmail.com>
>> Signed-off-by: Chao Shi <coshi036@gmail.com>
>> 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/
>> Co-developed-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>> Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>> ---
>>  drivers/nvme/host/nvme.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>> index 2d8f681079e85601a92624d196e77f1ba3ace34a..3ba0666da7be6a3de2e5f0fd50155af115b4e1f6 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;
>> +       }
>>         return rq;
>>  }
>>
>>
>> ---
>> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
>> change-id: 20261102-nvme-double-cmpl-uaf-blk-mq-e3d28f74cdaf
>>
>> Best regards,
>> --
>> Mauricio Faria de Oliveira <mfo@igalia.com>
>>

-- 
Mauricio

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

[Keith Busch]

On Tue, Nov 03, 2026 at 04:18:25PM -0300, Mauricio Faria de Oliveira wrote:
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 2d8f681079e85601a92624d196e77f1ba3ace34a..3ba0666da7be6a3de2e5f0fd50155af115b4e1f6 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;
> +	}
>  	return rq;
>  }

Is there something special about the NVMe protocol that makes this
problem unique to it? If not, why doesn't this check go into the generic
block layer instead?

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

[Jens Axboe]

On 11/3/26 12:43 PM, Keith Busch wrote:
> On Tue, Nov 03, 2026 at 04:18:25PM -0300, Mauricio Faria de Oliveira wrote:
>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>> index 2d8f681079e85601a92624d196e77f1ba3ace34a..3ba0666da7be6a3de2e5f0fd50155af115b4e1f6 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;
>> +	}
>>  	return rq;
>>  }
> 
> Is there something special about the NVMe protocol that makes this
> problem unique to it? If not, why doesn't this check go into the generic
> block layer instead?

Pretty sure I've nak'ed this kind of thing before. So no, it
should not go into the block layer and bloat it there. If a driver
does double completions it's... a driver bug.

Bigger question is "why does this patch exist in the first place"?

-- 
Jens Axboe

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

[Mauricio Faria de Oliveira]

On 2026-11-03 16:43, Keith Busch wrote:
> On Tue, Nov 03, 2026 at 04:18:25PM -0300, Mauricio Faria de Oliveira wrote:
>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>> index 2d8f681079e85601a92624d196e77f1ba3ace34a..3ba0666da7be6a3de2e5f0fd50155af115b4e1f6 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;
>> +	}
>>  	return rq;
>>  }
> 
> Is there something special about the NVMe protocol that makes this
> problem unique to it? If not, why doesn't this check go into the generic
> block layer instead?

Thanks for taking a look.

I noticed you asked the latter question in the previous
submission/review (Link: trailers), and addressed it in the commit
message; pasting it here for reference:

In Chao Shi's patch, there's been feedback/question of whether this
should
be done in blk-mq, as this is not specific to nvme. I'd say no, at
first:
- Some blk-mq calls for completion have no return value (e.g.,
completion
  on other CPUs), which suggests maybe an API change and updates to many
  callers, all for an edge case.
- An alternative available currently might be blk_mq_ops.complete(), but
  that is handled in the driver anyway, with risk of hitting an error in
  blk-mq (like this) as the code changes, and seems semantically wrong,
  i.e., completing a request that should not have been completed.
- Keeping in mind a small fix for stable, for now, should this be
merged.
- But happy to take suggestions if blk-mq is indeed the way to go.

For the former question, I'd imagine other block drivers might be
subject to a similar issue, under incorrect completions, and can take a
look at this later. I'd appreciate feedback on the points above before
trying to address it generically in the block layer, if at all possible.

cheers,

-- 
Mauricio

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

[Jens Axboe]

On 11/3/26 12:54 PM, Mauricio Faria de Oliveira wrote:
> On 2026-11-03 16:43, Keith Busch wrote:
>> On Tue, Nov 03, 2026 at 04:18:25PM -0300, Mauricio Faria de Oliveira wrote:
>>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>>> index 2d8f681079e85601a92624d196e77f1ba3ace34a..3ba0666da7be6a3de2e5f0fd50155af115b4e1f6 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;
>>> +	}
>>>  	return rq;
>>>  }
>>
>> Is there something special about the NVMe protocol that makes this
>> problem unique to it? If not, why doesn't this check go into the generic
>> block layer instead?
> 
> Thanks for taking a look.
> 
> I noticed you asked the latter question in the previous
> submission/review (Link: trailers), and addressed it in the commit
> message; pasting it here for reference:
> 
> In Chao Shi's patch, there's been feedback/question of whether this
> should
> be done in blk-mq, as this is not specific to nvme. I'd say no, at
> first:
> - Some blk-mq calls for completion have no return value (e.g.,
> completion
>   on other CPUs), which suggests maybe an API change and updates to many
>   callers, all for an edge case.
> - An alternative available currently might be blk_mq_ops.complete(), but
>   that is handled in the driver anyway, with risk of hitting an error in
>   blk-mq (like this) as the code changes, and seems semantically wrong,
>   i.e., completing a request that should not have been completed.
> - Keeping in mind a small fix for stable, for now, should this be
> merged.
> - But happy to take suggestions if blk-mq is indeed the way to go.
> 
> For the former question, I'd imagine other block drivers might be
> subject to a similar issue, under incorrect completions, and can take a
> look at this later. I'd appreciate feedback on the points above before
> trying to address it generically in the block layer, if at all possible.

This is either a protocol problem, in which case the driver should
handle it. Or it's not a problem at all in the first place, and
this is some kind of misguided defensive programming. In which case
the patch should go away.

If this is possible because of a poor API in the kernel, then we
fix the poor API. We don't add silly work-arounds.

-- 
Jens Axboe

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

[Keith Busch]

On Tue, Nov 03, 2026 at 04:54:27PM -0300, Mauricio Faria de Oliveira wrote:
> For the former question, I'd imagine other block drivers might be
> subject to a similar issue, under incorrect completions, and can take a
> look at this later. I'd appreciate feedback on the points above before
> trying to address it generically in the block layer, if at all possible.

Here's my real concern: the proposal is a race. Another CPU can change
the request state to in-flight, and this code will declare it complete
before the hardware even saw it. It's a silent data corruption problem
that we can't fix because it is impossible to tell the difference
between a bogus and a legit completion at that point. This kind of
hardware shouldn't be allowed to exist as it can't be used successfully
when it behaves this way.

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

[Chao S]

Hi Keith, Christoph, Jens,

Apologies for the slow reply. I've had several patches and crash analyses
from the same fuzzing effort in flight at once, along with writing the
work up against a paper deadline, and this thread slipped. That's on me;
I'll be prompt on follow-ups from here.

On the framing -- you're right, and I'll drop the "malicious device"
wording. e7006de6c238 was written for a malfunctioning controller; I've
reworded it to match, with no threat-model claim.

On whether this belongs in blk-mq rather than nvme -- I've kept the check
in nvme_find_rq() for now, since that is where a device-supplied command
id becomes a request. It isn't unique to nvme, though; if you'd rather the
check live in the blk-mq completion path, I'm happy to write it there.

Sending v2 with the reworded message shortly.

Thanks for the reviews,
Chao

On Wed, May 27, 2026 at 11:02 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 5/27/26 8:19 AM, Christoph Hellwig wrote:
> > On Fri, May 22, 2026 at 11:30:34AM -0400, Chao Shi wrote:
> >> nvme_find_rq() resolves a device-supplied command id to a request with
> >> blk_mq_tag_to_rq(), which returns whatever request last used that tag -
> >> possibly one that is no longer in flight (freed, or never dispatched and
> >> thus with a NULL rq->mq_hctx).  Commit e7006de6c238 ("nvme: code
> >> command_id with a genctr for use-after-free validation") guards against
> >> this, but its generation counter is only 4 bits wide and can be matched
> >> by a malfunctioning or malicious device replaying command ids.  The
> >> driver then completes a request that is not outstanding, dereferencing a
> >> NULL rq->mq_hctx or double-completing a command:
> >
> > I don't think an intentionally malicious device is part of the threat
> > model here.  This was added to protect against buggy devices.
>
> Malicious devices are explicitly NOT part of the linux threat model. If
> this is a real device, I'd say go talk to whomever made it and get the
> firmware fixed. If this is a "hardening" effort to protect against the
> threat of malicious devices, then I don't think we should bother.
>
> >> +     * blk_mq_tag_to_rq() returns whatever request last used this tag, which
> >> +     * may no longer be in flight if the device reports a bogus command id.
> >> +     * Completing it would deref a NULL rq->mq_hctx or double-complete a
> >> +     * command; the 4-bit genctr below only narrows the window.
> >> +     */
> >> +    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;
> >> +    }
> >
> > Although this check looks cheap enough that it should not hurt to add
> > it.  So I think this should be ok, but maybe respin with your planned
> > commit message update.
>
> Only for the right reasons, imho.
>
> --
> Jens Axboe

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

[Jens Axboe]

On 5/27/26 8:19 AM, Christoph Hellwig wrote:
> On Fri, May 22, 2026 at 11:30:34AM -0400, Chao Shi wrote:
>> nvme_find_rq() resolves a device-supplied command id to a request with
>> blk_mq_tag_to_rq(), which returns whatever request last used that tag -
>> possibly one that is no longer in flight (freed, or never dispatched and
>> thus with a NULL rq->mq_hctx).  Commit e7006de6c238 ("nvme: code
>> command_id with a genctr for use-after-free validation") guards against
>> this, but its generation counter is only 4 bits wide and can be matched
>> by a malfunctioning or malicious device replaying command ids.  The
>> driver then completes a request that is not outstanding, dereferencing a
>> NULL rq->mq_hctx or double-completing a command:
> 
> I don't think an intentionally malicious device is part of the threat
> model here.  This was added to protect against buggy devices.

Malicious devices are explicitly NOT part of the linux threat model. If
this is a real device, I'd say go talk to whomever made it and get the
firmware fixed. If this is a "hardening" effort to protect against the
threat of malicious devices, then I don't think we should bother.

>> +	 * blk_mq_tag_to_rq() returns whatever request last used this tag, which
>> +	 * may no longer be in flight if the device reports a bogus command id.
>> +	 * Completing it would deref a NULL rq->mq_hctx or double-complete a
>> +	 * command; the 4-bit genctr below only narrows the window.
>> +	 */
>> +	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;
>> +	}
> 
> Although this check looks cheap enough that it should not hurt to add
> it.  So I think this should be ok, but maybe respin with your planned
> commit message update.

Only for the right reasons, imho.

-- 
Jens Axboe

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

[Keith Busch]

On Fri, May 22, 2026 at 11:30:34AM -0400, Chao Shi wrote:
>   Oops: general protection fault ... KASAN: null-ptr-deref
>   RIP: blk_mq_complete_request_remote+0xe5/0xa80 block/blk-mq.c:1319
>    nvme_handle_cqe drivers/nvme/host/pci.c:1418 [inline]
>    nvme_poll_cq drivers/nvme/host/pci.c:1449
>    nvme_irq drivers/nvme/host/pci.c:1463

This scenario doesn't sound specific to nvme. Should blk-mq completion
check for the IN_FLIGHT state instead?

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

[Christoph Hellwig]

On Fri, May 22, 2026 at 11:30:34AM -0400, Chao Shi wrote:
> nvme_find_rq() resolves a device-supplied command id to a request with
> blk_mq_tag_to_rq(), which returns whatever request last used that tag -
> possibly one that is no longer in flight (freed, or never dispatched and
> thus with a NULL rq->mq_hctx).  Commit e7006de6c238 ("nvme: code
> command_id with a genctr for use-after-free validation") guards against
> this, but its generation counter is only 4 bits wide and can be matched
> by a malfunctioning or malicious device replaying command ids.  The
> driver then completes a request that is not outstanding, dereferencing a
> NULL rq->mq_hctx or double-completing a command:

I don't think an intentionally malicious device is part of the threat
model here.  This was added to protect against buggy devices.

> +	/*
> +	 * blk_mq_tag_to_rq() returns whatever request last used this tag, which
> +	 * may no longer be in flight if the device reports a bogus command id.
> +	 * Completing it would deref a NULL rq->mq_hctx or double-complete a
> +	 * command; the 4-bit genctr below only narrows the window.
> +	 */
> +	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;
> +	}

Although this check looks cheap enough that it should not hurt to add
it.  So I think this should be ok, but maybe respin with your planned
commit message update.

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

[Chao S]

Hi,

Since posting this I reproduced a more severe manifestation of the same
bug and confirmed the patch handles it; sharing as extra justification.

The commit message covers the freed / never-dispatched case (the NULL
rq->mq_hctx dereference).  When the stale command id instead maps to a
tag that has already been *reused*, the driver completes an unrelated,
still-in-flight request -- a use-after-free.  Under fuzzing (a device
that replays and reorders completions) this did not show up as a clean
NULL deref but as cross-subsystem memory corruption: general protection
faults in mtree_range_walk(), unlink_anon_vmas() and the slub freelist,
in unrelated tasks (modprobe, systemd-udevd, ...).  The trigger was a
stale completion delivered for a request that a concurrent controller
reset had just freed.

To confirm the fix addresses this, I rebuilt the kernel with the patch
and re-ran the same workload for ~10h.  The guard now rejects the
offending completion instead of acting on it:

  nvme nvme0: resetting controller
  nvme nvme0: completion for request 0x1c0 not in flight
  nvme nvme0: invalid id 448 completed on queue 2

and no use-after-free / corruption recurred over the run.

The code is unchanged; I'm happy to fold this into the commit message
as a v2 if you'd prefer it spelled out there.

Thanks,
Chao

On Fri, May 22, 2026 at 11:30 AM Chao Shi <coshi036@gmail.com> wrote:
>
> nvme_find_rq() resolves a device-supplied command id to a request with
> blk_mq_tag_to_rq(), which returns whatever request last used that tag -
> possibly one that is no longer in flight (freed, or never dispatched and
> thus with a NULL rq->mq_hctx).  Commit e7006de6c238 ("nvme: code
> command_id with a genctr for use-after-free validation") guards against
> this, but its generation counter is only 4 bits wide and can be matched
> by a malfunctioning or malicious device replaying command ids.  The
> driver then completes a request that is not outstanding, dereferencing a
> NULL rq->mq_hctx or double-completing a command:
>
>   Oops: general protection fault ... KASAN: null-ptr-deref
>   RIP: blk_mq_complete_request_remote+0xe5/0xa80 block/blk-mq.c:1319
>    nvme_handle_cqe drivers/nvme/host/pci.c:1418 [inline]
>    nvme_poll_cq drivers/nvme/host/pci.c:1449
>    nvme_irq drivers/nvme/host/pci.c:1463
>
> Require the request to be in flight before completing it.  The check uses
> the request state, so it also covers controllers with
> NVME_QUIRK_SKIP_CID_GEN.
>
> Found by FuzzNvme(Syzkaller with FEMU fuzzing framework).
>
> Acked-by: Sungwoo Kim <iam@sung-woo.kim>
> Acked-by: Dave Tian <daveti@purdue.edu>
> Acked-by: Weidong Zhu <weizhu@fiu.edu>
> Signed-off-by: Chao Shi <coshi036@gmail.com>
> ---
>  drivers/nvme/host/nvme.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 9a5f28c5103c..3a525c1dc818 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -615,6 +615,17 @@ static inline struct request *nvme_find_rq(struct blk_mq_tags *tags,
>                         tag);
>                 return NULL;
>         }
> +       /*
> +        * blk_mq_tag_to_rq() returns whatever request last used this tag, which
> +        * may no longer be in flight if the device reports a bogus command id.
> +        * Completing it would deref a NULL rq->mq_hctx or double-complete a
> +        * command; the 4-bit genctr below only narrows the window.
> +        */
> +       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;
> +       }
>         if (unlikely(nvme_genctr_mask(nvme_req(rq)->genctr) != genctr)) {
>                 dev_err(nvme_req(rq)->ctrl->device,
>                         "request %#x genctr mismatch (got %#x expected %#x)\n",
> --
> 2.43.0
>

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

[Chao Shi]

nvme_find_rq() resolves a device-supplied command id to a request with
blk_mq_tag_to_rq(), which returns whatever request last used that tag -
possibly one that is no longer in flight (freed, or never dispatched and
thus with a NULL rq->mq_hctx).  Commit e7006de6c238 ("nvme: code
command_id with a genctr for use-after-free validation") guards against
this, but its generation counter is only 4 bits wide and can be matched
by a malfunctioning or malicious device replaying command ids.  The
driver then completes a request that is not outstanding, dereferencing a
NULL rq->mq_hctx or double-completing a command:

  Oops: general protection fault ... KASAN: null-ptr-deref
  RIP: blk_mq_complete_request_remote+0xe5/0xa80 block/blk-mq.c:1319
   nvme_handle_cqe drivers/nvme/host/pci.c:1418 [inline]
   nvme_poll_cq drivers/nvme/host/pci.c:1449
   nvme_irq drivers/nvme/host/pci.c:1463

Require the request to be in flight before completing it.  The check uses
the request state, so it also covers controllers with
NVME_QUIRK_SKIP_CID_GEN.

Found by FuzzNvme(Syzkaller with FEMU fuzzing framework).

Acked-by: Sungwoo Kim <iam@sung-woo.kim>
Acked-by: Dave Tian <daveti@purdue.edu>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
---
 drivers/nvme/host/nvme.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 9a5f28c5103c..3a525c1dc818 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -615,6 +615,17 @@ static inline struct request *nvme_find_rq(struct blk_mq_tags *tags,
 			tag);
 		return NULL;
 	}
+	/*
+	 * blk_mq_tag_to_rq() returns whatever request last used this tag, which
+	 * may no longer be in flight if the device reports a bogus command id.
+	 * Completing it would deref a NULL rq->mq_hctx or double-complete a
+	 * command; the 4-bit genctr below only narrows the window.
+	 */
+	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;
+	}
 	if (unlikely(nvme_genctr_mask(nvme_req(rq)->genctr) != genctr)) {
 		dev_err(nvme_req(rq)->ctrl->device,
 			"request %#x genctr mismatch (got %#x expected %#x)\n",
-- 
2.43.0