2023-06-06 11:58:25

by Steffen Eiden

[permalink] [raw]
Subject: [PATCH v3 0/6] s390/uvdevice: Expose secret UVCs

IBM Secure Execution guests may want to inject secrets into the Ultravisor(UV).
Also they should be able to know which secrets the UV possesses and prevent the
further addition of more secrets.

Therefore, add three new Ultravisor-Calls and expose them via the uvdevice: Add
Secret, List Secrets, and Lock Secrets. The uvdevice still acts as the
messenger only and does not inspect or modify the requests. Only some sanity
checks are made to prevent the kernel from corruption.
Also add a new IOCTL to get information about the supported UV-calls of the
uvdevice. As userspace wants to know which secrets, types, etc. are supported
expose the corresponding UV Query info data to userspace via sysfs.

The series contains:
* A new info IOCTL, giving information about the capabilities of the uvdevice and UV
* 3 patches adding new Ultravisor-Calls and expose them to userspace
* A patch replacing scnprintf with sysfs_emit in arch/s390/kernel/uv.c
* A patch with an Ultravisor Query Info update for the new secret related information

Changes for v3:
* misc nits from Janosch

Changes for v2:
* use __set_bit instead of the atomic set_bit (Heiko)
* add a patch for replacing scnprintf with sysfs_emit in arch/s390/kernel/uv.c (Heiko)
* use scnprintf instead of sysfs_emit for the new sysfs entries in the last patch (Heiko)
* use hex values in struct definitions (Claudio)

Steffen

Steffen Eiden (6):
s390/uvdevice: Add info IOCTL
s390/uvdevice: Add 'Add Secret' UVC
s390/uvdevice: Add 'List Secrets' UVC
s390/uvdevice: Add 'Lock Secret Store' UVC
s390/uv: replace scnprintf with sysfs_emit
s390/uv: Update query for secret-UVCs

arch/s390/boot/uv.c | 4 +
arch/s390/include/asm/uv.h | 32 +++-
arch/s390/include/uapi/asm/uvdevice.h | 53 +++++-
arch/s390/kernel/uv.c | 94 +++++++----
drivers/s390/char/uvdevice.c | 225 +++++++++++++++++++++++++-
5 files changed, 368 insertions(+), 40 deletions(-)

--
2.40.1



2023-06-06 12:01:06

by Steffen Eiden

[permalink] [raw]
Subject: [PATCH v3 5/6] s390/uv: replace scnprintf with sysfs_emit

Replace scnprintf(page, PAGE_SIZE, ...) with the page size aware
sysfs_emit(buf, ...) which adds some sanity checks.

Signed-off-by: Steffen Eiden <[email protected]>
---
arch/s390/kernel/uv.c | 54 +++++++++++++++++++------------------------
1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index cb2ee06df286..cd3a591edab3 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -460,13 +460,13 @@ EXPORT_SYMBOL_GPL(arch_make_page_accessible);

#if defined(CONFIG_PROTECTED_VIRTUALIZATION_GUEST) || IS_ENABLED(CONFIG_KVM)
static ssize_t uv_query_facilities(struct kobject *kobj,
- struct kobj_attribute *attr, char *page)
+ struct kobj_attribute *attr, char *buf)
{
- return scnprintf(page, PAGE_SIZE, "%lx\n%lx\n%lx\n%lx\n",
- uv_info.inst_calls_list[0],
- uv_info.inst_calls_list[1],
- uv_info.inst_calls_list[2],
- uv_info.inst_calls_list[3]);
+ return sysfs_emit(buf, "%lx\n%lx\n%lx\n%lx\n",
+ uv_info.inst_calls_list[0],
+ uv_info.inst_calls_list[1],
+ uv_info.inst_calls_list[2],
+ uv_info.inst_calls_list[3]);
}

static struct kobj_attribute uv_query_facilities_attr =
@@ -491,30 +491,27 @@ static struct kobj_attribute uv_query_supp_se_hdr_pcf_attr =
__ATTR(supp_se_hdr_pcf, 0444, uv_query_supp_se_hdr_pcf, NULL);

static ssize_t uv_query_dump_cpu_len(struct kobject *kobj,
- struct kobj_attribute *attr, char *page)
+ struct kobj_attribute *attr, char *buf)
{
- return scnprintf(page, PAGE_SIZE, "%lx\n",
- uv_info.guest_cpu_stor_len);
+ return sysfs_emit(buf, "%lx\n", uv_info.guest_cpu_stor_len);
}

static struct kobj_attribute uv_query_dump_cpu_len_attr =
__ATTR(uv_query_dump_cpu_len, 0444, uv_query_dump_cpu_len, NULL);

