From 6f72aa821407e47ad3963e72e139f2ca2c69d9dd Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 14 May 2019 19:05:58 +0200 Subject: [PATCH] kbx: Fix deadlock in gpgsm on Windows due to a sharing violation. * 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 --- kbx/keybox-init.c | 20 ++++++++------------ kbx/keybox-search.c | 34 +++++++++++++++++++++++++++++++++- kbx/keybox-update.c | 2 +- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/kbx/keybox-init.c b/kbx/keybox-init.c index 6a83f7162..6d656f2f8 100644 --- a/kbx/keybox-init.c +++ b/kbx/keybox-init.c @@ -289,18 +289,14 @@ keybox_lock (KEYBOX_HANDLE hd, int yes) 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, -1)) { diff --git a/kbx/keybox-search.c b/kbx/keybox-search.c index 1f5dbdf97..9c396449a 100644 --- a/kbx/keybox-search.c +++ b/kbx/keybox-search.c @@ -844,16 +844,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 */ @@ -872,6 +877,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; @@ -896,6 +902,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 diff --git a/kbx/keybox-update.c b/kbx/keybox-update.c index 580330f52..e09fefc41 100644 --- a/kbx/keybox-update.c +++ b/kbx/keybox-update.c @@ -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);