linux-kernel - Re: [PATCH] nvme: disallow passthru cmd from targeting a nsid != nsid of the block dev

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 for Android: free password hash cracker in your pocket

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

Message-ID: <YF49GFrXUyMkNNhA@x1-carbon.lan>
Date:   Fri, 26 Mar 2021 19:59:21 +0000
From:   Niklas Cassel <Niklas.Cassel@....com>
To:     Keith Busch <kbusch@...nel.org>
CC:     Jens Axboe <axboe@...com>, Christoph Hellwig <hch@....de>,
        Sagi Grimberg <sagi@...mberg.me>,
        "minwoo.im.dev@...il.com" <minwoo.im.dev@...il.com>,
        "javier@...igon.com" <javier@...igon.com>,
        "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] nvme: disallow passthru cmd from targeting a nsid != nsid
 of the block dev

On Fri, Mar 26, 2021 at 12:19:42AM +0900, Keith Busch wrote:
> On Thu, Mar 25, 2021 at 09:48:37AM +0000, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cassel@....com>
> > 
> > When a passthru command targets a specific namespace, the ns parameter to
> > nvme_user_cmd()/nvme_user_cmd64() is set. However, there is currently no
> > validation that the nsid specified in the passthru command targets the
> > namespace/nsid represented by the block device that the ioctl was
> > performed on.
> > 
> > Add a check that validates that the nsid in the passthru command matches
> > that of the supplied namespace.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@....com>
> > ---
> > Currently, if doing NVME_IOCTL_IO_CMD on the controller char device,
> > if and only if there is a single namespace in the ctrl->namespaces list,
> > nvme_dev_user_cmd() will call nvme_user_cmd() with ns parameter set.
> > While it might be good that we validate the nsid even in this case,
> > perhaps we want to allow a nsid value in the passthru command to be
> > 0x0 and/or the NSID broadcast value? (Only when NVME_IOCTL_IO_CMD was
> > done on the controller char device though.)
> 
> There are no IO commands accepting a 0 NSID, so rejecting those from the
> driver should be okay.
> 
> FLUSH is the only IO command that takes a broadcast NSID. I suspect at
> least some entities were unfortunately sending broadcast flush through
> this interface, so it's possible we'll hear of breakage, but I'd agree
> your patch is still the right thing to do.

I don't think this should be a problem.

You shouldn't be sending a broadcast NSID via the per namespace block
device. It just seems silly to specify a specific namespace block device,
and then use the broadcast NSID. (This obviously should never have been
allowed.)

If you wanted to send a broadcast NSID, you should have used the controller
character device. However, nvme_dev_user_cmd() currently rejects any
NVME_IOCTL_IO_CMD when there is more than one namespace in ctrl->namespaces
list, so they could never have used the controller character device to send
a flush to more than one namespace. So here we don't change anything.


Kind regards,
Niklas