Re: [PATCH v5 22/25] s390/pkey: Use preallocated memory for retrieve of UV secret metadata

Heiko Carstens
2025-04-15T14:53:06Z

On Tue, Apr 15, 2025 at 04:24:35PM +0200, Harald Freudenberger wrote:
> The pkey uv functions may be called in a situation where memory
> allocations which trigger IO operations are not allowed. An example:
> decryption of the swap partition with protected key (PAES).
>
> The pkey uv code takes care of this by holding one preallocated
> struct uv_secret_list to be used with the new UV function
> uv_find_secret(). The older function uv_get_secret_metadata()
> used before always allocates/frees an ephemeral memory buffer.
> The preallocated struct is concurrency protected by a mutex.
>
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
> Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
> Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
> drivers/s390/crypto/pkey_uv.c | 37 ++++++++++++++++++++++++++++++++---
> 1 file changed, 34 insertions(+), 3 deletions(-)

...

> +static int get_secret_metadata(const u8 secret_id[UV_SECRET_ID_LEN],
> + struct uv_secret_list_item_hdr *secret)
> +{
> + int rc;
> +
> + mutex_lock(&uv_list_mutex);
> + rc = uv_find_secret(secret_id, uv_list, secret);
> + mutex_unlock(&uv_list_mutex);
> +
> + return rc;
> +}

This is a change to the previous code, and potentially is also a problem:
uvlist is not set to zero before it is used (unlike before). The condition
code of the uvc is partially ignored, and instead the return code of the
response is taken into account to tell if the uvc succeeded. This works since
memory was initialized, but now the contents of the previous invocation may be
used to tell if the uvc succeeded.

Or in other words: I think you need to add a memset() call to uv_find_secret()
before uv_list is used for the uvc.