Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 17 May 2023 18:03:35 +0200
From:   Claudio Imbrenda <imbrenda@linux.ibm.com>
To:     Steffen Eiden <seiden@linux.ibm.com>
Cc:     kvm@vger.kernel.org, linux-s390@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Viktor Mihajlovski <mihajlov@linux.ibm.com>,
        Janosch Frank <frankja@linux.ibm.com>,
        Nico Boehr <nrb@linux.ibm.com>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Heiko Carstens <hca@linux.ibm.com>,
        Hendrik Brueckner <brueckner@linux.ibm.com>
Subject: Re: [PATCH 3/5] s390/uvdevice: Add 'List Secrets' UVC
Message-ID: <20230517180335.05599c6a@p-imbrenda>
In-Reply-To: <20230512093153.206378-4-seiden@linux.ibm.com>
References: <20230512093153.206378-1-seiden@linux.ibm.com>
        <20230512093153.206378-4-seiden@linux.ibm.com>
Organization: IBM
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 12 May 2023 11:31:51 +0200
Steffen Eiden <seiden@linux.ibm.com> wrote:

> Userspace can call the List Secrets Ultravisor Call
> using IOCTLs on the uvdevice.
> During the handling of the new IOCTL nr the uvdevice will do some sanity
> checks first. Then, perform the Ultravisor command, and copy the answer
> to userspace.
> If the List Secrets UV facility is not present, UV will return
> invalid command rc. This won't be fenced in the driver and does not
> result in a negative return value. This is also true for any other
> possible error code the UV can return.
> 
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---

[...]

> diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
> index ba3e60dc4ba8..5fcd719047ab 100644
> --- a/drivers/s390/char/uvdevice.c
> +++ b/drivers/s390/char/uvdevice.c
> @@ -38,6 +38,7 @@ static const u64 ioctl_nr_to_uvc_bit[] __initconst = {
>  	[UVIO_IOCTL_UVDEV_INFO_NR] = -1UL,
>  	[UVIO_IOCTL_ATT_NR] = BIT_UVC_CMD_RETR_ATTEST,
>  	[UVIO_IOCTL_ADD_SECRET_NR] = BIT_UVC_CMD_ADD_SECRET,
> +	[UVIO_IOCTL_LIST_SECRETS_NR] = BIT_UVC_CMD_LIST_SECRETS,
>  };
>  
>  static_assert(ARRAY_SIZE(ioctl_nr_to_uvc_bit) == UVIO_IOCTL_NUM_IOCTLS);
> @@ -286,6 +287,57 @@ static int uvio_add_secret(struct uvio_ioctl_cb *uv_ioctl)
>  	return ret;
>  }
>  
> +/** uvio_list_secrets() - perform a List Secret UVC
> + *
> + * @uv_ioctl: ioctl control block
> + *
> + * uvio_list_secrets() performs the List Secret Ultravisor Call.
> + * It verifies that the given userspace argument address is valid and its size
> + * is sane. Every other check is made by the Ultravisor (UV) and won't result
> + * in a negative return value. It builds the request, performs the UV-call,
> + * and copies the result to userspace.
> + *
> + * The argument specifies the location for the result of the UV-Call.
> + *
> + * If the List Secrets UV facility is not present,
> + * UV will return invalid command rc. This won't be fenced in the driver
> + * and does not result in a negative return value.
> + *
> + * Context: might sleep
> + *
> + * Return: 0 on success or a negative error code on error.
> + */
> +static int uvio_list_secrets(struct uvio_ioctl_cb *uv_ioctl)
> +{
> +	void __user *user_buf_arg = (void __user *)uv_ioctl->argument_addr;
> +	struct uv_cb_guest_addr uvcb = {
> +		.header.len = sizeof(uvcb),
> +		.header.cmd = UVC_CMD_LIST_SECRETS,
> +	};
> +	void *secrets = NULL;
> +	int ret;
> +
> +	if (uv_ioctl->argument_len != UVIO_LIST_SECRETS_LEN)
> +		return -EINVAL;
> +
> +	secrets = kvzalloc(uv_ioctl->argument_len, GFP_KERNEL);
> +	if (!secrets)
> +		return -ENOMEM;
> +
> +	uvcb.addr = (u64)secrets;

I think you need virt_to_phys()

> +	uv_call_sched(0, (u64)&uvcb);
> +	uv_ioctl->uv_rc = uvcb.header.rc;
> +	uv_ioctl->uv_rrc = uvcb.header.rrc;
> +
> +	if (copy_to_user(user_buf_arg, secrets, uv_ioctl->argument_len))
> +		ret = -EFAULT;
> +	else
> +		ret = 0;
> +
> +	kvfree(secrets);
> +	return ret;
> +}
> +
>  static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
>  				     unsigned long cmd)
>  {
> @@ -333,6 +385,9 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  	case UVIO_IOCTL_ADD_SECRET_NR:
>  		ret = uvio_add_secret(&uv_ioctl);
>  		break;
> +	case UVIO_IOCTL_LIST_SECRETS_NR:
> +		ret = uvio_list_secrets(&uv_ioctl);
> +		break;
>  	default:
>  		ret = -ENOIOCTLCMD;
>  		break;