[git] GnuPG - branch, master, updated. gnupg-2.2.1-35-gfb78286
by NIIBE Yutaka
cvs at cvs.gnupg.org
Fri Oct 27 03:21:13 CEST 2017
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "The GNU Privacy Guard".
The branch, master has been updated
via fb7828676cc2c01047498898378711e049f73fee (commit)
from 3b66a256e3760e88066ca11b7b49d924e42aa46b (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit fb7828676cc2c01047498898378711e049f73fee
Author: NIIBE Yutaka <gniibe at fsij.org>
Date: Fri Oct 27 09:54:48 2017 +0900
agent: Clean up pinentry access locking.
* agent/agent.h (struct server_control_s): Rename PINENTRY_ACTIVE.
* agent/call-pinentry.c (entry_owner): Remove.
(agent_reset_query): Use thread private object of PINENTRY_ACTIVE.
(unlock_pinentry): Add CTRL to arguments to access thread private.
Check and decrement PINENTRY_ACTIVE for recursive use.
(start_pinentry): Check and increment PINENTRY_ACTIVE for recursion.
(agent_askpin): Follow the change of unlock_pinentry API.
(agent_get_passphrase, agent_get_confirmation): Likewise.
(agent_show_message, agent_popup_message_start): Likewise.
(agent_popup_message_stop, agent_clear_passphrase): Likewise.
--
We use the member PINENTRY_ACTIVE as a thread private object.
It's only valid for a single thread at a time.
It would be possible to have a thread shared object of
PINENTRY_ACTIVE, keeping ENTRY_OWNER for distinguishing its
owner (which is also a thread shared object). But, in this case,
access to ENTRY_OWNER is tricky (only comparison to accessing thread
would be OK with no lock), or we need to introduce another lock for
accessing ENTRY_OWNER, which complicates the code too much.
So, simply have a thread private object for recursive pinentry access.
GnuPG-bug-id: 3190
Signed-off-by: NIIBE Yutaka <gniibe at fsij.org>
diff --git a/agent/agent.h b/agent/agent.h
index 78a3764..bf8d244 100644
--- a/agent/agent.h
+++ b/agent/agent.h
@@ -256,8 +256,9 @@ struct server_control_s
count. */
unsigned long s2k_count;
- /* Recursion level of pinentry. */
- int pinentry_level;
+ /* If pinentry is active for this thread. It can be more than 1,
+ when pinentry is called recursively. */
+ int pinentry_active;
};
diff --git a/agent/call-pinentry.c b/agent/call-pinentry.c
index ef76007..af4eb06 100644
--- a/agent/call-pinentry.c
+++ b/agent/call-pinentry.c
@@ -67,12 +67,6 @@ static struct
} entry_features;
-/* The control variable of the connection owning the current pinentry.
- This is only valid if ENTRY_CTX is not NULL. Note, that we care
- only about the value of the pointer and that it should never be
- dereferenced. */
-static ctrl_t entry_owner;
-
/* A mutex used to serialize access to the pinentry. */
static npth_mutex_t entry_lock;
@@ -128,7 +122,7 @@ agent_query_dump_state (void)
void
agent_reset_query (ctrl_t ctrl)
{
- if (entry_ctx && popup_tid && entry_owner == ctrl)
+ if (entry_ctx && popup_tid && ctrl->pinentry_active)
{
agent_popup_message_stop (ctrl);
}
@@ -140,7 +134,7 @@ agent_reset_query (ctrl_t ctrl)
stalled pinentry does not block other threads. Fixme: We should
have a timeout in Assuan for the disconnect operation. */
static gpg_error_t
-unlock_pinentry (gpg_error_t rc)
+unlock_pinentry (ctrl_t ctrl, gpg_error_t rc)
{
assuan_context_t ctx = entry_ctx;
int err;
@@ -177,9 +171,8 @@ unlock_pinentry (gpg_error_t rc)
}
}
- if (--entry_owner->pinentry_level == 0)
+ if (--ctrl->pinentry_active == 0)
{
- entry_owner = NULL;
entry_ctx = NULL;
err = npth_mutex_unlock (&entry_lock);
if (err)
@@ -292,10 +285,11 @@ start_pinentry (ctrl_t ctrl)
char *flavor_version;
int err;
- if (entry_owner == ctrl)
+ if (ctrl->pinentry_active)
{
- /* Allow recursive use of pinentry. */
- ctrl->pinentry_level++;
+ /* It's trying to use pinentry recursively. In this situation,
+ the thread holds ENTRY_LOCK already. */
+ ctrl->pinentry_active++;
return 0;
}
@@ -313,8 +307,6 @@ start_pinentry (ctrl_t ctrl)
return rc;
}
- entry_owner = ctrl;
-
if (entry_ctx)
return 0;
@@ -336,7 +328,7 @@ start_pinentry (ctrl_t ctrl)
the Wine implementation does not flush stdin,stdout and stderr
- see above. Let's try to ignore the error. */
#ifndef HAVE_W32_SYSTEM
- return unlock_pinentry (tmperr);
+ return unlock_pinentry (ctrl, tmperr);
#endif
}
@@ -383,7 +375,7 @@ start_pinentry (ctrl_t ctrl)
return rc;
}
- ctrl->pinentry_level = 1;
+ ctrl->pinentry_active = 1;
entry_ctx = ctx;
/* We don't want to log the pinentry communication to make the logs
@@ -404,7 +396,7 @@ start_pinentry (ctrl_t ctrl)
{
log_error ("can't connect to the PIN entry module '%s': %s\n",
full_pgmname, gpg_strerror (rc));
- return unlock_pinentry (gpg_error (GPG_ERR_NO_PIN_ENTRY));
+ return unlock_pinentry (ctrl, gpg_error (GPG_ERR_NO_PIN_ENTRY));
}
if (DBG_IPC)
@@ -415,65 +407,65 @@ start_pinentry (ctrl_t ctrl)
{
char *optstr;
if (asprintf (&optstr, "OPTION pinentry-user-data=%s", value) < 0 )
- return unlock_pinentry (out_of_core ());
+ return unlock_pinentry (ctrl, out_of_core ());
rc = assuan_transact (entry_ctx, optstr, NULL, NULL, NULL, NULL, NULL,
NULL);
xfree (optstr);
if (rc && gpg_err_code (rc) != GPG_ERR_UNKNOWN_OPTION)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
rc = assuan_transact (entry_ctx,
opt.no_grab? "OPTION no-grab":"OPTION grab",
NULL, NULL, NULL, NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
value = session_env_getenv (ctrl->session_env, "GPG_TTY");
if (value)
{
char *optstr;
if (asprintf (&optstr, "OPTION ttyname=%s", value) < 0 )
- return unlock_pinentry (out_of_core ());
+ return unlock_pinentry (ctrl, out_of_core ());
rc = assuan_transact (entry_ctx, optstr, NULL, NULL, NULL, NULL, NULL,
NULL);
xfree (optstr);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
value = session_env_getenv (ctrl->session_env, "TERM");
if (value)
{
char *optstr;
if (asprintf (&optstr, "OPTION ttytype=%s", value) < 0 )
- return unlock_pinentry (out_of_core ());
+ return unlock_pinentry (ctrl, out_of_core ());
rc = assuan_transact (entry_ctx, optstr, NULL, NULL, NULL, NULL, NULL,
NULL);
xfree (optstr);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
if (ctrl->lc_ctype)
{
char *optstr;
if (asprintf (&optstr, "OPTION lc-ctype=%s", ctrl->lc_ctype) < 0 )
- return unlock_pinentry (out_of_core ());
+ return unlock_pinentry (ctrl, out_of_core ());
rc = assuan_transact (entry_ctx, optstr, NULL, NULL, NULL, NULL, NULL,
NULL);
xfree (optstr);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
if (ctrl->lc_messages)
{
char *optstr;
if (asprintf (&optstr, "OPTION lc-messages=%s", ctrl->lc_messages) < 0 )
- return unlock_pinentry (out_of_core ());
+ return unlock_pinentry (ctrl, out_of_core ());
rc = assuan_transact (entry_ctx, optstr, NULL, NULL, NULL, NULL, NULL,
NULL);
xfree (optstr);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
@@ -489,7 +481,7 @@ start_pinentry (ctrl_t ctrl)
rc = assuan_transact (entry_ctx, "OPTION allow-external-password-cache",
NULL, NULL, NULL, NULL, NULL, NULL);
if (rc && gpg_err_code (rc) != GPG_ERR_UNKNOWN_OPTION)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
if (opt.allow_emacs_pinentry)
@@ -499,7 +491,7 @@ start_pinentry (ctrl_t ctrl)
rc = assuan_transact (entry_ctx, "OPTION allow-emacs-prompt",
NULL, NULL, NULL, NULL, NULL, NULL);
if (rc && gpg_err_code (rc) != GPG_ERR_UNKNOWN_OPTION)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
@@ -537,7 +529,7 @@ start_pinentry (ctrl_t ctrl)
if (*s == '|' && (s2=strchr (s+1,'|')))
s = s2+1;
if (asprintf (&optstr, "OPTION default-%s=%s", tbl[idx].key, s) < 0 )
- return unlock_pinentry (out_of_core ());
+ return unlock_pinentry (ctrl, out_of_core ());
assuan_transact (entry_ctx, optstr, NULL, NULL, NULL, NULL, NULL,
NULL);
xfree (optstr);
@@ -665,8 +657,8 @@ start_pinentry (ctrl_t ctrl)
rc = agent_inq_pinentry_launched (ctrl, pinentry_pid, flavor_version);
if (gpg_err_code (rc) == GPG_ERR_CANCELED
|| gpg_err_code (rc) == GPG_ERR_FULLY_CANCELED)
- return unlock_pinentry (gpg_err_make (GPG_ERR_SOURCE_DEFAULT,
- gpg_err_code (rc)));
+ return unlock_pinentry (ctrl, gpg_err_make (GPG_ERR_SOURCE_DEFAULT,
+ gpg_err_code (rc)));
rc = 0;
}
@@ -1036,18 +1028,18 @@ agent_askpin (ctrl_t ctrl,
rc = assuan_transact (entry_ctx, line,
NULL, NULL, NULL, NULL, NULL, NULL);
if (rc && gpg_err_code (rc) != GPG_ERR_ASS_UNKNOWN_CMD)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
build_cmd_setdesc (line, DIM(line), desc_text);
rc = assuan_transact (entry_ctx, line, NULL, NULL, NULL, NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
snprintf (line, DIM(line), "SETPROMPT %s",
prompt_text? prompt_text : is_pin? L_("PIN:") : L_("Passphrase:"));
rc = assuan_transact (entry_ctx, line, NULL, NULL, NULL, NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
/* If a passphrase quality indicator has been requested and a
minimum passphrase length has not been disabled, send the command
@@ -1056,7 +1048,7 @@ agent_askpin (ctrl_t ctrl,
{
rc = setup_qualitybar (ctrl);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
if (initial_errtext)
@@ -1065,7 +1057,7 @@ agent_askpin (ctrl_t ctrl,
rc = assuan_transact (entry_ctx, line,
NULL, NULL, NULL, NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
if (pininfo->with_repeat)
@@ -1096,7 +1088,7 @@ agent_askpin (ctrl_t ctrl,
rc = assuan_transact (entry_ctx, line,
NULL, NULL, NULL, NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
errtext = NULL;
}
@@ -1106,7 +1098,7 @@ agent_askpin (ctrl_t ctrl,
rc = assuan_transact (entry_ctx, line,
NULL, NULL, NULL, NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
saveflag = assuan_get_flag (entry_ctx, ASSUAN_CONFIDENTIAL);
@@ -1134,7 +1126,7 @@ agent_askpin (ctrl_t ctrl,
errtext = is_pin? L_("PIN too long")
: L_("Passphrase too long");
else if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
if (!errtext && pininfo->min_digits)
{
@@ -1160,7 +1152,7 @@ agent_askpin (ctrl_t ctrl,
|| gpg_err_code (rc) == GPG_ERR_BAD_PIN)
errtext = (is_pin? L_("Bad PIN") : L_("Bad Passphrase"));
else if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
if (!errtext)
@@ -1168,7 +1160,7 @@ agent_askpin (ctrl_t ctrl,
if (pininfo->with_repeat
&& (pinentry_status & PINENTRY_STATUS_PIN_REPEATED))
pininfo->repeat_okay = 1;
- return unlock_pinentry (0); /* okay, got a PIN or passphrase */
+ return unlock_pinentry (ctrl, 0); /* okay, got a PIN or passphrase */
}
if ((pinentry_status & PINENTRY_STATUS_PASSWORD_FROM_CACHE))
@@ -1177,7 +1169,7 @@ agent_askpin (ctrl_t ctrl,
pininfo->failed_tries --;
}
- return unlock_pinentry (gpg_error (pininfo->min_digits? GPG_ERR_BAD_PIN
+ return unlock_pinentry (ctrl, gpg_error (pininfo->min_digits? GPG_ERR_BAD_PIN
: GPG_ERR_BAD_PASSPHRASE));
}
@@ -1243,7 +1235,7 @@ agent_get_passphrase (ctrl_t ctrl,
rc = assuan_transact (entry_ctx, line,
NULL, NULL, NULL, NULL, NULL, NULL);
if (rc && gpg_err_code (rc) != GPG_ERR_ASS_UNKNOWN_CMD)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
if (desc)
@@ -1252,18 +1244,18 @@ agent_get_passphrase (ctrl_t ctrl,
snprintf (line, DIM(line), "RESET");
rc = assuan_transact (entry_ctx, line, NULL, NULL, NULL, NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
snprintf (line, DIM(line), "SETPROMPT %s", prompt);
rc = assuan_transact (entry_ctx, line, NULL, NULL, NULL, NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
if (with_qualitybar && opt.min_passphrase_len)
{
rc = setup_qualitybar (ctrl);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
if (errtext)
@@ -1271,14 +1263,14 @@ agent_get_passphrase (ctrl_t ctrl,
snprintf (line, DIM(line), "SETERROR %s", errtext);
rc = assuan_transact (entry_ctx, line, NULL, NULL, NULL, NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
memset (&parm, 0, sizeof parm);
parm.size = ASSUAN_LINELENGTH/2 - 5;
parm.buffer = gcry_malloc_secure (parm.size+10);
if (!parm.buffer)
- return unlock_pinentry (out_of_core ());
+ return unlock_pinentry (ctrl, out_of_core ());
saveflag = assuan_get_flag (entry_ctx, ASSUAN_CONFIDENTIAL);
assuan_begin_confidential (entry_ctx);
@@ -1302,7 +1294,7 @@ agent_get_passphrase (ctrl_t ctrl,
xfree (parm.buffer);
else
*retpass = parm.buffer;
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
@@ -1346,7 +1338,7 @@ agent_get_confirmation (ctrl_t ctrl,
rc = gpg_err_make (gpg_err_source (rc), GPG_ERR_CANCELED);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
if (ok)
{
@@ -1354,7 +1346,7 @@ agent_get_confirmation (ctrl_t ctrl,
rc = assuan_transact (entry_ctx,
line, NULL, NULL, NULL, NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
if (notok)
{
@@ -1377,7 +1369,7 @@ agent_get_confirmation (ctrl_t ctrl,
NULL, NULL, NULL, NULL, NULL, NULL);
}
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
rc = assuan_transact (entry_ctx, "CONFIRM",
@@ -1385,7 +1377,7 @@ agent_get_confirmation (ctrl_t ctrl,
if (rc && gpg_err_source (rc) && gpg_err_code (rc) == GPG_ERR_ASS_CANCELED)
rc = gpg_err_make (gpg_err_source (rc), GPG_ERR_CANCELED);
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
@@ -1419,7 +1411,7 @@ agent_show_message (ctrl_t ctrl, const char *desc, const char *ok_btn)
rc = gpg_err_make (gpg_err_source (rc), GPG_ERR_CANCELED);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
if (ok_btn)
{
@@ -1427,7 +1419,7 @@ agent_show_message (ctrl_t ctrl, const char *desc, const char *ok_btn)
rc = assuan_transact (entry_ctx, line, NULL, NULL, NULL,
NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
rc = assuan_transact (entry_ctx, "CONFIRM --one-button", NULL, NULL, NULL,
@@ -1435,7 +1427,7 @@ agent_show_message (ctrl_t ctrl, const char *desc, const char *ok_btn)
if (rc && gpg_err_source (rc) && gpg_err_code (rc) == GPG_ERR_ASS_CANCELED)
rc = gpg_err_make (gpg_err_source (rc), GPG_ERR_CANCELED);
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
@@ -1483,19 +1475,19 @@ agent_popup_message_start (ctrl_t ctrl, const char *desc, const char *ok_btn)
snprintf (line, DIM(line), "RESET");
rc = assuan_transact (entry_ctx, line, NULL, NULL, NULL, NULL, NULL, NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
if (ok_btn)
{
snprintf (line, DIM(line), "SETOK %s", ok_btn);
rc = assuan_transact (entry_ctx, line, NULL,NULL,NULL,NULL,NULL,NULL);
if (rc)
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
err = npth_attr_init (&tattr);
if (err)
- return unlock_pinentry (gpg_error_from_errno (err));
+ return unlock_pinentry (ctrl, gpg_error_from_errno (err));
npth_attr_setdetachstate (&tattr, NPTH_CREATE_JOINABLE);
popup_finished = 0;
@@ -1506,7 +1498,7 @@ agent_popup_message_start (ctrl_t ctrl, const char *desc, const char *ok_btn)
rc = gpg_error_from_errno (err);
log_error ("error spawning popup message handler: %s\n",
strerror (err) );
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
npth_setname_np (popup_tid, "popup-message");
@@ -1567,7 +1559,7 @@ agent_popup_message_stop (ctrl_t ctrl)
memset (&popup_tid, '\0', sizeof (popup_tid));
/* Now we can close the connection. */
- unlock_pinentry (0);
+ unlock_pinentry (ctrl, 0);
}
int
@@ -1593,5 +1585,5 @@ agent_clear_passphrase (ctrl_t ctrl,
rc = assuan_transact (entry_ctx, line,
NULL, NULL, NULL, NULL, NULL, NULL);
- return unlock_pinentry (rc);
+ return unlock_pinentry (ctrl, rc);
}
-----------------------------------------------------------------------
Summary of changes:
agent/agent.h | 5 ++-
agent/call-pinentry.c | 122 +++++++++++++++++++++++---------------------------
2 files changed, 60 insertions(+), 67 deletions(-)
hooks/post-receive
--
The GNU Privacy Guard
http://git.gnupg.org
More information about the Gnupg-commits
mailing list