Re: [PATCH v3 2/6] s390/uvdevice: Add 'Add Secret' UVC

Janosch Frank
2023-06-06T14:18:59Z

On 6/6/23 13:37, Steffen Eiden wrote:
> Userspace can call the Add Secret Ultravisor Call
> using IOCTLs on the uvdevice.
> During the handling of the new IOCTL nr the uvdevice will do some sanity
> checks first. Then, copy the request data to kernel space, perform the
> Ultravisor command, and copy the return codes to userspace.
> If the Add Secret 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.

The Add Secret UV call sends an encrypted and cryptographically verified
request to the Ultravisor. The request inserts a protected guest's
secret into the Ultravisor for later use.

The uvdevice is merely transporting the request from userspace to the
Ultravisor. It's neither checking nor manipulating the request data.

>
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
> arch/s390/include/asm/uv.h | 14 +++++++
> arch/s390/include/uapi/asm/uvdevice.h | 4 ++
> drivers/s390/char/uvdevice.c | 57 +++++++++++++++++++++++++++
> 3 files changed, 75 insertions(+)
>
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index 28a9ad57b6f1..1babc70ea5d4 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -58,6 +58,7 @@
> #define UVC_CMD_SET_SHARED_ACCESS 0x1000
> #define UVC_CMD_REMOVE_SHARED_ACCESS 0x1001
> #define UVC_CMD_RETR_ATTEST 0x1020
> +#define UVC_CMD_ADD_SECRET 0x1031
>
> /* Bits in installed uv calls */
> enum uv_cmds_inst {
> @@ -88,6 +89,7 @@ enum uv_cmds_inst {
> BIT_UVC_CMD_DUMP_CPU = 26,
> BIT_UVC_CMD_DUMP_COMPLETE = 27,
> BIT_UVC_CMD_RETR_ATTEST = 28,
> + BIT_UVC_CMD_ADD_SECRET = 29,
> };
>
> enum uv_feat_ind {
> @@ -292,6 +294,18 @@ struct uv_cb_dump_complete {
> u64 reserved30[5];
> } __packed __aligned(8);
>
> +/*
> + * A common UV call struct for pv guests that contains a single address
> + * Examples:
> + * Add Secret
> + */
> +struct uv_cb_guest_addr {
> + struct uv_cb_header header;
> + u64 reserved08[3];
> + u64 addr;
> + u64 reserved28[4];
> +} __packed __aligned(8);
> +
> static inline int __uv_call(unsigned long r1, unsigned long r2)
> {
> int cc;
> diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
> index 9d9b684836c2..e77410226598 100644
> --- a/arch/s390/include/uapi/asm/uvdevice.h
> +++ b/arch/s390/include/uapi/asm/uvdevice.h
> @@ -69,6 +69,7 @@ struct uvio_uvdev_info {
> #define UVIO_ATT_ARCB_MAX_LEN 0x100000
> #define UVIO_ATT_MEASUREMENT_MAX_LEN 0x8000
> #define UVIO_ATT_ADDITIONAL_MAX_LEN 0x8000
> +#define UVIO_ADD_SECRET_MAX_LEN 0x100000
>
> #define UVIO_DEVICE_NAME "uv"
> #define UVIO_TYPE_UVC 'u'
> @@ -76,6 +77,7 @@ struct uvio_uvdev_info {
> enum UVIO_IOCTL_NR {
> UVIO_IOCTL_UVDEV_INFO_NR = 0x00,
> UVIO_IOCTL_ATT_NR,
> + UVIO_IOCTL_ADD_SECRET_NR,
> /* must be the last entry */
> UVIO_IOCTL_NUM_IOCTLS
> };
> @@ -83,9 +85,11 @@ enum UVIO_IOCTL_NR {
> #define UVIO_IOCTL(nr) _IOWR(UVIO_TYPE_UVC, nr, struct uvio_ioctl_cb)
> #define UVIO_IOCTL_UVDEV_INFO UVIO_IOCTL(UVIO_IOCTL_UVDEV_INFO_NR)
> #define UVIO_IOCTL_ATT UVIO_IOCTL(UVIO_IOCTL_ATT_NR)
> +#define UVIO_IOCTL_ADD_SECRET UVIO_IOCTL(UVIO_IOCTL_ADD_SECRET_NR)
>
> #define UVIO_SUPP_CALL(nr) (1ULL << (nr))
> #define UVIO_SUPP_UDEV_INFO UVIO_SUPP_CALL(UVIO_IOCTL_UDEV_INFO_NR)
> #define UVIO_SUPP_ATT UVIO_SUPP_CALL(UVIO_IOCTL_ATT_NR)
> +#define UVIO_SUPP_ADD_SECRET UVIO_SUPP_CALL(UVIO_IOCTL_ADD_SECRET_NR)
>
> #endif /* __S390_ASM_UVDEVICE_H */
> diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
> index 4efeebcaf382..c10554bc1fee 100644
> --- a/drivers/s390/char/uvdevice.c
> +++ b/drivers/s390/char/uvdevice.c
> @@ -37,6 +37,7 @@
> static const u32 ioctl_nr_to_uvc_bit[] __initconst = {
> [UVIO_IOCTL_UVDEV_INFO_NR] = BIT_UVIO_INTERNAL,
> [UVIO_IOCTL_ATT_NR] = BIT_UVC_CMD_RETR_ATTEST,
> + [UVIO_IOCTL_ADD_SECRET_NR] = BIT_UVC_CMD_ADD_SECRET,
> };
>
> static_assert(ARRAY_SIZE(ioctl_nr_to_uvc_bit) == UVIO_IOCTL_NUM_IOCTLS);
> @@ -231,6 +232,59 @@ static int uvio_attestation(struct uvio_ioctl_cb *uv_ioctl)
> return ret;
> }
>
> +/** uvio_add_secret() - perform an Add Secret UVC
> + *
> + * @uv_ioctl: ioctl control block
> + *
> + * uvio_add_secret() performs the Add 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 copies the request to kernelspace, performs the UV-call, and
> + * copies the return codes to the ioctl control block. The argument has to point
> + * to an Add Secret Request Control Block. It is an encrypted and
> + * cryptographically verified request generated by userspace to insert the
> + * actual secret into the UV. If the Add Secret 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.
> + */

Maybe:

/** uvio_add_secret() - perform an Add Secret UVC

*

* @uv_ioctl: ioctl control block

*

* uvio_add_secret() performs the Add Secret Ultravisor Call.

*

* The given userspace argument address and size are verified to be

* valid but every other check is made by the Ultravisor

* (UV). Therefore UV errors won't result in a negative return

* value. The request is then copied to kernelspace, the UV-call is

* performed and the results are copied back to userspace.

*

* The argument has to point to an Add Secret Request Control Block

* which is an encrypted and cryptographically verified request that

* inserts a protected guest's secrets into the Ultravisor for later

* use.

*

* If the Add Secret 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_add_secret(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_ADD_SECRET,
> + };
> + void *asrcb = NULL;
> + int ret;
> +
> + if (uv_ioctl->argument_len > UVIO_ADD_SECRET_MAX_LEN)
> + return -EINVAL;
> + if (uv_ioctl->argument_len == 0)
> + return -EINVAL;
> +
> + asrcb = kvzalloc(uv_ioctl->argument_len, GFP_KERNEL);
> + if (!asrcb)
> + return -ENOMEM;
> +
> + ret = -EFAULT;
> + if (copy_from_user(asrcb, user_buf_arg, uv_ioctl->argument_len))
> + goto out;
> +
> + ret = 0;
> + uvcb.addr = (u64)asrcb;
> + uv_call_sched(0, (u64)&uvcb);
> + uv_ioctl->uv_rc = uvcb.header.rc;
> + uv_ioctl->uv_rrc = uvcb.header.rrc;
> +
> +out:
> + kvfree(asrcb);
> + return ret;
> +}
> +
> static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
> unsigned long cmd)
> {
> @@ -275,6 +329,9 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> case UVIO_IOCTL_ATT_NR:
> ret = uvio_attestation(&uv_ioctl);
> break;
> + case UVIO_IOCTL_ADD_SECRET_NR:
> + ret = uvio_add_secret(&uv_ioctl);
> + break;
> default:
> ret = -ENOIOCTLCMD;
> break;