Re: [PATCH v3] nvme: fix memory corruption for passthrough metadata

[Christoph Hellwig <hch@lst.de>]

On Tue, Oct 10, 2023 at 07:09:54PM +0530, Kanchan Joshi wrote:
> The case is for the single interleaved buffer with both data and
> metadata. When the driver sends this buffer to blk_rq_map_user_iov(),
> it may make a copy of it.
> This kernel buffer will be used for DMA rather than user buffer. If
> the user-buffer is short, the kernel buffer is also short.

Yes.  Note that we'll corrupt memory either way, so user vs kernel
does not matter.

> Does this explanation help?
> I can move the part to a separate patch.

Definitively separate function please, not sure if a separate
patch is required.

> Yes, not io_uring specific.
> Just that I was not sure on (i) whether to go back that far in
> history, and (ii) what patch to tag.

I think the one that adds the original problem is:

63263d60e0f9f37bfd5e6a1e83a62f0e62fc459f
Author: Keith Busch <kbusch@kernel.org>
Date:   Tue Aug 29 17:46:04 2017 -0400

    nvme: Use metadata for passthrough commands


> > > +     /* Exclude commands that do not have nlb in cdw12 */
> > > +     if (!nvme_nlb_in_cdw12(c->common.opcode))
> > > +             return true;
> >
> > So we can still get exactly the same corruption for all commands that
> > are not known?  That's not a very safe way to deal with the issue..
> 
> Given the way things are in NVMe, I do not find a better way.
> Maybe another day for commands that do (or can do) things very
> differently for nlb and PI representation.

Fixing just a subset of these problems is pointless.  If people want
to use metadata on vendor specific commands they need to work with
NVMe to figure out a generic way to pass the length.