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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YigVg/URukuwwKWF@T590>
Date:   Wed, 9 Mar 2022 10:48:35 +0800
From:   Ming Lei <ming.lei@...hat.com>
To:     Keith Busch <kbusch@...nel.org>
Cc:     Maurizio Lombardi <mlombard@...hat.com>,
        linux-nvme@...ts.infradead.org, axboe@...com,
        Christoph Hellwig <hch@....de>,
        Sagi Grimberg <sagi@...mberg.me>, Ming Lei <minlei@...hat.com>,
        linux-kernel@...r.kernel.org
Subject: Re: nvme-host: disk corruptions when issuing IDENTIFY commands via
 ioctl()

On Tue, Mar 08, 2022 at 05:14:29PM -0800, Keith Busch wrote:
> On Wed, Mar 09, 2022 at 09:02:42AM +0800, Ming Lei wrote:
> > On Tue, Mar 08, 2022 at 04:39:04PM -0800, Keith Busch wrote:
> > > On Wed, Mar 09, 2022 at 08:18:47AM +0800, Ming Lei wrote:
> > > > Given NVMe spec states that data length of IDENTIFY command should be
> > > > 4096bytes, and PRP list can't be used. 
> > > > 
> > > > So looks nvme driver need to validate the command before submitting to
> > > > hardware, otherwise any buggy application can break FS or memory easily.
> > > 
> > > No way. The driver does not police the user passthrough interface for
> > > these kinds of things.
> > 
> > So you trust application to provide correct data always?
> >
> > From user viewpoint, this defect provides one easy hole to break FS or
> > memory, it is one serious issue, IMO. The FS/memory corruption can
> > be reproduced easily even in VM.
> 
> It doesn't seem so serious considering it's been this way for 10 years,
> and we already knew about this. It's even been reported before:
> 
>   http://lists.infradead.org/pipermail/linux-nvme/2013-August/000365.html

BTW, this issue is actually one real report from one Red Hat Customer.

> 
> > > It couldn't ever be complete or future proof if
> > > it did.
> > 
> > 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.

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

> 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.

Thanks, 
Ming