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

Heiko Carstens
2025-04-24T06:45:46Z

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.