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

[Steffen Eiden <seiden@linux.ibm.com>]

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!