[PATCH v4 1/1] nvme : Add ioctl to query nvme attributes

[Joel Granados]

On Tue, Nov 29, 2022 at 07:57:43AM -0700, Keith Busch wrote:
> On Tue, Nov 22, 2022 at 10:22:44AM +0100, Joel Granados wrote:
> > An ioctl (NVME_IOCTL_GET_ATTR) is added to query nvme controller
> > attributes needed to effectively write to the char device without needing
> > to be a privileged user.  They are also provided on block devices for
> > convenience. The attributes here are meant to complement what you can
> > already get with the allowed identify commands.
> > 
> > New members are added to the nvme_ctrl struct: dmsl, vsl and wusl from
> > the I/O Command Set Specific Identify Controller Data Structure in the nvme
> > spec.
> 
> I think what you have here is fine given the constraints you have work
> with, though inventing new structures to report things in existing
> structures still seems off to me. Is there actually any privileged
This is a very good point that I 100% share (Less is more! Specially
when you are maintaining it :).

This relates directly to Kanchans patch for allow-listing the nvme
passthru commands (https://lore.kernel.org/linux-nvme/20221020070205.57366-3-joshi.k@samsung.com/)
as we were originally going to add the controller identify commands to
that list but decided not to after discussions in ALPSS.

> information contained within the controller scope identifications that
I have not come across anything that points to privileged information
leaking through this command. Discussions have touched on excluding
fabrics and vendor specific commands in IO queues
(https://lore.kernel.org/linux-nvme/20220910053536.GB23158@lst.de/) as
well as noting that there is not need to check the read permissions on
the device for the allow list
(https://lore.kernel.org/linux-nvme/47e19485-f321-cd2a-3408-173434b04d01@kernel.dk/)
as the device file has already been opened at that point.

> require the driver prevent non-admin users from seeing? If not, I think
> relaxing that access restriction will set us up for longer term success.
Also agree here. In order to access everything contained in this patch
we would have to add both NVME_ID_CNS_CS_CTRL (vsl, wusl, dmrl, dmsl, dmrsl,
oncs, wzsl) and NVME_ID_CNS_CTRL (mdts) to kanchans patch.
I will send a V5 that contains this approach as the next progression of
the patch. We can have further discussion there if needed.

Thx for the feedback.

Joel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20221205/ed0c0b1a/attachment-0001.sig>