We observe a file is still open when unlock operation is done.
This may cause troubles.
IIUC, I identified the bug.
Description
Description
Revisions and Commits
Revisions and Commits
| Status | Assigned | Task | ||
|---|---|---|---|---|
| Testing | • ikloecker | T7827 Kleopatra: Add workaround for locking issue on key generation | ||
| Testing | None | T2196 keydb locking can result in deadlock in 2.2 | ||
| Open | • gniibe | T7855 keybox/keydb locking issue in 2.6 |
Event Timeline
Comment Actions
Here are places where I found problems.
diff --git a/g10/keydb.c b/g10/keydb.c index 590b45713..3dbfbbc82 100644 --- a/g10/keydb.c +++ b/g10/keydb.c @@ -766,10 +766,11 @@ keydb_add_resource (const char *url, unsigned int flags) if (!keybox_lock (kbxhd, 1, 0)) { keybox_compress (kbxhd); + keybox_release (kbxhd); keybox_lock (kbxhd, 0, 0); } - - keybox_release (kbxhd); + else + keybox_release (kbxhd); } } used_resources++; @@ -907,7 +908,6 @@ internal_keydb_deinit (KEYDB_HANDLE hd) active_handles--; hd->keep_lock = 0; - unlock_all (hd); for (i=0; i < hd->used; i++) { switch (hd->active[i].type) @@ -922,7 +922,7 @@ internal_keydb_deinit (KEYDB_HANDLE hd) break; } } - + unlock_all (hd); keyblock_cache_clear (hd); } diff --git a/sm/keydb.c b/sm/keydb.c index 1f93268a8..0aa5c27e8 100644 --- a/sm/keydb.c +++ b/sm/keydb.c @@ -460,10 +460,11 @@ keydb_add_resource (ctrl_t ctrl, const char *url, int force, int *auto_created) if (!keybox_lock (kbxhd, 1, 0)) { keybox_compress (kbxhd); + keybox_release (kbxhd); keybox_lock (kbxhd, 0, 0); } - - keybox_release (kbxhd); + else + keybox_release (kbxhd); } used_resources++; @@ -695,7 +696,6 @@ keydb_release (KEYDB_HANDLE hd) else { hd->keep_lock = 0; - unlock_all (hd); for (i=0; i < hd->used; i++) { switch (hd->active[i].type) @@ -707,6 +707,7 @@ keydb_release (KEYDB_HANDLE hd) break; } } + unlock_all (hd); } xfree (hd);
The patch itself is incorrect, doesn't work well because it releases the structure.
My point is that keybox_fp_release or something like that is needed to close FP.
Comment Actions
Shall we merge this with T2196 ? BTW, I have some unpushed commit and a test installer.
Comment Actions
Except for the release/unlock thing after keybox_compress I already have the other fixes in my 2.2 commits. I noticed that the gpgsm keydb lock/release stuff differes from the one for gpg: For gpg we use the keybox_lock function but that is bot used at all by gpgsm. In theory this should be unified but I fear a regression risk and thus for 2.2 we better don't touch it.
Let us keep this ticket for things in gnupg26 and later with it additional support of keyboxd. And use the old ticket for 2.2 which has anyway a different code layout in both keydb.c modules.
Comment Actions
I understand that this is for 2.6.
I had misunderstood that the way of gpg and the way of gpgsm were same for the locking/unlocking. No, it's different.
For gpg, it locks (by dotlock) pubring.kbx.lock both for read access and write access.
For gpgsm, it only locks only for write access.