gpg: Keep a lock during the read-update/insert cycle in import.
authorWerner Koch <wk@gnupg.org>
Wed, 18 Oct 2017 16:28:52 +0000 (18:28 +0200)
committerWerner Koch <wk@gnupg.org>
Thu, 19 Oct 2017 13:02:28 +0000 (15:02 +0200)
* g10/keydb.c (keydb_handle): New field 'keep_lock'.
(keydb_release): Clear that flag.
(keydb_lock): New function.
(unlock_all): Skip if KEEP_LOCK is set.
* g10/getkey.c (get_keyblock_byfprint_fast): Call keep_lock if
requested.
--

That change is straightforward.  It helps to avoid the race condition
that another gpg process inserts a key while the first process is
between the search and the insert.

A similar change is due for gpgsm.

Note that the key edit operations may still suffer from a race.

GnuPG-bug-id: 3446

g10/getkey.c
g10/keydb.c
g10/keydb.h

index 5ce5805..c58e8ff 100644 (file)
@@ -1877,14 +1877,25 @@ get_keyblock_byfprint_fast (kbnode_t *r_keyblock, KEYDB_HANDLE *r_hd,
   hd = keydb_new ();
   if (!hd)
     return gpg_error_from_syserror ();
-  if (r_hd)
-    *r_hd = hd;
 
   if (lock)
     {
+      err = keydb_lock (hd);
+      if (err)
+        {
+          /* If locking did not work, we better don't return a handle
+           * at all - there was a reason that locking has been
+           * requested.  */
+          keydb_release (hd);
+          return err;
+        }
       keydb_disable_caching (hd);
     }
 
+  /* Fo all other errors we return the handle.  */
+  if (r_hd)
+    *r_hd = hd;
+
   err = keydb_search_fpr (hd, fprbuf);
   if (gpg_err_code (err) == GPG_ERR_NOT_FOUND)
     {
index 0f28bc3..58a14a8 100644 (file)
@@ -96,6 +96,10 @@ struct keydb_handle
      / keybox_lock, as appropriate).  */
   int locked;
 
+  /* If this flag is set a lock will only be released by
+   * keydb_release.  */
+  int keep_lock;
+
   /* The index into ACTIVE of the resources in which the last search
      result was found.  Initially -1.  */
   int found;
@@ -964,6 +968,7 @@ keydb_release (KEYDB_HANDLE hd)
   log_assert (active_handles > 0);
   active_handles--;
 
+  hd->keep_lock = 0;
   unlock_all (hd);
   for (i=0; i < hd->used; i++)
     {
@@ -985,6 +990,24 @@ keydb_release (KEYDB_HANDLE hd)
 }
 
 
+/* Take a lock on the files immediately and not only during insert or
+ * update.  This lock is released with keydb_release.  */
+gpg_error_t
+keydb_lock (KEYDB_HANDLE hd)
+{
+  gpg_error_t err;
+
+  if (!hd)
+    return gpg_error (GPG_ERR_INV_ARG);
+
+  err = lock_all (hd);
+  if (!err)
+    hd->keep_lock = 1;
+
+  return err;
+}
+
+
 /* Set a flag on the handle to suppress use of cached results.  This
  * is required for updating a keyring and for key listings.  Fixme:
  * Using a new parameter for keydb_new might be a better solution.  */
@@ -1098,7 +1121,7 @@ unlock_all (KEYDB_HANDLE hd)
 {
   int i;
 
-  if (!hd->locked)
+  if (!hd->locked || hd->keep_lock)
     return;
 
   for (i=hd->used-1; i >= 0; i--)
index b173751..7393768 100644 (file)
@@ -154,6 +154,10 @@ KEYDB_HANDLE keydb_new (void);
 /* Free all resources owned by the database handle.  */
 void keydb_release (KEYDB_HANDLE hd);
 
+/* Take a lock on the files immediately and not only during insert or
+ * update.  This lock is released with keydb_release.  */
+gpg_error_t keydb_lock (KEYDB_HANDLE hd);
+
 /* Set a flag on the handle to suppress use of cached results.  This
    is required for updating a keyring and for key listings.  Fixme:
    Using a new parameter for keydb_new might be a better solution.  */