gpg: Fix trustdb updates without lock held.
authorWerner Koch <wk@gnupg.org>
Mon, 26 Mar 2018 14:57:04 +0000 (16:57 +0200)
committerWerner Koch <wk@gnupg.org>
Mon, 26 Mar 2018 14:57:04 +0000 (16:57 +0200)
* g10/tdbio.c (is_locked): Turn into a counter.
(take_write_lock, release_write_lock): Implement recursive locks.
--

On trustdb creation we have this call sequence:

  init_trustdb                 -> takes lock
    tdbio_set_dbname
      create_version_record
       tdbio_write_record
         put_record_into_cache -> takes lock
         put_record_into_cache -> releases lock
  init_trustdb                 -> releases lock

The second take lock does noting but the first release lock has
already released the lock and the second release lock is a thus a NOP.
This is likely the cause for the corrupted trustdb as reported in

GnuPG-bug-id: 3839
Signed-off-by: Werner Koch <wk@gnupg.org>
g10/tdbio.c

index fb3cf15..4940c5c 100644 (file)
@@ -105,10 +105,11 @@ struct cmp_xdir_struct
 /* The name of the trustdb file.  */
 static char *db_name;
 
 /* The name of the trustdb file.  */
 static char *db_name;
 
-/* The handle for locking the trustdb file and a flag to record
-   whether a lock has been taken.  */
+/* The handle for locking the trustdb file and a counter to record how
+ * often this lock has been taken.  That counter is required becuase
+ * dotlock does not implemen recursive locks.  */
 static dotlock_t lockhandle;
 static dotlock_t lockhandle;
-static int is_locked;
+static unsigned int is_locked;
 
 /* The file descriptor of the trustdb.  */
 static int  db_fd = -1;
 
 /* The file descriptor of the trustdb.  */
 static int  db_fd = -1;
@@ -135,6 +136,8 @@ static void create_hashtable (ctrl_t ctrl, TRUSTREC *vr, int type);
 static int
 take_write_lock (void)
 {
 static int
 take_write_lock (void)
 {
+  int rc;
+
   if (!lockhandle)
     lockhandle = dotlock_create (db_name, 0);
   if (!lockhandle)
   if (!lockhandle)
     lockhandle = dotlock_create (db_name, 0);
   if (!lockhandle)
@@ -144,12 +147,16 @@ take_write_lock (void)
     {
       if (dotlock_take (lockhandle, -1) )
         log_fatal ( _("can't lock '%s'\n"), db_name );
     {
       if (dotlock_take (lockhandle, -1) )
         log_fatal ( _("can't lock '%s'\n"), db_name );
-      else
-        is_locked = 1;
-      return 0;
+      rc = 0;
     }
   else
     }
   else
-    return 1;
+    rc = 1;
+
+  if (opt.lock_once)
+    is_locked = 1;
+  else
+    is_locked++;
+  return rc;
 }
 
 
 }
 
 
@@ -160,10 +167,22 @@ take_write_lock (void)
 static void
 release_write_lock (void)
 {
 static void
 release_write_lock (void)
 {
-  if (!opt.lock_once)
-    if (!dotlock_release (lockhandle))
-      is_locked = 0;
+  if (opt.lock_once)
+    return;  /* Don't care; here IS_LOCKED is fixed to 1.  */
+
+  if (!is_locked)
+    {
+      log_error ("Ooops, tdbio:release_write_lock with no lock held\n");
+      return;
+    }
+  if (--is_locked)
+    return;
+
+  if (dotlock_release (lockhandle))
+    log_error ("Oops, tdbio:release_write_locked failed\n");
 }
 }
+
+
 \f
 /*************************************
  ************* record cache **********
 \f
 /*************************************
  ************* record cache **********