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

[Mauricio Faria de Oliveira <mfo@igalia.com>]

On 2026-11-03 17:06, Keith Busch wrote:
> 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.

Could you please clarify? Not that I'm defending the patch; just so I
understand your point.

The reason I ask is, IIUIC, if another CPU changes the request state to
in-flight and wins the race over an incorrect completion that maps to
that request, the request is declared complete anyway, even without this
code, no?
 
> [...] 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. [...]

Yes, but even though the driver can't tell a bogus completion is
different, it may tell it is unexpected — at least some times; for
example, the existing generation-counter (genctr) guard, based on driver
state [1].

> [...] This kind of
> hardware shouldn't be allowed to exist as it can't be used successfully
> when it behaves this way.

I agree that a device in this state can't be used successfully, but the
kernel should, IMHO. 
Say, in case the device hits this due to firmware which needs an update,
the kernel crash might prevent the user from applying it, in the first
place.

Please let me know your thoughts. If there is interest, I'm happy to try
a different approach.

Thanks,

[1] commit e7006de6c238 ("nvme: code command_id with a genctr for
use-after-free validation")

-- 
Mauricio