From 663c5d129a8f400cc6eb8ab7b91772d6e578152d Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 14 Jan 2016 20:45:33 +0100 Subject: [PATCH] w32: Fix deadlock introduced by keybox_file_rename. * g10/keyring.c (keyring_lock) [W32]: Flush the close cache before locking. * kbx/keybox-init.c (keybox_lock) [W32]: Close the file before locking. Signed-off-by: Werner Koch --- g10/keyring.c | 21 +++++++++++++++++---- kbx/keybox-init.c | 44 ++++++++++++++++++++++++++++++-------------- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/g10/keyring.c b/g10/keyring.c index 7ae50a392..ca9a69874 100644 --- a/g10/keyring.c +++ b/g10/keyring.c @@ -328,8 +328,20 @@ keyring_lock (KEYRING_HANDLE hd, int yes) if (!keyring_is_writable(kr)) continue; if (kr->is_locked) - ; - else if (dotlock_take (kr->lockhd, -1) ) { + continue; + +#ifdef HAVE_W32_SYSTEM + /* Under Windows we need to CloseHandle 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. */ + iobuf_ioctl (NULL, IOBUF_IOCTL_INVALIDATE_CACHE, 0, + (char*)kr->fname); +#endif /*HAVE_W32_SYSTEM*/ + if (dotlock_take (kr->lockhd, -1) ) { log_info ("can't lock '%s'\n", kr->fname ); rc = GPG_ERR_GENERAL; } @@ -343,8 +355,9 @@ keyring_lock (KEYRING_HANDLE hd, int yes) if (!keyring_is_writable(kr)) continue; if (!kr->is_locked) - ; - else if (dotlock_release (kr->lockhd)) + continue; + + if (dotlock_release (kr->lockhd)) log_info ("can't unlock '%s'\n", kr->fname ); else kr->is_locked = 0; diff --git a/kbx/keybox-init.c b/kbx/keybox-init.c index 01d29f0f6..3b53cd5ec 100644 --- a/kbx/keybox-init.c +++ b/kbx/keybox-init.c @@ -286,27 +286,43 @@ keybox_lock (KEYBOX_HANDLE hd, int yes) if (yes) /* Take the lock. */ { - if (kb->is_locked) - ; - else if (dotlock_take (kb->lockhd, -1)) + if (!kb->is_locked) { - err = gpg_error_from_syserror (); - log_info ("can't lock '%s'\n", kb->fname ); +#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; + } +#endif /*HAVE_W32_SYSTEM*/ + if (dotlock_take (kb->lockhd, -1)) + { + err = gpg_error_from_syserror (); + log_info ("can't lock '%s'\n", kb->fname ); + } + else + kb->is_locked = 1; } - else - kb->is_locked = 1; } else /* Release the lock. */ { - if (!kb->is_locked) - ; - else if (dotlock_release (kb->lockhd)) + if (kb->is_locked) { - err = gpg_error_from_syserror (); - log_info ("can't unlock '%s'\n", kb->fname ); + if (dotlock_release (kb->lockhd)) + { + err = gpg_error_from_syserror (); + log_info ("can't unlock '%s'\n", kb->fname ); + } + else + kb->is_locked = 0; } - else - kb->is_locked = 0; } return err;