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

[Keith Busch]

On Wed, Jun 16, 2021 at 09:28:53AM -0700, Sagi Grimberg wrote:
> 
> > 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>