[PATCH] crypto: x86/sha1: fix coverity CID 1195603

[PATCH] crypto: x86/sha1: fix coverity CID 1195603

[Milos Vyletel]

Coverity detected possible use of uninitialized pointer when printing info
message during module load. While this is higly unlikely to cause any troubles
simple change in sha1_ssse3_mod_init to make it look like sha256/512 init
function will fix this.

260
   	6. Condition sha1_transform_asm, taking true branch
261        if (sha1_transform_asm) {

CID 1195603 (#1 of 1): Uninitialized pointer read (UNINIT)
7. uninit_use_in_call: Using uninitialized value algo_name when calling printk.
262                pr_info("Using %s optimized SHA-1 implementation\n", algo_name);
263                return crypto_register_shash(&alg);
264        }

Reported-by: <scan-admin@coverity.com>
Signed-off-by: Milos Vyletel <milos.vyletel@gmail.com>
---
 arch/x86/crypto/sha1_ssse3_glue.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/x86/crypto/sha1_ssse3_glue.c b/arch/x86/crypto/sha1_ssse3_glue.c
index 74d16ef..5352196 100644
--- a/arch/x86/crypto/sha1_ssse3_glue.c
+++ b/arch/x86/crypto/sha1_ssse3_glue.c
@@ -235,31 +235,33 @@ static bool __init avx2_usable(void)
 
 static int __init sha1_ssse3_mod_init(void)
 {
-	char *algo_name;
-
 	/* test for SSSE3 first */
-	if (cpu_has_ssse3) {
+	if (cpu_has_ssse3)
 		sha1_transform_asm = sha1_transform_ssse3;
-		algo_name = "SSSE3";
-	}
 
 #ifdef CONFIG_AS_AVX
 	/* allow AVX to override SSSE3, it's a little faster */
 	if (avx_usable()) {
 		sha1_transform_asm = sha1_transform_avx;
-		algo_name = "AVX";
 #ifdef CONFIG_AS_AVX2
 		/* allow AVX2 to override AVX, it's a little faster */
-		if (avx2_usable()) {
+		if (avx2_usable())
 			sha1_transform_asm = sha1_apply_transform_avx2;
-			algo_name = "AVX2";
-		}
 #endif
 	}
 #endif
 
 	if (sha1_transform_asm) {
-		pr_info("Using %s optimized SHA-1 implementation\n", algo_name);
+#ifdef CONFIG_AS_AVX
+		if (sha1_transform_asm == sha1_transform_avx)
+			pr_info("Using AVX optimized SHA-1 implementation\n");
+#ifdef CONFIG_AS_AVX2
+		else if (sha1_transform_asm == sha1_transform_avx2)
+			pr_info("Using AVX2 optimized SHA-1 implementation\n");
+#endif
+		else
+#endif
+			pr_info("Using SSSE3 optimized SHA-1 implementation\n");
 		return crypto_register_shash(&alg);
 	}
 	pr_info("Neither AVX nor AVX2 nor SSSE3 is available/usable.\n");
-- 
1.9.0

Re: [PATCH] crypto: x86/sha1: fix coverity CID 1195603

[Pavel Machek]

On Wed 2014-04-30 15:17:54, Milos Vyletel wrote:
> Coverity detected possible use of uninitialized pointer when printing info
> message during module load. While this is higly unlikely to cause any troubles
> simple change in sha1_ssse3_mod_init to make it look like sha256/512 init
> function will fix this.
> 
> 260
>    	6. Condition sha1_transform_asm, taking true branch
> 261        if (sha1_transform_asm) {
> 
> CID 1195603 (#1 of 1): Uninitialized pointer read (UNINIT)
> 7. uninit_use_in_call: Using uninitialized value algo_name when calling printk.
> 262                pr_info("Using %s optimized SHA-1 implementation\n", algo_name);
> 263                return crypto_register_shash(&alg);
> 264        }
> 
> Reported-by: <scan-admin@coverity.com>
> Signed-off-by: Milos Vyletel <milos.vyletel@gmail.com>
> ---
>  arch/x86/crypto/sha1_ssse3_glue.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/crypto/sha1_ssse3_glue.c b/arch/x86/crypto/sha1_ssse3_glue.c
> index 74d16ef..5352196 100644
> --- a/arch/x86/crypto/sha1_ssse3_glue.c
> +++ b/arch/x86/crypto/sha1_ssse3_glue.c
> @@ -235,31 +235,33 @@ static bool __init avx2_usable(void)
>  
>  static int __init sha1_ssse3_mod_init(void)
>  {
> -	char *algo_name;
> -

Would simple "algo_name = NULL" be enough to fix this?

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] crypto: x86/sha1: fix coverity CID 1195603

[Milos Vyletel]

And this time in plain text...

Most likely yes but I wanted to keep sha1_ssse3_mod_init consistent
with sha256_ssse3_mod_init/sha512_ssse3_mod_init functions.

Milos

On Tue, May 6, 2014 at 4:28 AM, Pavel Machek <pavel@ucw.cz> wrote:
> On Wed 2014-04-30 15:17:54, Milos Vyletel wrote:
>> Coverity detected possible use of uninitialized pointer when printing info
>> message during module load. While this is higly unlikely to cause any troubles
>> simple change in sha1_ssse3_mod_init to make it look like sha256/512 init
>> function will fix this.
>>
>> 260
>>       6. Condition sha1_transform_asm, taking true branch
>> 261        if (sha1_transform_asm) {
>>
>> CID 1195603 (#1 of 1): Uninitialized pointer read (UNINIT)
>> 7. uninit_use_in_call: Using uninitialized value algo_name when calling printk.
>> 262                pr_info("Using %s optimized SHA-1 implementation\n", algo_name);
>> 263                return crypto_register_shash(&alg);
>> 264        }
>>
>> Reported-by: <scan-admin@coverity.com>
>> Signed-off-by: Milos Vyletel <milos.vyletel@gmail.com>
>> ---
>>  arch/x86/crypto/sha1_ssse3_glue.c | 22 ++++++++++++----------
>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/x86/crypto/sha1_ssse3_glue.c b/arch/x86/crypto/sha1_ssse3_glue.c
>> index 74d16ef..5352196 100644
>> --- a/arch/x86/crypto/sha1_ssse3_glue.c
>> +++ b/arch/x86/crypto/sha1_ssse3_glue.c
>> @@ -235,31 +235,33 @@ static bool __init avx2_usable(void)
>>
>>  static int __init sha1_ssse3_mod_init(void)
>>  {
>> -     char *algo_name;
>> -
>
> Would simple "algo_name = NULL" be enough to fix this?
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] crypto: x86/sha1: fix coverity CID 1195603

[Pavel Machek]

Hi!

> Most likely yes but I wanted to keep sha1_ssse3_mod_init consistent
> with sha256_ssse3_mod_init/sha512_ssse3_mod_init functions.


> > > Reported-by: <scan-admin@coverity.com>
> > > Signed-off-by: Milos Vyletel <milos.vyletel@gmail.com>
> > > ---
> > >  arch/x86/crypto/sha1_ssse3_glue.c | 22 ++++++++++++----------
> > >  1 file changed, 12 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/arch/x86/crypto/sha1_ssse3_glue.c
> > b/arch/x86/crypto/sha1_ssse3_glue.c
> > > index 74d16ef..5352196 100644
> > > --- a/arch/x86/crypto/sha1_ssse3_glue.c
> > > +++ b/arch/x86/crypto/sha1_ssse3_glue.c
> > > @@ -235,31 +235,33 @@ static bool __init avx2_usable(void)
> > >
> > >  static int __init sha1_ssse3_mod_init(void)
> > >  {
> > > -     char *algo_name;
> > > -
> >
> > Would simple "algo_name = NULL" be enough to fix this?

Dunno. I'd say that single-line change that is obviously right is
preferable to rewrite inspired by some other function.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] crypto: x86/sha1: fix coverity CID 1195603

[Herbert Xu]

On Wed, Apr 30, 2014 at 03:17:54PM -0400, Milos Vyletel wrote:
> Coverity detected possible use of uninitialized pointer when printing info
> message during module load. While this is higly unlikely to cause any troubles
> simple change in sha1_ssse3_mod_init to make it look like sha256/512 init
> function will fix this.
> 
> 260
>    	6. Condition sha1_transform_asm, taking true branch
> 261        if (sha1_transform_asm) {
> 
> CID 1195603 (#1 of 1): Uninitialized pointer read (UNINIT)
> 7. uninit_use_in_call: Using uninitialized value algo_name when calling printk.
> 262                pr_info("Using %s optimized SHA-1 implementation\n", algo_name);
> 263                return crypto_register_shash(&alg);
> 264        }
> 
> Reported-by: <scan-admin@coverity.com>
> Signed-off-by: Milos Vyletel <milos.vyletel@gmail.com>

Unless I'm missing something there is no way this code can use
the variable without initialising it.

So this is a false positive and I'm not applying this.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: x86/sha1: fix coverity CID 1195603

[Marek Vasut]

On Thursday, May 08, 2014 at 03:30:25 PM, Herbert Xu wrote:
> On Wed, Apr 30, 2014 at 03:17:54PM -0400, Milos Vyletel wrote:
> > Coverity detected possible use of uninitialized pointer when printing
> > info message during module load. While this is higly unlikely to cause
> > any troubles simple change in sha1_ssse3_mod_init to make it look like
> > sha256/512 init function will fix this.
> > 
> > 260
> > 
> >    	6. Condition sha1_transform_asm, taking true branch
> > 
> > 261        if (sha1_transform_asm) {
> > 
> > CID 1195603 (#1 of 1): Uninitialized pointer read (UNINIT)
> > 7. uninit_use_in_call: Using uninitialized value algo_name when calling
> > printk. 262                pr_info("Using %s optimized SHA-1
> > implementation\n", algo_name); 263                return
> > crypto_register_shash(&alg);
> > 264        }
> > 
> > Reported-by: <scan-admin@coverity.com>
> > Signed-off-by: Milos Vyletel <milos.vyletel@gmail.com>
> 
> Unless I'm missing something there is no way this code can use
> the variable without initialising it.
> 
> So this is a false positive and I'm not applying this.

I suppose changing the commit message to "align the code with sha256 ... NOTE: 
this also fixed CIDxyz." would work better and might get this applied ? I think 
unification of code is always good.

Best regards,
Marek Vasut