linux-kernel - Re: [RFC] nvmet: configfs: use ctrl->instance to track passthru subsystems

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] [day] [month] [year] [list]

Message-ID: <b6169317-4714-4240-87ab-3e80b2aac2c4@grimberg.me>
Date:   Wed, 13 Dec 2023 15:40:43 +0200
From:   Sagi Grimberg <sagi@...mberg.me>
To:     Evan Burgess <evan.burgess@...gate.com>,
        Christoph Hellwig <hch@....de>,
        Chaitanya Kulkarni <kch@...dia.com>
Cc:     "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] nvmet: configfs: use ctrl->instance to track passthru
 subsystems


> To prevent enabling more than one passthrough subsystem per NVMe
> controller, passthru.c maintains an xarray indexed by cntlid values.
> An nvmet subsystem cannot enable passthrough via configfs if its
> passthru_ctrl->cntlid value is already accounted for by the xarray.
> 
> However, according to the NVMe base spec (rev 2.0c, p.145), "The
> Controller ID (CNTLID) value returned in the Identify Controller data
> structure may be used to uniquely identify a controller within an NVM
> subsystem," meaning that cntlid values are not guaranteed to be
> globally unique across multiple subsystems. Instead, the cntlid only
> uniquely identifies multiple controllers _within_ a subsystem.
> 
> As a result, multiple unique & valid NVMe targets can be blocked from
> enabling passthrough at the same time if their controllers share cntlid
> values, a behavior (seemingly) allowed by the spec. This could be
> remedied by indexing the xarray with passthru_ctrl->instance values,
> which are allocated per controller by IDA and thus should act as truly
> unique controller identifiers.
> 
> I have seen this issue in practice, but have found a suspicious lack of
> corroboration across this ML and elsewhere (so far). So, I am not
> discounting a possible misunderstanding of the spec and/or code here.

You are correct AFAICT.
Reviewed-by: Sagi Grimberg <sagi@...mberg.me>