Re: nvme-host: disk corruptions when issuing IDENTIFY commands via ioctl()

[Keith Busch <kbusch@kernel.org>]

On Thu, Mar 10, 2022 at 05:04:00PM +0100, Christoph Hellwig wrote:
> On Wed, Mar 09, 2022 at 08:23:03AM -0800, Keith Busch wrote:
> > > Combination of a broken application (does what the spec explicitly
> > > tells it not do) and broken hardware (does the most stupid thing when
> > > fed invalid input), not much the driver can do here.
> > 
> > There's nothing the hardware can do either to know it was given invalid
> > input here if PRP2 is page aligned. There's no way it can tell the
> > difference between a PRP List vs PRP destination.
> 
> Well, it can know that there must be at most two PRP2 for Identify when
> the MPS is set to 4k.  Yes, this is annoying especially with hardware
> allerated frontends, but that's what you get for that stupid globally
> harmful microptimization that PRPs are.

Yep. If the host is purposufully tricked into making a PRP list when the
real payload didn't need it, the device will think PRP2 is the
destination buffer and corrupt that memory, and it's not the device's
fault. 

I agree the optimization was not worth the trouble it inflicted.

I don't believe it's the driver's responsibility either, though, and am
completely against the driver providing any sanity checks for broken
apps as Ming advocated. It is a maintenance nightmare and doomed for
failure. Just one example: 1.0's 'Get Log Page' said no PRP lists
allowed, so an older driver enforcing that would have crippled 1.1+
devices.

The app must own the responsibility for using the interface correctly.