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

[Keith Busch]

On Wed, Mar 09, 2022 at 10:48:35AM +0800, Ming Lei wrote:
> > > On Tue, Mar 08, 2022 at 04:39:04PM -0800, Keith Busch wrote:
> 
> BTW, this issue is actually one real report from one Red Hat Customer.

And the correct fix has always been to fix the application.

> > > 
> > > But the spec states clearly the data length of IDENTIFY command is 4096
> > > and PRP list can't be used, so why do you think it isn't complete or
> > > future proof to validate data length of IDENTIFY in nvme driver?
> > 
> > The current spec says that opcode uses 4k today. What about some time in
> > the future?
> 
> spec change should only be applied on future hardware, which can not break
> current in-market hardware.

If a new Identify CNS were invented by committee that uses 8k, then the
older driver enforcing only 4k mappings will create more corruption, and
then it would be the driver's fault.

> nvme target has validated the Identify's transfer length already.

nvmet provides a fabrics targets, which uses SGL, not PRP. The SGL
encodes the length, so it's possible to validate it.
 
> > And why are you focusing on Identify anyway?
> 
> Nvme spec states explicitly that the following 4 commands can't use PRP list:
> 
> - Identify command
> - Namespace Attachment command
> - Namespace Management command
> - Set Features command
> 
> So it should be enough to just validate these commands.

Why are these 4 opcodes so special that the driver should provide
training wheels for broken apps, yet it must trust the same app with the
hundreds of other possible opcodes through the same interface?