kbx: Fix deadlock in gpgsm on Windows due to a sharing violation.
authorWerner Koch <wk@gnupg.org>
Tue, 14 May 2019 17:05:58 +0000 (19:05 +0200)
committerWerner Koch <wk@gnupg.org>
Tue, 14 May 2019 17:05:58 +0000 (19:05 +0200)
* kbx/keybox-init.c (keybox_lock) [W32]: Use _keybox_close_file
instead of fclose so that a close is done if the file is opened by
another handle.
* kbx/keybox-search.c (keybox_search): Remember the last offset and
use that in NEXT search mode if we had to re-open the file.
--

GnuPG-bug-id: 4505
Signed-off-by: Werner Koch <wk@gnupg.org>
kbx/keybox-init.c
kbx/keybox-search.c
kbx/keybox-update.c

index 9a65c13..2223f4d 100644 (file)
@@ -291,18 +291,14 @@ keybox_lock (KEYBOX_HANDLE hd, int yes, long timeout)
       if (!kb->is_locked)
         {
 #ifdef HAVE_W32_SYSTEM
-            /* Under Windows we need to close the file before we try
-             * to lock it.  This is because another process might have
-             * taken the lock and is using keybox_file_rename to
-             * rename the base file.  How if our dotlock_take below is
-             * waiting for the lock but we have the base file still
-             * open, keybox_file_rename will never succeed as we are
-             * in a deadlock.  */
-          if (hd->fp)
-            {
-              fclose (hd->fp);
-              hd->fp = NULL;
-            }
+          /* Under Windows we need to close the file before we try
+           * to lock it.  This is because another process might have
+           * taken the lock and is using keybox_file_rename to
+           * rename the base file.  Now if our dotlock_take below is
+           * waiting for the lock but we have the base file still
+           * open, keybox_file_rename will never succeed as we are
+           * in a deadlock.  */
+          _keybox_close_file (hd);
 #endif /*HAVE_W32_SYSTEM*/
           if (dotlock_take (kb->lockhd, timeout))
             {
index 101e1b5..1600861 100644 (file)
@@ -873,16 +873,21 @@ keybox_search (KEYBOX_HANDLE hd, KEYBOX_SEARCH_DESC *desc, size_t ndesc,
   KEYBOXBLOB blob = NULL;
   struct sn_array_s *sn_array = NULL;
   int pk_no, uid_no;
+  off_t lastfoundoff;
 
   if (!hd)
     return gpg_error (GPG_ERR_INV_VALUE);
 
-  /* clear last found result */
+  /* Clear last found result but reord the offset of the last found
+   * blob which we may need later. */
   if (hd->found.blob)
     {
+      lastfoundoff = _keybox_get_blob_fileoffset (hd->found.blob);
       _keybox_release_blob (hd->found.blob);
       hd->found.blob = NULL;
     }
+  else
+    lastfoundoff = 0;
 
   if (hd->error)
     return hd->error; /* still in error state */
@@ -901,6 +906,7 @@ keybox_search (KEYBOX_HANDLE hd, KEYBOX_SEARCH_DESC *desc, size_t ndesc,
         case KEYDB_SEARCH_MODE_FIRST:
           /* always restart the search in this mode */
           keybox_search_reset (hd);
+          lastfoundoff = 0;
           break;
         default:
           break;
@@ -925,6 +931,32 @@ keybox_search (KEYBOX_HANDLE hd, KEYBOX_SEARCH_DESC *desc, size_t ndesc,
           xfree (sn_array);
           return rc;
         }
+      /* log_debug ("%s: re-opened file\n", __func__); */
+      if (ndesc && desc[0].mode == KEYDB_SEARCH_MODE_NEXT && lastfoundoff)
+        {
+          /* Search mode is next and the last search operation
+           * returned a blob which also was not the first one.  We now
+           * need to skip over that blob and hope that the file has
+           * not changed.  */
+          if (fseeko (hd->fp, lastfoundoff, SEEK_SET))
+            {
+              rc = gpg_error_from_syserror ();
+              log_debug ("%s: seeking to last found offset failed: %s\n",
+                         __func__, gpg_strerror (rc));
+              xfree (sn_array);
+              return gpg_error (GPG_ERR_NOTHING_FOUND);
+            }
+          /* log_debug ("%s: re-opened file and sought to last offset\n", */
+          /*            __func__); */
+          rc = _keybox_read_blob (NULL, hd->fp, NULL);
+          if (rc)
+            {
+              log_debug ("%s: skipping last found blob failed: %s\n",
+                         __func__, gpg_strerror (rc));
+              xfree (sn_array);
+              return gpg_error (GPG_ERR_NOTHING_FOUND);
+            }
+        }
     }
 
   /* Kludge: We need to convert an SN given as hexstring to its binary
index 580330f..e09fefc 100644 (file)
@@ -423,7 +423,7 @@ keybox_update_keyblock (KEYBOX_HANDLE hd, const void *image, size_t imagelen)
   if (off == (off_t)-1)
     return gpg_error (GPG_ERR_GENERAL);
 
-  /* Close this the file so that we do no mess up the position for a
+  /* Close the file so that we do no mess up the position for a
      next search.  */
   _keybox_close_file (hd);