linux-kernel - Re: [PATCH 05/12] s390: zcrypt: initialize variables before

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 9 Apr 2019 11:54:30 +0200
From:   Harald Freudenberger <freude@...ux.ibm.com>
To:     Arnd Bergmann <arnd@...db.de>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>
Cc:     clang-built-linux@...glegroups.com,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Nathan Chancellor <natechancellor@...il.com>,
        linux-s390@...r.kernel.org, Ingo Franzki <ifranzki@...ibm.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/12] s390: zcrypt: initialize variables before_use

On 08.04.19 23:26, Arnd Bergmann wrote:
> The 'func_code' variable gets printed in debug statements without
> a prior initialization in multiple functions, as reported when building
> with clang:
>
> drivers/s390/crypto/zcrypt_api.c:659:6: warning: variable 'func_code' is used uninitialized whenever 'if' condition is true
>       [-Wsometimes-uninitialized]
>         if (mex->outputdatalength < mex->inputdatalength) {
>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/crypto/zcrypt_api.c:725:29: note: uninitialized use occurs here
>         trace_s390_zcrypt_rep(mex, func_code, rc,
>                                    ^~~~~~~~~
> drivers/s390/crypto/zcrypt_api.c:659:2: note: remove the 'if' if its condition is always false
>         if (mex->outputdatalength < mex->inputdatalength) {
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/crypto/zcrypt_api.c:654:24: note: initialize the variable 'func_code' to silence this warning
>         unsigned int func_code;
>                               ^
>
> Add initializations to all affected code paths to shut up the warning
> and make the warning output consistent.
>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  drivers/s390/crypto/zcrypt_api.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
> index eb93c2d27d0a..23472063d9a8 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -657,6 +657,7 @@ static long zcrypt_rsa_modexpo(struct ap_perms *perms,
>  	trace_s390_zcrypt_req(mex, TP_ICARSAMODEXPO);
>  
>  	if (mex->outputdatalength < mex->inputdatalength) {
> +		func_code = -1;
>  		rc = -EINVAL;
>  		goto out;
>  	}
> @@ -739,6 +740,7 @@ static long zcrypt_rsa_crt(struct ap_perms *perms,
>  	trace_s390_zcrypt_req(crt, TP_ICARSACRT);
>  
>  	if (crt->outputdatalength < crt->inputdatalength) {
> +		func_code = -1;
>  		rc = -EINVAL;
>  		goto out;
>  	}
> @@ -946,6 +948,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
>  
>  		targets = kcalloc(target_num, sizeof(*targets), GFP_KERNEL);
>  		if (!targets) {
> +			func_code = -1;
>  			rc = -ENOMEM;
>  			goto out;
>  		}
> @@ -953,6 +956,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
>  		uptr = (struct ep11_target_dev __force __user *) xcrb->targets;
>  		if (copy_from_user(targets, uptr,
>  				   target_num * sizeof(*targets))) {
> +			func_code = -1;
>  			rc = -EFAULT;
>  			goto out_free;
>  		}
Thanks Arnd, but as Nathan already wrote, I'd prefer to have the
variable initialized with 0 instead of -1.
If you agree with this, I'll rewrite the patch and apply it to our
internal git and it will appear at kernel org with the next s390 code merge then.
regards Harald Freudenberger