Re: [PATCH 2/2] nvme: identify-namespace without CAP_SYS_ADMIN

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

On 10/20/22 00:02, Kanchan Joshi wrote:
> Allow all identify-namespace variants (CNS 00h, 05h and 08h) without
> requiring CAP_SYS_ADMIN. The information (retrieved using id-ns) is
> needed to form IO commands for passthrough interface.
> 
> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
> ---
>   drivers/nvme/host/ioctl.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
> index 9c581b1a8956..9273db147872 100644
> --- a/drivers/nvme/host/ioctl.c
> +++ b/drivers/nvme/host/ioctl.c
> @@ -15,9 +15,19 @@ bool nvme_cmd_allowed(struct nvme_ns *ns, struct nvme_command *c, fmode_t mode)
>   	if (capable(CAP_SYS_ADMIN))
>   		return true;
>   
> -	/* admin commands are not allowed */
> -	if (!ns)
> +	/* policy for admin commands */

above comment is not needed as it is clear from the opcode below
you are dealing with admin commands only that too specific cns
values ..

> +	if (!ns) {
> +		if (opcode == nvme_admin_identify) {
> +			switch (c->identify.cns) {
> +			case NVME_ID_CNS_NS:
> +			case NVME_ID_CNS_CS_NS:
> +			case NVME_ID_CNS_NS_CS_INDEP:
> +				return true;
> +			}
> +		}
> +		/* other admin commands are not allowed */

same here..

>   		return false;

if and swicth and two returns are looking confusing, I'd use
nested switch case default here..

> +	}
>   
>   	/* exclude vendor-specific io and fabrics commands */
>   	if (opcode >= nvme_cmd_vendor_start || opcode == nvme_fabrics_command)


-ck