[PATCH v6 0/1] nvme : Add ctrl id to allowed passthru commands

[PATCH v6 0/1] nvme : Add ctrl id to allowed passthru commands

[Joel Granados]

What?
In this patch I add NVME_ID_CNS_CS_CTRL and NVME_ID_CNS_CTRL to the nvme
admin commands that are allowed in unprivileged passthru.

Why?
This will allow access to attributes that are needed to effectively write
to the char device in passthru.  Applications with write permissions should
not need to be privileged to write to the device. With Kanchan's latest patch
(https://lore.kernel.org/linux-nvme/20221020070205.57366-1-joshi.k@samsung.com/)
the nvme IO and identify commands in passthru now follow device
permissions; however there are still some controller attributes like
minimal data transfer size (MDTS) which need a privileged user to be
queried.

How?
Add NVME_ID_CNS_CS_CTRL and NVME_ID_CNS_CTRL to the allow list in
nvme_cmd_allowed.

V6:
* Added two examples of write attributes in the commit message.
* Added review tags

V5:
* Drop the ioclt implementation in favor of just adding the controller id
  commands to the allow list in nvme_cmd_allowed. This resulted after a
  comment from Kieth Busch pointing out that adding a struct and an ioctl
  for stuff that is already there, makes no sense.  IMO the ioctl
  patch is mostly done and is there if we need it.

V4:
* Fixed an error where the ioctl number would change if new members were
  added. Now I use _IO instead of _IOWR to avoid leave the ioctl number
  static. This is very similar to the implementation contained in
  https://github.com/torvalds/linux/blob/master/include/uapi/linux/vfio.h

V3:
* Removed unneeded comments in nvme_ioctl.h
* Added a comment to the nvme_sectors_to_mps function
* Moved size checks to nvme_check_size in core.h
* Changed struct initialization to match what we use in nvme driver {} vs
  {0}

V2:
* Changed comment from // to /**/
* Took a call out from an if condition and assigned it to ret var.

Joel Granados (1):
  nvme : Add ctrl id to allowed passthru commands

 drivers/nvme/host/ioctl.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.30.2

[PATCH v6 1/1] nvme : Add ctrl id to allowed passthru commands

[Joel Granados]

Add NVME_ID_CNS_CS_CTRL and NVME_ID_CNS_CTRL to the nvme admin commands
that are allowed in unprivileged passthru. This will make attributes like
maximum data transfer size (mdts) and write zeroes size limit (wzsl)
available so they can be used to effectively write to the char device in
passthru.

Signed-off-by: Joel Granados <j.granados@samsung.com>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/host/ioctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 9550a69029b3..9ddda571f046 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -34,6 +34,8 @@ static bool nvme_cmd_allowed(struct nvme_ns *ns, struct nvme_command *c,
 			case NVME_ID_CNS_NS:
 			case NVME_ID_CNS_CS_NS:
 			case NVME_ID_CNS_NS_CS_INDEP:
+			case NVME_ID_CNS_CS_CTRL:
+			case NVME_ID_CNS_CTRL:
 				return true;
 			}
 		}
-- 
2.30.2

Re: [PATCH v6 0/1] nvme : Add ctrl id to allowed passthru commands

[Christoph Hellwig]

Thanks,

I've applied this to the nvme-6.2 branch with a rewritten commit log to
explain a bit better why this feature is useful.

Re: [PATCH v6 0/1] nvme : Add ctrl id to allowed passthru commands

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 216 bytes --]

On Tue, Dec 06, 2022 at 09:16:37AM +0100, Christoph Hellwig wrote:
> Thanks,
> 
> I've applied this to the nvme-6.2 branch with a rewritten commit log to
> explain a bit better why this feature is useful.
Thx.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]