[PATCH] crypto: drbg - convert to guard(mutex)

[PATCH] crypto: drbg - convert to guard(mutex)

[Rajveer Chaudhari]

Replaced old manual mutex locking/unlocking with
new safe guard(mutex) in drbg_instantiate().
This ensures mutex gets unlocked on every return and prevents deadlocks.

Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
---
 crypto/drbg.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 1d433dae9955..d52a7bd07322 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -103,6 +103,7 @@
 #include <linux/kernel.h>
 #include <linux/jiffies.h>
 #include <linux/string_choices.h>
+#include <linux/cleanup.h>
 
 /***************************************************************
  * Backend cipher definitions available to DRBG
@@ -1349,7 +1350,7 @@ static int drbg_instantiate(struct drbg_state *drbg, struct drbg_string *pers,
 
 	pr_devel("DRBG: Initializing DRBG core %d with prediction resistance "
 		 "%s\n", coreref, str_enabled_disabled(pr));
-	mutex_lock(&drbg->drbg_mutex);
+	guard(mutex)(&drbg->drbg_mutex);
 
 	/* 9.1 step 1 is implicit with the selected DRBG type */
 
@@ -1370,7 +1371,7 @@ static int drbg_instantiate(struct drbg_state *drbg, struct drbg_string *pers,
 
 		ret = drbg_alloc_state(drbg);
 		if (ret)
-			goto unlock;
+			return ret;
 
 		ret = drbg_prepare_hrng(drbg);
 		if (ret)
@@ -1384,15 +1385,9 @@ static int drbg_instantiate(struct drbg_state *drbg, struct drbg_string *pers,
 	if (ret && !reseed)
 		goto free_everything;
 
-	mutex_unlock(&drbg->drbg_mutex);
-	return ret;
-
-unlock:
-	mutex_unlock(&drbg->drbg_mutex);
 	return ret;
 
 free_everything:
-	mutex_unlock(&drbg->drbg_mutex);
 	drbg_uninstantiate(drbg);
 	return ret;
 }
-- 
2.52.0

Re: [PATCH] crypto: drbg - convert to guard(mutex)

[Herbert Xu]

On Sun, Feb 08, 2026 at 04:59:25AM +0530, Rajveer Chaudhari wrote:
> Replaced old manual mutex locking/unlocking with
> new safe guard(mutex) in drbg_instantiate().
> This ensures mutex gets unlocked on every return and prevents deadlocks.
> 
> Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
> ---
>  crypto/drbg.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/crypto/drbg.c b/crypto/drbg.c
> index 1d433dae9955..d52a7bd07322 100644
> --- a/crypto/drbg.c
> +++ b/crypto/drbg.c
> @@ -103,6 +103,7 @@
>  #include <linux/kernel.h>
>  #include <linux/jiffies.h>
>  #include <linux/string_choices.h>
> +#include <linux/cleanup.h>

Please keep the headers sorted alphabetically.

>  free_everything:
> -	mutex_unlock(&drbg->drbg_mutex);
>  	drbg_uninstantiate(drbg);
>  	return ret;

This is a subtle change and now drbg_uninstantiate will be called
within the critical section.  Are you sure this is safe?

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 - convert to guard(mutex)

[Rajveer Chaudhari]

On Sat, Feb 28, 2026 at 8:51 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Please keep the headers sorted alphabetically.
>
> >  free_everything:
> > -     mutex_unlock(&drbg->drbg_mutex);
> >       drbg_uninstantiate(drbg);
> >       return ret;
>
> This is a subtle change and now drbg_uninstantiate will be called
> within the critical section.  Are you sure this is safe?

Yes, this is safe. I traced through all functions called by
drbg_uninstantiate():

  - crypto_free_rng()
  - d_ops->crypto_fini()
  - drbg_dealloc_state()

None of them attempt to acquire drbg_mutex, so there is no risk of
deadlock. The mutex only coordinates thread access and does not
restrict memory access itself, so drbg_uninstantiate() can safely
access and free drbg fields while the lock is held.

In fact, holding the mutex during drbg_uninstantiate() is
more correct than the original, as it prevents another thread from
accessing the drbg state while it is being freed on the error path.

I will also fix the header ordering in v2.

Thanks for the review.

Regards,
Rajveer Chaudhari

Re: [PATCH] crypto: drbg - convert to guard(mutex)

[Herbert Xu]

On Sun, Mar 01, 2026 at 05:16:24PM +0530, Rajveer Chaudhari wrote:
>
> Yes, this is safe. I traced through all functions called by
> drbg_uninstantiate():

Even if it happens to be safe we still shouldn't do it unless it is
actually required for correctness.  We've had many dead-locks
caused by freeing resources while holding a mutex used for
allocation.
 
> In fact, holding the mutex during drbg_uninstantiate() is
> more correct than the original, as it prevents another thread from
> accessing the drbg state while it is being freed on the error path.

It's either correct or not.  If the mutex is required for
freeing resources please point out the race condition without
it.

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