[PATCH] crypto: drbg - drop useless check in drbg_get_random_bytes()

[PATCH] crypto: drbg - drop useless check in drbg_get_random_bytes()

[Sergey Shtylyov]

drbg_fips_continuous_test() only returns 0 and -EAGAIN, so an early return
from drbg_get_random_bytes() could never happen, so we can drop the result
check from the *do/while* loop...

Found by Linux Verification Center (linuxtesting.org) with the Svace static
analysis tool.

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

---
The patch is against the master branch of Linus Torvalds' linux.git repo
(I'm unable to use the other repos on git.kernel.org and I have to update
Linus' repo from GitHub).

 crypto/drbg.c |    2 --
 1 file changed, 2 deletions(-)

Index: linux/crypto/drbg.c
===================================================================
--- linux.orig/crypto/drbg.c
+++ linux/crypto/drbg.c
@@ -1067,8 +1067,6 @@ static inline int drbg_get_random_bytes(
 	do {
 		get_random_bytes(entropy, entropylen);
 		ret = drbg_fips_continuous_test(drbg, entropy);
-		if (ret && ret != -EAGAIN)
-			return ret;
 	} while (ret);
 
 	return 0;

Re: [PATCH] crypto: drbg - drop useless check in drbg_get_random_bytes()

[Herbert Xu]

On Sun, Sep 21, 2025 at 11:33:36PM +0300, Sergey Shtylyov wrote:
> drbg_fips_continuous_test() only returns 0 and -EAGAIN, so an early return
> from drbg_get_random_bytes() could never happen, so we can drop the result
> check from the *do/while* loop...
> 
> Found by Linux Verification Center (linuxtesting.org) with the Svace static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> ---
> The patch is against the master branch of Linus Torvalds' linux.git repo
> (I'm unable to use the other repos on git.kernel.org and I have to update
> Linus' repo from GitHub).
> 
>  crypto/drbg.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> Index: linux/crypto/drbg.c
> ===================================================================
> --- linux.orig/crypto/drbg.c
> +++ linux/crypto/drbg.c
> @@ -1067,8 +1067,6 @@ static inline int drbg_get_random_bytes(
>  	do {
>  		get_random_bytes(entropy, entropylen);
>  		ret = drbg_fips_continuous_test(drbg, entropy);
> -		if (ret && ret != -EAGAIN)
> -			return ret;

That's a bug waiting to happen.  Does the compiler actually fail
to optimise away?

If you can show that this makes a difference to the compiled output,
then please change the drbg_fips_continuous_test function signature
so that it can no longer return an error.

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: drbg - drop useless check in drbg_get_random_bytes()

[Yann Droneaud]

Hi,

Le 21/09/2025 à 22:33, Sergey Shtylyov a écrit :
> Index: linux/crypto/drbg.c
> ===================================================================
> --- linux.orig/crypto/drbg.c
> +++ linux/crypto/drbg.c
> @@ -1067,8 +1067,6 @@ static inline int drbg_get_random_bytes(
>   	do {
>   		get_random_bytes(entropy, entropylen);
>   		ret = drbg_fips_continuous_test(drbg, entropy);
> -		if (ret && ret != -EAGAIN)
> -			return ret;
>   	} while (ret);

} while (ret == EAGAIN);

return ret;


-- 

Yann Droneaud

Re: [PATCH] crypto: drbg - drop useless check in drbg_get_random_bytes()

[Sergey Shtylyov]

On 9/22/25 9:19 AM, Yann Droneaud wrote:
[...]

>> Index: linux/crypto/drbg.c
>> ===================================================================
>> --- linux.orig/crypto/drbg.c
>> +++ linux/crypto/drbg.c
>> @@ -1067,8 +1067,6 @@ static inline int drbg_get_random_bytes(
>>       do {
>>           get_random_bytes(entropy, entropylen);
>>           ret = drbg_fips_continuous_test(drbg, entropy);
>> -        if (ret && ret != -EAGAIN)
>> -            return ret;
>>       } while (ret);
> 
> } while (ret == EAGAIN);
> 
> return ret;

   Yeah, that's more clear, thanks! I'll mention you in the Suggested-by tag
when I recast. :-)

MBR, Sergey

Re: [PATCH] crypto: drbg - drop useless check in drbg_get_random_bytes()

[Sergey Shtylyov]

On 9/22/25 4:39 AM, Herbert Xu wrote:

>> drbg_fips_continuous_test() only returns 0 and -EAGAIN, so an early return
>> from drbg_get_random_bytes() could never happen, so we can drop the result
>> check from the *do/while* loop...
>>
>> Found by Linux Verification Center (linuxtesting.org) with the Svace static
>> analysis tool.
>>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>
>> ---
>> The patch is against the master branch of Linus Torvalds' linux.git repo
>> (I'm unable to use the other repos on git.kernel.org and I have to update
>> Linus' repo from GitHub).
>>
>>  crypto/drbg.c |    2 --
>>  1 file changed, 2 deletions(-)
>>
>> Index: linux/crypto/drbg.c
>> ===================================================================
>> --- linux.orig/crypto/drbg.c
>> +++ linux/crypto/drbg.c
>> @@ -1067,8 +1067,6 @@ static inline int drbg_get_random_bytes(
>>  	do {
>>  		get_random_bytes(entropy, entropylen);
>>  		ret = drbg_fips_continuous_test(drbg, entropy);
>> -		if (ret && ret != -EAGAIN)
>> -			return ret;
> 
> That's a bug waiting to happen.  Does the compiler actually fail

   You mean the change is risky WRT the drbg_fips_continuous_test() changes?

> to optimise away?

   Looks like both gcc (14.3.1) and clang (19.1.7) are able to optimize away
this *if*...

> If you can show that this makes a difference to the compiled output,

   It does, however the code (from my cursory look) seems to get even worse,
as *return* -EINVAL generates actual mov, at least with gcc 14.3.1...
   However, it's clear that drbg_get_random_bytes() can be made shorter/clearer
(see the other reply), so I'm going to recast.

> then please change the drbg_fips_continuous_test function signature
> so that it can no longer return an error.

   I didn't understand what change you meant here...

> Thanks,

MBR, Sergey

Re: [PATCH] crypto: drbg - drop useless check in drbg_get_random_bytes()

[Herbert Xu]

On Mon, Sep 22, 2025 at 10:43:59PM +0300, Sergey Shtylyov wrote:
>
> > then please change the drbg_fips_continuous_test function signature
> > so that it can no longer return an error.
> 
>    I didn't understand what change you meant here...

Make it return a boolean instead of an int.

Cheers,
-- 
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: drbg - drop useless check in drbg_get_random_bytes()

[Sergey Shtylyov]

On 9/23/25 4:33 AM, Herbert Xu wrote:

>>> then please change the drbg_fips_continuous_test function signature
>>> so that it can no longer return an error.
>>
>>    I didn't understand what change you meant here...
> 
> Make it return a boolean instead of an int.

   That means drbg_get_random_bytes() can be made *void* inst6ead of *int*...
I guess I should do a series 2 patches then?

> Cheers,

MBR, Sergey

Re: [PATCH] crypto: drbg - drop useless check in drbg_get_random_bytes()

[Sergey Shtylyov]

On 12/12/25 11:30 PM, Sergey Shtylyov wrote:
[...]

>>>> then please change the drbg_fips_continuous_test function signature
>>>> so that it can no longer return an error.
>>>
>>>    I didn't understand what change you meant here...
>>
>> Make it return a boolean instead of an int.
> 
>    That means drbg_get_random_bytes() can be made *void* inst6ead of *int*...
> I guess I should do a series 2 patches then?

    A series of 2 patches, of course. :-)
>> Cheers,

MBR, Sergey