Re: [PATCH v1 2/6] s390/uv: Retrieve UV secrets support

Christoph Schlameuss
2024-10-01T16:06:53Z

On Mon Sep 30, 2024 at 3:19 PM CEST, Steffen Eiden wrote:
> Provide a kernel API to retrieve secrets from the UV secret store.
> Add two new functions:
> * `uv_get_secret_metadata` - get metadata for a given secret identifier
> * `uv_retrieve_secret` - get the secret value for the secret index
>
> With those two functions one can extract the secret for a given secret
> id, if the secret is retrievable.
>
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
> arch/s390/include/asm/uv.h | 131 ++++++++++++++++++++++++++++++++++++-
> arch/s390/kernel/uv.c | 124 +++++++++++++++++++++++++++++++++++
> 2 files changed, 254 insertions(+), 1 deletion(-)

[...]

> /* Bits in installed uv calls */
> enum uv_cmds_inst {
> @@ -95,6 +96,7 @@ enum uv_cmds_inst {
> BIT_UVC_CMD_ADD_SECRET = 29,
> BIT_UVC_CMD_LIST_SECRETS = 30,
> BIT_UVC_CMD_LOCK_SECRETS = 31,

Is 32 skipped intentionally? Should there be a comment here that it is reserved?

> + BIT_UVC_CMD_RETR_SECRETS = 33,
> };

[...]

> +/**
> + * uv_secret_list - UV secret-metadata list
> + * @num_secr_stored: Number of secrets stored in this list
> + * @total_num_secrets: Number of secrets stored in the UV for this guest
> + * @next_valid_idx: positive number if there are more secrets available or zero

s/next_valid_idx/next_secret_idx/

> + * @secrets: Up to 85 UV-secret metadata entries.
> + */
> +struct uv_secret_list {
> + u16 num_secr_stored;
> + u16 total_num_secrets;
> + u16 next_secret_idx;
> + u16 reserved_06;
> + u64 reserved_08;
> + struct uv_secret_list_item secrets[85];
> +} __packed __aligned(8);
> +static_assert(sizeof(struct uv_secret_list) == PAGE_SIZE);
> +
> static inline int __uv_call(unsigned long r1, unsigned long r2)
> {
> int cc;
> @@ -383,6 +469,45 @@ static inline int uv_cmd_nodata(u64 handle, u16 cmd, u16 *rc, u16 *rrc)
> return cc ? -EINVAL : 0;
> }
>
> +/** uv_list_secrets() - Do a List Secrets UVC
> + * @buf: Buffer to write list into; size of one page
> + * @start_idx: The smallest index that should be included in the list.
> + * For the fist invocation use 0.
> + * @rc: Pointer to store the return code or NULL.
> + * @rrc: Pointer to store the return reason code or NULL.
> + *
> + * This function calls the List Secrets UVC. The result is written into `buf`,
> + * that needs to be at least one page of writable memory.
> + * `buf` consists of:
> + * * %struct uv_secret_list_hdr
> + * * %struct uv_secret_list_item (multiple)
> + *
> + * For `start_idx` use _0_ for the first call. If there are more secrets available
> + * but could not fit into the page then `rc` is `UVC_RC_MORE_DATA`.
> + * In this case use `uv_secret_list_hdr.next_valid_idx` for `start_idx`.

s/next_valid_idx/next_secret_idx/

> + *
> + * Context: might sleep
> + *
> + * Return: The UVC condition code
> + */
> +static inline int uv_list_secrets(u8 *buf, u16 start_idx, u16 *rc, u16 *rrc)
> +{
> + struct uv_cb_list_secrets uvcb = {
> + .header.len = sizeof(uvcb),
> + .header.cmd = UVC_CMD_LIST_SECRETS,
> + .start_idx = start_idx,
> + .list_addr = (u64)buf,
> + };
> + int cc = uv_call_sched(0, (u64)&uvcb);
> +
> + if (rc)
> + *rc = uvcb.header.rc;
> + if (rrc)
> + *rrc = uvcb.header.rrc;
> +
> + return cc;
> +}
> +
> struct uv_info {
> unsigned long inst_calls_list[4];
> unsigned long uv_base_stor_len;
> @@ -469,6 +594,10 @@ static inline int uv_remove_shared(unsigned long addr)
> return share(addr, UVC_CMD_REMOVE_SHARED_ACCESS);
> }
>
> +int uv_get_secret_metadata(const u8 secret_id[UV_SECRET_ID_LEN],
> + struct uv_secret_list_item_hdr *secret);
> +int uv_retrieve_secret(u16 secret_idx, u8 *buf, size_t buf_size);
> +
> extern int prot_virt_host;
>
> static inline int is_prot_virt_host(void)
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index 36db065c7cf7..090246efc1fa 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -786,3 +786,127 @@ static int __init uv_info_init(void)
> return rc;
> }
> device_initcall(uv_info_init);
> +
> +/*
> + * Find the secret with the secret_id in the provided list.
> + *
> + * Context: might sleep
> + */

The method comment formatting is in general a bit inconsistent. Sometimes with,
sometimes without newlines / dots.

> +static int find_secret_in_page(const u8 secret_id[UV_SECRET_ID_LEN],
> + const struct uv_secret_list *list,
> + struct uv_secret_list_item_hdr *secret)
> +{
> + u16 i;
> +
> + for (i = 0; i < list->total_num_secrets; i++) {
> + if (memcmp(secret_id, list->secrets[i].id, UV_SECRET_ID_LEN) == 0) {
> + *secret = list->secrets[i].hdr;
> + return 0;
> + }
> + }
> + return -ENOENT;
> +}

[...]

> +/**
> + * uv_get_secret_metadata() - get secret metadata for a given secret id
> + * @secret_id: search pattern
> + * @secret: output data, containing the secret's metadata
> + *
> + * Search for a secret with the given secret_id in the Ultravisor secret store.
> + *
> + * Context: might sleep
> + *
> + * Return:
> + * * %0: - Found entry; secret_idx and secret type are valid

You mean secret->index and secret->type?

> + * * %ENOENT - No entry found
> + * * %ENODEV: - Not supported: UV not available or command not available
> + * * %EIO: - Other unexpected UV error
> + */
> +int uv_get_secret_metadata(const u8 secret_id[UV_SECRET_ID_LEN],
> + struct uv_secret_list_item_hdr *secret)
> +{
> + struct uv_secret_list *buf;
> + int rc;
> +
> + buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> + rc = find_secret(secret_id, buf, secret);
> + kfree(buf);
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(uv_get_secret_metadata); …