[git] GnuPG - branch, master, updated. gnupg-2.1.17-15-gc48cf7e
by NIIBE Yutaka
cvs at cvs.gnupg.org
Thu Dec 29 02:23:50 CET 2016
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 c48cf7e32ffa02ebdf00265543344c611bef0431 (commit)
from 4cc9fc5eb9bd91d943c185d59da4a2b4cb885ee6 (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 c48cf7e32ffa02ebdf00265543344c611bef0431
Author: NIIBE Yutaka <gniibe at fsij.org>
Date: Thu Dec 29 10:07:43 2016 +0900
scd: Fix a race condition for new_reader_slot.
* scd/apdu.c (reader_table_lock, apdu_init): New.
(new_reader_slot): Serialize by reader_table_lock.
* scd/app.c (lock_app, unlock_app, app_new_register): Fix error code
usage.
(initialize_module_command): Call apdu_init.
* scd/scdaemon.c (main): Handle error for initialize_module_command.
--
This is a long standing bug. There are two different things; The
serialization of allocating a new SLOT, and the serialization of using
the SLOT. The latter was implemented in new_reader_slot by lock_slot.
However, the former was not done. Thus, there was a possible race where
a same SLOT is allocated to multiple threads.
Signed-off-by: NIIBE Yutaka <gniibe at fsij.org>
diff --git a/scd/apdu.c b/scd/apdu.c
index 177cd8e..d0b75c8 100644
--- a/scd/apdu.c
+++ b/scd/apdu.c
@@ -144,7 +144,6 @@ struct reader_table_s {
not yet been read; i.e. the card is not
ready for use. */
#ifdef USE_NPTH
- int lock_initialized;
npth_mutex_t lock;
#endif
};
@@ -153,6 +152,10 @@ typedef struct reader_table_s *reader_table_t;
/* A global table to keep track of active readers. */
static struct reader_table_s reader_table[MAX_READER];
+#ifdef USE_NPTH
+static npth_mutex_t reader_table_lock;
+#endif
+
/* ct API function pointer. */
static char (* DLSTDCALL CT_init) (unsigned short ctn, unsigned short Pn);
@@ -424,35 +427,29 @@ static int
new_reader_slot (void)
{
int i, reader = -1;
- int err;
+ npth_mutex_lock (&reader_table_lock);
for (i=0; i < MAX_READER; i++)
- {
- if (!reader_table[i].used && reader == -1)
+ if (!reader_table[i].used)
+ {
reader = i;
- }
+ reader_table[reader].used = 1;
+ break;
+ }
+ npth_mutex_unlock (&reader_table_lock);
+
if (reader == -1)
{
log_error ("new_reader_slot: out of slots\n");
return -1;
}
-#ifdef USE_NPTH
- if (!reader_table[reader].lock_initialized)
- {
- err = npth_mutex_init (&reader_table[reader].lock, NULL);
- if (err)
- {
- log_error ("error initializing mutex: %s\n", strerror (err));
- return -1;
- }
- reader_table[reader].lock_initialized = 1;
- }
-#endif /*USE_NPTH*/
+
if (lock_slot (reader))
{
- log_error ("error locking mutex: %s\n", strerror (errno));
+ reader_table[reader].used = 0;
return -1;
}
+
reader_table[reader].connect_card = NULL;
reader_table[reader].disconnect_card = NULL;
reader_table[reader].close_reader = NULL;
@@ -465,7 +462,6 @@ new_reader_slot (void)
reader_table[reader].pinpad_verify = pcsc_pinpad_verify;
reader_table[reader].pinpad_modify = pcsc_pinpad_modify;
- reader_table[reader].used = 1;
reader_table[reader].is_t0 = 1;
reader_table[reader].is_spr532 = 0;
reader_table[reader].pinpad_varlen_supported = 0;
@@ -650,7 +646,6 @@ static int
close_ct_reader (int slot)
{
CT_close (slot);
- reader_table[slot].used = 0;
return 0;
}
@@ -1435,7 +1430,6 @@ close_pcsc_reader_direct (int slot)
pcsc_release_context (reader_table[slot].pcsc.context);
xfree (reader_table[slot].rdrname);
reader_table[slot].rdrname = NULL;
- reader_table[slot].used = 0;
return 0;
}
#endif /*!NEED_PCSC_WRAPPER*/
@@ -2439,7 +2433,6 @@ close_ccid_reader (int slot)
{
ccid_close_reader (reader_table[slot].ccid.handle);
reader_table[slot].rdrname = NULL;
- reader_table[slot].used = 0;
return 0;
}
@@ -2670,7 +2663,6 @@ static int
close_rapdu_reader (int slot)
{
rapdu_release (reader_table[slot].rapdu.handle);
- reader_table[slot].used = 0;
return 0;
}
@@ -3180,10 +3172,12 @@ apdu_close_reader (int slot)
if (reader_table[slot].close_reader)
{
sw = reader_table[slot].close_reader (slot);
+ reader_table[slot].used = 0;
if (DBG_READER)
log_debug ("leave: apdu_close_reader => 0x%x (close_reader)\n", sw);
return sw;
}
+ reader_table[slot].used = 0;
if (DBG_READER)
log_debug ("leave: apdu_close_reader => SW_HOST_NOT_SUPPORTED\n");
return SW_HOST_NOT_SUPPORTED;
@@ -3203,6 +3197,7 @@ apdu_prepare_exit (void)
if (!sentinel)
{
sentinel = 1;
+ npth_mutex_lock (&reader_table_lock);
for (slot = 0; slot < MAX_READER; slot++)
if (reader_table[slot].used)
{
@@ -3211,6 +3206,7 @@ apdu_prepare_exit (void)
reader_table[slot].close_reader (slot);
reader_table[slot].used = 0;
}
+ npth_mutex_unlock (&reader_table_lock);
sentinel = 0;
}
}
@@ -4222,3 +4218,28 @@ apdu_get_reader_name (int slot)
{
return reader_table[slot].rdrname;
}
+
+gpg_error_t
+apdu_init (void)
+{
+#ifdef USE_NPTH
+ gpg_error_t err;
+ int i;
+
+ if (npth_mutex_init (&reader_table_lock, NULL))
+ goto leave;
+
+ for (i = 0; i < MAX_READER; i++)
+ if (npth_mutex_init (&reader_table[i].lock, NULL))
+ goto leave;
+
+ /* All done well. */
+ return 0;
+
+ leave:
+ err = gpg_error_from_syserror ();
+ log_error ("apdu: error initializing mutex: %s\n", gpg_strerror (err));
+ return err;
+#endif /*USE_NPTH*/
+ return 0;
+}
diff --git a/scd/apdu.h b/scd/apdu.h
index ba856af..3021cf7 100644
--- a/scd/apdu.h
+++ b/scd/apdu.h
@@ -84,6 +84,8 @@ enum {
#define APDU_CARD_ACTIVE (4) /* Card is active. */
+gpg_error_t apdu_init (void);
+
/* Note, that apdu_open_reader returns no status word but -1 on error. */
int apdu_open_reader (const char *portstr);
int apdu_open_remote_reader (const char *portstr,
diff --git a/scd/app.c b/scd/app.c
index a78d652..fa05475 100644
--- a/scd/app.c
+++ b/scd/app.c
@@ -58,14 +58,12 @@ print_progress_line (void *opaque, const char *what, int pc, int cur, int tot)
static gpg_error_t
lock_app (app_t app, ctrl_t ctrl)
{
- gpg_error_t err;
-
- err = npth_mutex_lock (&app->lock);
- if (err)
+ if (npth_mutex_lock (&app->lock))
{
+ gpg_error_t err = gpg_error_from_syserror ();
log_error ("failed to acquire APP lock for %p: %s\n",
- app, strerror (err));
- return gpg_error_from_errno (err);
+ app, gpg_strerror (err));
+ return err;
}
apdu_set_progress_cb (app->slot, print_progress_line, ctrl);
@@ -77,14 +75,14 @@ lock_app (app_t app, ctrl_t ctrl)
static void
unlock_app (app_t app)
{
- gpg_error_t err;
-
apdu_set_progress_cb (app->slot, NULL, NULL);
- err = npth_mutex_unlock (&app->lock);
- if (err)
- log_error ("failed to release APP lock for %p: %s\n",
- app, strerror (err));
+ if (npth_mutex_unlock (&app->lock))
+ {
+ gpg_error_t err = gpg_error_from_syserror ();
+ log_error ("failed to release APP lock for %p: %s\n",
+ app, gpg_strerror (err));
+ }
}
@@ -194,11 +192,11 @@ app_new_register (int slot, ctrl_t ctrl, const char *name)
}
app->slot = slot;
- err = npth_mutex_init (&app->lock, NULL);
- if (err)
+
+ if (npth_mutex_init (&app->lock, NULL))
{
err = gpg_error_from_syserror ();
- log_error ("error initializing mutex: %s\n", strerror (err));
+ log_error ("error initializing mutex: %s\n", gpg_strerror (err));
xfree (app);
return err;
}
@@ -1057,16 +1055,17 @@ scd_update_reader_status_file (void)
has to be done before a second thread is spawned. We can't do the
static initialization because Pth emulation code might not be able
to do a static init; in particular, it is not possible for W32. */
-void
+gpg_error_t
initialize_module_command (void)
{
- static int initialized;
- int err;
+ gpg_error_t err;
- if (!initialized)
+ if (npth_mutex_init (&app_list_lock, NULL))
{
- err = npth_mutex_init (&app_list_lock, NULL);
- if (!err)
- initialized = 1;
+ err = gpg_error_from_syserror ();
+ log_error ("app: error initializing mutex: %s\n", gpg_strerror (err));
+ return err;
}
+
+ return apdu_init ();
}
diff --git a/scd/scdaemon.c b/scd/scdaemon.c
index 38e3c40..74fed44 100644
--- a/scd/scdaemon.c
+++ b/scd/scdaemon.c
@@ -640,7 +640,12 @@ main (int argc, char **argv )
set_debug (debug_level);
- initialize_module_command ();
+ if (initialize_module_command ())
+ {
+ log_error ("initialization failed\n");
+ cleanup ();
+ exit (1);
+ }
if (gpgconf_list == 2)
scd_exit (0);
diff --git a/scd/scdaemon.h b/scd/scdaemon.h
index e18ebfe..22b17b4 100644
--- a/scd/scdaemon.h
+++ b/scd/scdaemon.h
@@ -118,7 +118,7 @@ void scd_exit (int rc);
const char *scd_get_socket_name (void);
/*-- command.c --*/
-void initialize_module_command (void);
+gpg_error_t initialize_module_command (void);
int scd_command_handler (ctrl_t, int);
void send_status_info (ctrl_t ctrl, const char *keyword, ...)
GPGRT_ATTR_SENTINEL(1);
-----------------------------------------------------------------------
Summary of changes:
scd/apdu.c | 67 ++++++++++++++++++++++++++++++++++++++--------------------
scd/apdu.h | 2 ++
scd/app.c | 43 ++++++++++++++++++-------------------
scd/scdaemon.c | 7 +++++-
scd/scdaemon.h | 2 +-
5 files changed, 74 insertions(+), 47 deletions(-)
hooks/post-receive
--
The GNU Privacy Guard
http://git.gnupg.org
More information about the Gnupg-commits
mailing list