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

Steffen Eiden
2025-04-16T08:59:06Z

On Tue, Apr 15, 2025 at 04:52:57PM +0200, Heiko Carstens wrote:
> 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.

Yes you do.

Thanks Heiko!