Re: [PATCH v7 02/25] s390/ap/zcrypt: Rework AP message buffer allocation

[Heiko Carstens <hca@linux.ibm.com>]

On Wed, Apr 23, 2025 at 12:15:11PM +0200, Harald Freudenberger wrote:
> Slight rework on the way how AP message buffers are allocated.
> Instead of having multiple places with kmalloc() calls all
> the AP message buffers are now allocated and freed on exactly
> one place: ap_init_apmsg() allocates the current AP bus max
> limit of ap_max_msg_size (defaults to 12KB). The AP message
> buffer is then freed in ap_release_apmsg().
> 
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
> Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
> ---
>  drivers/s390/crypto/ap_bus.c           | 26 ++++++++++++++
>  drivers/s390/crypto/ap_bus.h           | 21 ++---------
>  drivers/s390/crypto/zcrypt_api.c       | 49 +++++++++++++++-----------
>  drivers/s390/crypto/zcrypt_msgtype50.c | 20 ++++++-----
>  drivers/s390/crypto/zcrypt_msgtype6.c  | 35 +++++++++---------
>  5 files changed, 86 insertions(+), 65 deletions(-)

...

> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> index 5020696f1379..78df25da0b95 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -642,16 +642,17 @@ static long zcrypt_rsa_modexpo(struct ap_perms *perms,
>  	struct zcrypt_queue *zq, *pref_zq;
>  	struct ap_message ap_msg;
>  	unsigned int wgt = 0, pref_wgt = 0;
> -	unsigned int func_code;
> -	int cpen, qpen, qid = 0, rc = -ENODEV;
> +	unsigned int func_code = 0;
> +	int cpen, qpen, qid = 0, rc;
>  	struct module *mod;
>  
>  	trace_s390_zcrypt_req(mex, TP_ICARSAMODEXPO);
>  
> -	ap_init_message(&ap_msg);
> +	rc = ap_init_apmsg(&ap_msg);
> +	if (rc)
> +		goto out;

clang says:

    CC [M]  drivers/s390/crypto/zcrypt_api.o
      drivers/s390/crypto/zcrypt_api.c:1223:6: warning: variable 'func_code' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
       1223 |         if (rc)
            |             ^~
      drivers/s390/crypto/zcrypt_api.c:1271:32: note: uninitialized use occurs here
       1271 |         trace_s390_zcrypt_rep(buffer, func_code, rc,
            |                                       ^~~~~~~~~
      drivers/s390/crypto/zcrypt_api.c:1223:2: note: remove the 'if' if its condition is always false
       1223 |         if (rc)
            |         ^~~~~~~
       1224 |                 goto out;
            |                 ~~~~~~~~
      drivers/s390/crypto/zcrypt_api.c:1214:24: note: initialize the variable 'func_code' to silence this warning
       1214 |         unsigned int func_code;
            |                               ^
            |                                = 0

Actually the problem that func_code may be used uninitialized existed before
your patch series, but now at least clang can prove it and complains.