Page MenuHome GnuPG

libgcrypt: Simply use BSS (not secure heap) for DRBG instance
Closed, ResolvedPublic

Description

In random/random-drbg.c, drbg_state is allocated by xtrycalloc_secure, but since it is only a single instance and no need to be in secure memory (structure has only flags and addresses to actual data), it is rather better to use BSS area for its memory, outside the management of xtrycalloc/xfree.

Event Timeline

Here is my proposal patch:

diff --git a/random/random-drbg.c b/random/random-drbg.c
index 5a46fd92..f1cfe286 100644
--- a/random/random-drbg.c
+++ b/random/random-drbg.c
@@ -341,6 +341,9 @@ enum drbg_prefixes
  * Global variables
  ***************************************************************/
 
+/* The instance of the DRBG, to be refereed by drbg_state.  */
+static struct drbg_state_s drbg_instance;
+
 /* Global state variable holding the current instance of the DRBG.  */
 static drbg_state_t drbg_state;
 
@@ -1783,9 +1786,7 @@ _drbg_init_internal (u32 flags, drbg_string_t *pers)
     }
   else
     {
-      drbg_state = xtrycalloc_secure (1, sizeof *drbg_state);
-      if (!drbg_state)
-	return gpg_err_code_from_syserror ();
+      drbg_state = &drbg_instance;
     }
   if (flags & DRBG_PREDICTION_RESIST)
     pr = 1;
@@ -1879,7 +1880,6 @@ _gcry_rngdrbg_close_fds (void)
   if (drbg_state)
     {
       drbg_uninstantiate (drbg_state);
-      xfree (drbg_state);
       drbg_state = NULL;
     }
   drbg_unlock ();
werner triaged this task as Normal priority.Apr 20 2022, 8:45 AM
werner removed a project: Bug Report.
werner added a subscriber: werner.

Full ack.

werner changed the task status from Open to Testing.Sep 22 2022, 11:02 AM
werner removed a project: Restricted Project.