static ssize_t uv_query_dump_storage_state_len(struct kobject *kobj,
- struct kobj_attribute *attr, char *page)
+ struct kobj_attribute *attr, char *buf)
{
- return scnprintf(page, PAGE_SIZE, "%lx\n",
- uv_info.conf_dump_storage_state_len);
+ return sysfs_emit(buf, "%lx\n", uv_info.conf_dump_storage_state_len);
}

static struct kobj_attribute uv_query_dump_storage_state_len_attr =
__ATTR(dump_storage_state_len, 0444, uv_query_dump_storage_state_len, NULL);

static ssize_t uv_query_dump_finalize_len(struct kobject *kobj,
- struct kobj_attribute *attr, char *page)
+ struct kobj_attribute *attr, char *buf)
{
- return scnprintf(page, PAGE_SIZE, "%lx\n",
- uv_info.conf_dump_finalize_len);
+ return sysfs_emit(buf, "%lx\n", uv_info.conf_dump_finalize_len);
}

static struct kobj_attribute uv_query_dump_finalize_len_attr =
@@ -530,48 +527,45 @@ static struct kobj_attribute uv_query_feature_indications_attr =
__ATTR(feature_indications, 0444, uv_query_feature_indications, NULL);

static ssize_t uv_query_max_guest_cpus(struct kobject *kobj,
- struct kobj_attribute *attr, char *page)
+ struct kobj_attribute *attr, char *buf)
{
- return scnprintf(page, PAGE_SIZE, "%d\n",
- uv_info.max_guest_cpu_id + 1);
+ return sysfs_emit(buf, "%d\n", uv_info.max_guest_cpu_id + 1);
}

static struct kobj_attribute uv_query_max_guest_cpus_attr =
__ATTR(max_cpus, 0444, uv_query_max_guest_cpus, NULL);

static ssize_t uv_query_max_guest_vms(struct kobject *kobj,
- struct kobj_attribute *attr, char *page)
+ struct kobj_attribute *attr, char *buf)
{
- return scnprintf(page, PAGE_SIZE, "%d\n",
- uv_info.max_num_sec_conf);
+ return sysfs_emit(buf, "%d\n", uv_info.max_num_sec_conf);
}

static struct kobj_attribute uv_query_max_guest_vms_attr =
__ATTR(max_guests, 0444, uv_query_max_guest_vms, NULL);

static ssize_t uv_query_max_guest_addr(struct kobject *kobj,
- struct kobj_attribute *attr, char *page)
+ struct kobj_attribute *attr, char *buf)
{
- return scnprintf(page, PAGE_SIZE, "%lx\n",
- uv_info.max_sec_stor_addr);
+ return sysfs_emit(buf, "%lx\n", uv_info.max_sec_stor_addr);
}

static struct kobj_attribute uv_query_max_guest_addr_attr =
__ATTR(max_address, 0444, uv_query_max_guest_addr, NULL);

static ssize_t uv_query_supp_att_req_hdr_ver(struct kobject *kobj,
- struct kobj_attribute *attr, char *page)
+ struct kobj_attribute *attr, char *buf)
{
- return scnprintf(page, PAGE_SIZE, "%lx\n", uv_info.supp_att_req_hdr_ver);
+ return sysfs_emit(buf, "%lx\n", uv_info.supp_att_req_hdr_ver);
}

static struct kobj_attribute uv_query_supp_att_req_hdr_ver_attr =
__ATTR(supp_att_req_hdr_ver, 0444, uv_query_supp_att_req_hdr_ver, NULL);

static ssize_t uv_query_supp_att_pflags(struct kobject *kobj,
- struct kobj_attribute *attr, char *page)
+ struct kobj_attribute *attr, char *buf)
{
- return scnprintf(page, PAGE_SIZE, "%lx\n", uv_info.supp_att_pflags);
+ return sysfs_emit(buf, "%lx\n", uv_info.supp_att_pflags);
}

static struct kobj_attribute uv_query_supp_att_pflags_attr =
@@ -605,7 +599,7 @@ static ssize_t uv_is_prot_virt_guest(struct kobject *kobj,
#ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
val = prot_virt_guest;
#endif
- return scnprintf(page, PAGE_SIZE, "%d\n", val);
+ return sysfs_emit(page, "%d\n", val);
}

