[PATCH v2 0/4] nvme: protect against possible request reference after completion

[Sagi Grimberg]

>>> Nothing in nvme protects against referencing a request after it was completed.
>>> For example, in case a buggy controller sends a completion twice for the same
>>> request, the host can access and modify a request that was already completed.
>>>
>>> At best, this will cause a panic, but on the worst case, this can cause a silent
>>> data corruption if the request was already reused and executed by the time
>>> we reference it.
>>>
>>> The nvme command_id is an opaque that we simply placed the request tag thus far.
>>> To protect against a access after completion, we introduce a generation counter
>>> to the upper 4-bits of the command_id that will increment every invocation and
>>> be validated upon the reception of a completion. This will limit the maximum
>>> queue depth to be effectively 4095, but we hardly ever use such long queues
>>> (in fabrics the maximum is already 1024).
>>
>> Keith,
>>
>> Did you get a chance to look at the performance impact of this patch set? I
>> think we are all in agreement that this is a useful safeguard if
>> there is no major performance impact.
> 
> Yes, I was able to set up my more powerful machine for perf tests
> earlier this week. I ran fio hipri tests against this patch yesterday,
> and did not observe a measurable difference with either io_uring or
> pvsync2 engines, so no objection from me here.
> 
> Acked-by: Keith Busch <kbusch at kernel.org>

Cool, Christoph, where do you stand on this? Can we move forward with
this?