linux-kernel - Re: [PATCH v4 2/6] s390/uv: Retrieve UV secrets support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <76f80f85-0dbb-425d-8390-521d0a74d9ee@linux.ibm.com>
Date: Tue, 22 Oct 2024 14:16:51 +0200
From: Janosch Frank <frankja@...ux.ibm.com>
To: Steffen Eiden <seiden@...ux.ibm.com>, linux-kernel@...r.kernel.org,
        linux-s390@...r.kernel.org
Cc: Ingo Franzki <ifranzki@...ux.ibm.com>,
        Harald Freudenberger <freude@...ux.ibm.com>,
        Christoph Schlameuss <schlameuss@...ux.ibm.com>,
        Claudio Imbrenda <imbrenda@...ux.ibm.com>
Subject: Re: [PATCH v4 2/6] s390/uv: Retrieve UV secrets support

On 10/18/24 11:15 AM, 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@...ux.ibm.com>
> ---

[...]

> +/**
> + * uv_secret_list_item_hdr - UV secret metadata.
> + * @index: Index of the secret in the secret list.
> + * @type: Type of the secret. See `enum uv_secret_types`.
> + * @length: Length of the stored secret.
> + */
> +struct uv_secret_list_item_hdr {
> +	u16 index;
> +	u16 type;
> +	u32 length;
> +};
> +
> +#define UV_SECRET_ID_LEN 32
> +/**
> + * uv_secret_list_item - UV secret entry.
> + * @hdr: The metadata of this secret.
> + * @id: The ID of this secret, not the secret itself.
> + */
> +struct uv_secret_list_item {
> +	struct uv_secret_list_item_hdr hdr;
> +	u64 reserverd08;
> +	u8 id[UV_SECRET_ID_LEN];
> +};

Are you skipping the size asserts since the list is asserted?
It might still make sense to pack them, no?

>   static inline int __uv_call(unsigned long r1, unsigned long r2)
>   {
>   	int cc;
> @@ -383,6 +469,47 @@ 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_secret_idx` for `start_idx`.
> + *
> + *  Context: might sleep.
> + *
> + *  Return: The UVC condition code.
> + */
> +static inline int uv_list_secrets(u8 *buf, u16 start_idx, u16 *rc, u16 *rrc)
>

Why is buf (u8 *) if you have it as (struct uv_secret_list *) in 
find_secret()?

You have a second caller in list_secrets() but that can also be (struct 
uv_secret_list *) since copy_to_user() shouldn't care and you need to 
cast it anyway for alloc_page().

If you'd be passing buf as u64 and not as a pointer it would make sense 
but you're casting it to u64 here