static ssize_t uv_is_prot_virt_host(struct kobject *kobj,
@@ -617,7 +611,7 @@ static ssize_t uv_is_prot_virt_host(struct kobject *kobj,
val = prot_virt_host;
#endif

- return scnprintf(page, PAGE_SIZE, "%d\n", val);
+ return sysfs_emit(page, "%d\n", val);
}

static struct kobj_attribute uv_prot_virt_guest =
--
2.40.1


2023-06-06 12:02:18

by Steffen Eiden

[permalink] [raw]
Subject: [PATCH v3 2/6] s390/uvdevice: Add 'Add Secret' UVC

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.

Signed-off-by: Steffen Eiden <[email protected]>
---
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.
+ */
+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;
--
2.40.1


2023-06-06 12:02:22

by Steffen Eiden

[permalink] [raw]
Subject: [PATCH v3 4/6] s390/uvdevice: Add 'Lock Secret Store' UVC

Userspace can call the Lock Secret Store 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
return codes to userspace.
If the Lock 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 <[email protected]>
---
arch/s390/include/asm/uv.h | 2 ++
arch/s390/include/uapi/asm/uvdevice.h | 3 +++
drivers/s390/char/uvdevice.c | 39 +++++++++++++++++++++++++++
3 files changed, 44 insertions(+)

diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index 3739c8f6a129..3203ffbdde6b 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -60,6 +60,7 @@
#define UVC_CMD_RETR_ATTEST 0x1020
#define UVC_CMD_ADD_SECRET 0x1031
#define UVC_CMD_LIST_SECRETS 0x1033
+#define UVC_CMD_LOCK_SECRETS 0x1034

/* Bits in installed uv calls */
enum uv_cmds_inst {
@@ -92,6 +93,7 @@ enum uv_cmds_inst {
BIT_UVC_CMD_RETR_ATTEST = 28,
BIT_UVC_CMD_ADD_SECRET = 29,
BIT_UVC_CMD_LIST_SECRETS = 30,
+ BIT_UVC_CMD_LOCK_SECRETS = 31,
};

enum uv_feat_ind {
diff --git a/arch/s390/include/uapi/asm/uvdevice.h b/arch/s390/include/uapi/asm/uvdevice.h
index 76045da44868..b9c2f14a6af3 100644
--- a/arch/s390/include/uapi/asm/uvdevice.h
+++ b/arch/s390/include/uapi/asm/uvdevice.h
@@ -80,6 +80,7 @@ enum UVIO_IOCTL_NR {
UVIO_IOCTL_ATT_NR,
UVIO_IOCTL_ADD_SECRET_NR,
UVIO_IOCTL_LIST_SECRETS_NR,
+ UVIO_IOCTL_LOCK_SECRETS_NR,
/* must be the last entry */
UVIO_IOCTL_NUM_IOCTLS
};
@@ -89,11 +90,13 @@ enum UVIO_IOCTL_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_IOCTL_LIST_SECRETS UVIO_IOCTL(UVIO_IOCTL_LIST_SECRETS_NR)
+#define UVIO_IOCTL_LOCK_SECRETS UVIO_IOCTL(UVIO_IOCTL_LOCK_SECRETS_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)
#define UVIO_SUPP_LIST_SECRETS UVIO_SUPP_CALL(UVIO_IOCTL_LIST_SECRETS_NR)
+#define UVIO_SUPP_LOCK_SECRETS UVIO_SUPP_CALL(UVIO_IOCTL_LOCK_SECRETS_NR)

#endif /* __S390_ASM_UVDEVICE_H */
diff --git a/drivers/s390/char/uvdevice.c b/drivers/s390/char/uvdevice.c
index 54a3730f5d0c..8079e15e309c 100644
--- a/drivers/s390/char/uvdevice.c
+++ b/drivers/s390/char/uvdevice.c
@@ -39,6 +39,7 @@ static const u32 ioctl_nr_to_uvc_bit[] __initconst = {
[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,
+ [UVIO_IOCTL_LOCK_SECRETS_NR] = BIT_UVC_CMD_LOCK_SECRETS,
};

static_assert(ARRAY_SIZE(ioctl_nr_to_uvc_bit) == UVIO_IOCTL_NUM_IOCTLS);
@@ -334,6 +335,41 @@ static int uvio_list_secrets(struct uvio_ioctl_cb *uv_ioctl)
return ret;
}

+/** uvio_lock_secrets() - perform a Lock Secret Store UVC
+ * @uv_ioctl: ioctl control block
+ *
+ * uvio_lock_secrets() performs the Lock Secret Store Ultravisor Call. It
+ * performs the UV-call and copies the return codes to the ioctl control block.
+ * After this call was dispatched successfully every following Add Secret UVC
+ * and Lock Secrets UVC will fail with return code 0x102.
+ *
+ * The argument address and size must be 0.
+ *
+ * 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_lock_secrets(struct uvio_ioctl_cb *ioctl)
+{
+ struct uv_cb_nodata uvcb = {
+ .header.len = sizeof(uvcb),
+ .header.cmd = UVC_CMD_LOCK_SECRETS,
+ };
+
+ if (ioctl->argument_addr || ioctl->argument_len)
+ return -EINVAL;
+
+ uv_call(0, (u64)&uvcb);
+ ioctl->uv_rc = uvcb.header.rc;
+ ioctl->uv_rrc = uvcb.header.rrc;
+
+ return 0;
+}
+
static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
unsigned long cmd)
{
@@ -384,6 +420,9 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
case UVIO_IOCTL_LIST_SECRETS_NR:
ret = uvio_list_secrets(&uv_ioctl);
break;
+ case UVIO_IOCTL_LOCK_SECRETS_NR:
+ ret = uvio_lock_secrets(&uv_ioctl);
+ break;
default:
ret = -ENOIOCTLCMD;
break;
--
2.40.1


2023-06-06 14:03:11

by Janosch Frank

[permalink] [raw]
Subject: Re: [PATCH v3 5/6] s390/uv: replace scnprintf with sysfs_emit

On 6/6/23 13:37, Steffen Eiden wrote:
> Replace scnprintf(page, PAGE_SIZE, ...) with the page size aware
> sysfs_emit(buf, ...) which adds some sanity checks.
>
> Signed-off-by: Steffen Eiden <[email protected]>
> ---
> arch/s390/kernel/uv.c | 54 +++++++++++++++++++------------------------
> 1 file changed, 24 insertions(+), 30 deletions(-)
>
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index cb2ee06df286..cd3a591edab3 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -460,13 +460,13 @@ EXPORT_SYMBOL_GPL(arch_make_page_accessible);

> static struct kobj_attribute uv_query_supp_att_pflags_attr =
> @@ -605,7 +599,7 @@ static ssize_t uv_is_prot_virt_guest(struct kobject *kobj,
> #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
> val = prot_virt_guest;
> #endif
> - return scnprintf(page, PAGE_SIZE, "%d\n", val);
> + return sysfs_emit(page, "%d\n", val);
> }
>
> static ssize_t uv_is_prot_virt_host(struct kobject *kobj,
> @@ -617,7 +611,7 @@ static ssize_t uv_is_prot_virt_host(struct kobject *kobj,
> val = prot_virt_host;
> #endif
>
> - return scnprintf(page, PAGE_SIZE, "%d\n", val);
> + return sysfs_emit(page, "%d\n", val);
> }

These are still named page


2023-06-06 14:32:05

by Janosch Frank

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

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 <[email protected]>
> ---
> 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;


2023-06-06 15:36:08

by Janosch Frank

[permalink] [raw]
Subject: Re: [PATCH v3 4/6] s390/uvdevice: Add 'Lock Secret Store' UVC

On 6/6/23 13:37, Steffen Eiden wrote:
> Userspace can call the Lock Secret Store 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
> return codes to userspace.
> If the Lock 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 <[email protected]>

[...]

> +/** uvio_lock_secrets() - perform a Lock Secret Store UVC
> + * @uv_ioctl: ioctl control block
> + *
> + * uvio_lock_secrets() performs the Lock Secret Store Ultravisor Call. It
> + * performs the UV-call and copies the return codes to the ioctl control block.
> + * After this call was dispatched successfully every following Add Secret UVC
> + * and Lock Secrets UVC will fail with return code 0x102.
> + *
> + * The argument address and size must be 0.
> + *
> + * 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 not "list secrets" though.

> + *
> + * Context: might sleep
> + *
> + * Return: 0 on success or a negative error code on error.
> + */
> +static int uvio_lock_secrets(struct uvio_ioctl_cb *ioctl)
> +{
> + struct uv_cb_nodata uvcb = {
> + .header.len = sizeof(uvcb),
> + .header.cmd = UVC_CMD_LOCK_SECRETS,
> + };
> +
> + if (ioctl->argument_addr || ioctl->argument_len)
> + return -EINVAL;
> +
> + uv_call(0, (u64)&uvcb);
> + ioctl->uv_rc = uvcb.header.rc;
> + ioctl->uv_rrc = uvcb.header.rrc;
> +
> + return 0;
> +}
> +
> static int uvio_copy_and_check_ioctl(struct uvio_ioctl_cb *ioctl, void __user *argp,
> unsigned long cmd)
> {
> @@ -384,6 +420,9 @@ static long uvio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> case UVIO_IOCTL_LIST_SECRETS_NR:
> ret = uvio_list_secrets(&uv_ioctl);
> break;
> + case UVIO_IOCTL_LOCK_SECRETS_NR:
> + ret = uvio_lock_secrets(&uv_ioctl);
> + break;
> default:
> ret = -ENOIOCTLCMD;
> break;