From 7c73db3d31c6457dfbdc82a8dc89951c023f0603 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 18 Oct 2017 18:28:52 +0200 Subject: [PATCH] gpg: Keep a lock during the read-update/insert cycle in import. * 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 | 15 +++++++++++++-- g10/keydb.c | 25 ++++++++++++++++++++++++- g10/keydb.h | 4 ++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/g10/getkey.c b/g10/getkey.c index 5ce580541..c58e8ff2c 100644 --- a/g10/getkey.c +++ b/g10/getkey.c @@ -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) { diff --git a/g10/keydb.c b/g10/keydb.c index 0f28bc301..58a14a83d 100644 --- a/g10/keydb.c +++ b/g10/keydb.c @@ -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--) diff --git a/g10/keydb.h b/g10/keydb.h index b173751ca..739376838 100644 --- a/g10/keydb.h +++ b/g10/keydb.h @@ -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. */