[svn] GnuPG - r4030 - in branches/GNUPG-1-9-BRANCH: . scd
svn author wk
cvs at cvs.gnupg.org
Wed Mar 1 12:05:49 CET 2006
Author: wk
Date: 2006-03-01 12:05:47 +0100 (Wed, 01 Mar 2006)
New Revision: 4030
Modified:
branches/GNUPG-1-9-BRANCH/TODO
branches/GNUPG-1-9-BRANCH/scd/ChangeLog
branches/GNUPG-1-9-BRANCH/scd/apdu.c
branches/GNUPG-1-9-BRANCH/scd/app.c
branches/GNUPG-1-9-BRANCH/scd/ccid-driver.c
branches/GNUPG-1-9-BRANCH/scd/command.c
Log:
Fixed card removal problems
Modified: branches/GNUPG-1-9-BRANCH/TODO
===================================================================
--- branches/GNUPG-1-9-BRANCH/TODO 2006-02-27 19:31:13 UTC (rev 4029)
+++ branches/GNUPG-1-9-BRANCH/TODO 2006-03-01 11:05:47 UTC (rev 4030)
@@ -67,7 +67,12 @@
Using the session_list in command.c and the lock_table in app.c. IT
would be better to do this just at one place. First we need to see
how we can support cards with multiple applications.
-
+** Detecting a removed card works only after the ticker detected it.
+ We should check the card status in open-card to make this smoother.
+ Needs to be integrated with the status file update, though. It is
+ not a real problem because application will get a card removed status
+ and should the send a reset to try solving the problem.
+
* tests
** Makefile.am
We use printf(1) to setup the library path, this is not portable.
Modified: branches/GNUPG-1-9-BRANCH/scd/ChangeLog
===================================================================
--- branches/GNUPG-1-9-BRANCH/scd/ChangeLog 2006-02-27 19:31:13 UTC (rev 4029)
+++ branches/GNUPG-1-9-BRANCH/scd/ChangeLog 2006-03-01 11:05:47 UTC (rev 4030)
@@ -1,10 +1,26 @@
+2006-03-01 Werner Koch <wk at g10code.com>
+
+ * command.c (status_file_update_lock): New.
+ (scd_update_reader_status_file): Use lock and factor existing code
+ out to ..
+ (update_reader_status_file): .. this.
+ (do_reset): Use the lock and call update_reader_status_file.
+
+2006-02-20 Werner Koch <wk at g10code.com>
+
+ * apdu.c (open_pcsc_reader): Fixed double free. Thanks to Moritz.
+
2006-02-09 Werner Koch <wk at g10code.com>
+ * command.c (get_reader_slot, do_reset)
+ (scd_update_reader_status_file): Rewrote.
+
* app.c (release_application): Factored code out to ..
(deallocate_app): new function.
(select_application): Introduce new saved application stuff.
(application_notify_card_removed): New.
- * command.c (update_card_removed): Call it.
+ * command.c (update_card_removed): Call it here.
+ (do_reset): And here.
* app.c (check_application_conflict): New.
* command.c (open_card): Use it here.
Modified: branches/GNUPG-1-9-BRANCH/scd/apdu.c
===================================================================
--- branches/GNUPG-1-9-BRANCH/scd/apdu.c 2006-02-27 19:31:13 UTC (rev 4029)
+++ branches/GNUPG-1-9-BRANCH/scd/apdu.c 2006-03-01 11:05:47 UTC (rev 4030)
@@ -1594,6 +1594,7 @@
}
strcpy (reader_table[slot].rdrname, portstr? portstr : list);
xfree (list);
+ list = NULL;
err = pcsc_connect (reader_table[slot].pcsc.context,
reader_table[slot].rdrname,
@@ -1611,7 +1612,6 @@
xfree (reader_table[slot].rdrname);
reader_table[slot].rdrname = NULL;
reader_table[slot].used = 0;
- xfree (list);
return -1 /*pcsc_error_to_sw (err)*/;
}
@@ -2369,7 +2369,7 @@
}
/* Shutdown a reader; that is basically the same as a close but keeps
- the handle ready for later use. A apdu_reset_header should be used
+ the handle ready for later use. A apdu_reset_reader should be used
to get it active again. */
int
apdu_shutdown_reader (int slot)
Modified: branches/GNUPG-1-9-BRANCH/scd/app.c
===================================================================
--- branches/GNUPG-1-9-BRANCH/scd/app.c 2006-02-27 19:31:13 UTC (rev 4029)
+++ branches/GNUPG-1-9-BRANCH/scd/app.c 2006-03-01 11:05:47 UTC (rev 4030)
@@ -165,13 +165,17 @@
return;
/* Deallocate a saved application for that slot, so that we won't
- try to reuse it. */
- if (lock_table[slot].initialized && lock_table[slot].last_app)
+ try to reuse it. If there is no saved application, set a flag so
+ that we won't save the current state. */
+ if (lock_table[slot].initialized)
{
app_t app = lock_table[slot].last_app;
- lock_table[slot].last_app = NULL;
- deallocate_app (app);
+ if (app)
+ {
+ lock_table[slot].last_app = NULL;
+ deallocate_app (app);
+ }
}
}
@@ -380,7 +384,7 @@
/* Free the resources associated with the application APP. APP is
allowed to be NULL in which case this is a no-op. Note that we are
using reference counting to track the users of the application and
- actually deferiing the deallcoation to allow for a later resuse by
+ actually deferring the deallocation to allow for a later reuse by
a new connection. */
void
release_application (app_t app)
Modified: branches/GNUPG-1-9-BRANCH/scd/ccid-driver.c
===================================================================
--- branches/GNUPG-1-9-BRANCH/scd/ccid-driver.c 2006-02-27 19:31:13 UTC (rev 4029)
+++ branches/GNUPG-1-9-BRANCH/scd/ccid-driver.c 2006-03-01 11:05:47 UTC (rev 4030)
@@ -989,7 +989,12 @@
fd = open (transports[i].name, O_RDWR);
if (fd == -1)
- continue;
+ {
+ log_debug ("failed to open `%s': %s\n",
+ transports[i].name, strerror (errno));
+ continue;
+ }
+ log_debug ("opened `%s': fd=%d\n", transports[i].name, fd);
rid = malloc (strlen (transports[i].name) + 30 + 10);
if (!rid)
@@ -1042,6 +1047,7 @@
}
free (rid);
close (fd);
+ log_debug ("closed fd %d\n", fd);
}
if (scan_mode)
@@ -1202,7 +1208,10 @@
if (idev)
usb_close (idev);
if (dev_fd != -1)
- close (dev_fd);
+ {
+ close (dev_fd);
+ log_debug ("closed fd %d\n", dev_fd);
+ }
free (*handle);
*handle = NULL;
}
@@ -1245,6 +1254,7 @@
if (handle->dev_fd != -1)
{
close (handle->dev_fd);
+ log_debug ("closed fd %d\n", handle->dev_fd);
handle->dev_fd = -1;
}
}
@@ -1314,7 +1324,10 @@
usb_close (handle->idev);
handle->idev = NULL;
if (handle->dev_fd != -1)
- close (handle->dev_fd);
+ {
+ close (handle->dev_fd);
+ log_debug ("closed fd %d\n", handle->dev_fd);
+ }
handle->dev_fd = -1;
}
@@ -1327,7 +1340,7 @@
int
ccid_close_reader (ccid_driver_t handle)
{
- if (!handle || !handle->idev)
+ if (!handle || (!handle->idev && handle->dev_fd == -1))
return 0;
do_close_reader (handle);
Modified: branches/GNUPG-1-9-BRANCH/scd/command.c
===================================================================
--- branches/GNUPG-1-9-BRANCH/scd/command.c 2006-02-27 19:31:13 UTC (rev 4029)
+++ branches/GNUPG-1-9-BRANCH/scd/command.c 2006-03-01 11:05:47 UTC (rev 4030)
@@ -62,9 +62,26 @@
&& (c)->reader_slot == locked_session->ctrl_backlink->reader_slot)
+/* This structure is used to keep track of open readers (slots). */
+struct slot_status_s
+{
+ int valid; /* True if the other objects are valid. */
+ int slot; /* Slot number of the reader or -1 if not open. */
+
+ int reset_failed; /* A reset failed. */
+
+ int any; /* Flag indicating whether any status check has been
+ done. This is set once to indicate that the status
+ tracking for the slot has been initialized. */
+ unsigned int status; /* Last status of the slot. */
+ unsigned int changed; /* Last change counter of teh slot. */
+};
+
+
/* Data used to associate an Assuan context with local server data.
This object describes the local properties of one session. */
-struct server_local_s {
+struct server_local_s
+{
/* We keep a list of all active sessions with the anchor at
SESSION_LIST (see below). This field is used for linking. */
struct server_local_s *next_session;
@@ -86,6 +103,10 @@
};
+/* The table with information on all used slots. */
+static struct slot_status_s slot_table[10];
+
+
/* To keep track of all running sessions, we link all active server
contexts and the anchor in this variable. */
static struct server_local_s *session_list;
@@ -94,8 +115,15 @@
in this variable. */
static struct server_local_s *locked_session;
+/* While doing a reset we need to make sure that the ticker does not
+ call scd_update_reader_status_file while we are using it. */
+static pth_mutex_t status_file_update_lock = PTH_MUTEX_INIT;
+
+/*-- Local prototypes --*/
+static void update_reader_status_file (void);
+
/* Update the CARD_REMOVED element of all sessions using the reader
given by SLOT to VALUE */
@@ -107,7 +135,9 @@
for (sl=session_list; sl; sl = sl->next_session)
if (sl->ctrl_backlink
&& sl->ctrl_backlink->reader_slot == slot)
- sl->card_removed = value;
+ {
+ sl->card_removed = value;
+ }
if (value)
application_notify_card_removed (slot);
}
@@ -126,69 +156,52 @@
}
-/* Reset the card and free the application context. With DO_CLOSE set
- to true and this is the last session with a reference to the
- reader, close the reader and don't do just a reset. */
+/* Reset the card and free the application context. With SEND_RESET
+ set to true actually send a RESET to the reader. */
static void
-do_reset (ctrl_t ctrl, int do_close)
+do_reset (ctrl_t ctrl, int send_reset)
{
int slot = ctrl->reader_slot;
+ if (!(slot == -1 || (slot >= 0 && slot < DIM(slot_table))))
+ BUG ();
+
if (ctrl->app_ctx)
{
release_application (ctrl->app_ctx);
ctrl->app_ctx = NULL;
}
- if (ctrl->reader_slot != -1)
+
+ if (slot != -1 && send_reset && !IS_LOCKED (ctrl) )
{
- struct server_local_s *sl;
-
- /* If we are the only session with the reader open we may close
- it. If not, do a reset unless a lock is held on the
- reader. */
- for (sl=session_list; sl; sl = sl->next_session)
- if (sl != ctrl->server_local
- && sl->ctrl_backlink->reader_slot == ctrl->reader_slot)
- break;
- if (sl) /* There is another session with the reader open. */
+ if (apdu_reset (slot))
{
- if ( IS_LOCKED (ctrl) ) /* If it is locked, release it. */
- ctrl->reader_slot = -1;
- else
- {
- if (do_close) /* Always mark reader unused. */
- ctrl->reader_slot = -1;
- else if (apdu_reset (ctrl->reader_slot)) /* Reset only if
- not locked */
- {
- /* The reset failed. Mark the reader as closed. */
- ctrl->reader_slot = -1;
- }
-
- if (locked_session && ctrl->server_local == locked_session)
- {
- locked_session = NULL;
- log_debug ("implicitly unlocking due to RESET\n");
- }
- }
+ slot_table[slot].reset_failed = 1;
}
- else /* No other session has the reader open. */
- {
- if (do_close || apdu_reset (ctrl->reader_slot))
- {
- apdu_close_reader (ctrl->reader_slot);
- ctrl->reader_slot = -1;
- }
- if ( IS_LOCKED (ctrl) )
- {
- log_debug ("WARNING: cleaning up stale session lock\n");
- locked_session = NULL;
- }
- }
}
+ ctrl->reader_slot = -1;
- /* Reset card removed flag for the current reader. */
+ /* If we hold a lock, unlock now. */
+ if (locked_session && ctrl->server_local == locked_session)
+ {
+ locked_session = NULL;
+ log_info ("implicitly unlocking due to RESET\n");
+ }
+
+ /* Reset card removed flag for the current reader. We need to take
+ the lock here so that the ticker thread won't concurrently try to
+ update the file. Note that the update function will set the card
+ removed flag and we will later reset it - not a particualar nice
+ way of implementing it but it works. */
+ if (!pth_mutex_acquire (&status_file_update_lock, 0, NULL))
+ {
+ log_error ("failed to acquire status_fle_update lock\n");
+ return;
+ }
+ update_reader_status_file ();
update_card_removed (slot, 0);
+ if (!pth_mutex_release (&status_file_update_lock))
+ log_error ("failed to release status_file_update lock\n");
}
@@ -197,7 +210,7 @@
{
ctrl_t ctrl = assuan_get_pointer (ctx);
- do_reset (ctrl, 0);
+ do_reset (ctrl, 1);
}
@@ -226,18 +239,22 @@
static int
get_reader_slot (void)
{
- struct server_local_s *sl;
- int slot= -1;
+ struct slot_status_s *ss;
- for (sl=session_list; sl; sl = sl->next_session)
- if (sl->ctrl_backlink
- && (slot = sl->ctrl_backlink->reader_slot) != -1)
- break;
+ ss = &slot_table[0]; /* One reader for now. */
- if (slot == -1)
- slot = apdu_open_reader (opt.reader_port);
+ /* Initialize the item if needed. */
+ if (!ss->valid)
+ {
+ ss->slot = -1;
+ ss->valid = 1;
+ }
- return slot;
+ /* Try to open the reader. */
+ if (ss->slot == -1)
+ ss->slot = apdu_open_reader (opt.reader_port);
+
+ return ss->slot;
}
/* If the card has not yet been opened, do it. Note that this
@@ -349,7 +366,7 @@
{
if ( IS_LOCKED (ctrl) )
return gpg_error (GPG_ERR_LOCKED);
- do_reset (ctrl, 0);
+ do_reset (ctrl, 1);
}
if ((rc = open_card (ctrl, *line? line:NULL)))
@@ -1305,7 +1322,7 @@
/* RESTART
- Restart the current connection; this is a kind of warn reset. It
+ Restart the current connection; this is a kind of warm reset. It
deletes the context used by this connection but does not send a
RESET to the card. Thus the card itself won't get reset.
@@ -1462,7 +1479,7 @@
}
/* Cleanup. */
- do_reset (&ctrl, 1);
+ do_reset (&ctrl, 0);
/* Release the server object. */
if (session_list == ctrl.server_local)
@@ -1532,77 +1549,88 @@
}
-/* This function is called by the ticker thread to check for changes
- of the reader stati. It updates the reader status files and if
- requested by the caller also send a signal to the caller. */
-void
-scd_update_reader_status_file (void)
+/* This is the core of scd_update_reader_status_file but the caller
+ needs to take care of the locking. */
+static void
+update_reader_status_file (void)
{
- static struct {
- int any;
- unsigned int status;
- unsigned int changed;
- } last[10];
- int slot;
- int used;
+ int idx;
unsigned int status, changed;
/* Note, that we only try to get the status, because it does not
make sense to wait here for a operation to complete. If we are
busy working with a card, delays in the status file update should
be acceptable. */
- for (slot=0; (slot < DIM(last)
- &&!apdu_enum_reader (slot, &used)); slot++)
- if (used && !apdu_get_status (slot, 0, &status, &changed))
- {
- if (!last[slot].any || last[slot].status != status
- || last[slot].changed != changed )
- {
- char *fname;
- char templ[50];
- FILE *fp;
- struct server_local_s *sl;
+ for (idx=0; idx < DIM(slot_table); idx++)
+ {
+ struct slot_status_s *ss = slot_table + idx;
- log_info ("updating status of slot %d to 0x%04X\n", slot, status);
-
- sprintf (templ, "reader_%d.status", slot);
- fname = make_filename (opt.homedir, templ, NULL );
- fp = fopen (fname, "w");
- if (fp)
- {
- fprintf (fp, "%s\n",
- (status & 1)? "USABLE":
- (status & 4)? "ACTIVE":
- (status & 2)? "PRESENT": "NOCARD");
- fclose (fp);
- }
- xfree (fname);
+ if (!ss->valid || ss->slot == -1)
+ continue; /* Not valid or reader not yet open. */
+
+ if ( apdu_get_status (ss->slot, 0, &status, &changed) )
+ continue; /* Get status failed. */
- /* Set the card removed flag for all current sessions. We
- will set this on any card change because a reset or
- SERIALNO request must be done in any case. */
- if (last[slot].any)
- update_card_removed (slot, 1);
+ if (!ss->any || ss->status != status || ss->changed != changed )
+ {
+ char *fname;
+ char templ[50];
+ FILE *fp;
+ struct server_local_s *sl;
- last[slot].any = 1;
- last[slot].status = status;
- last[slot].changed = changed;
+ log_info ("updating status of slot %d to 0x%04X\n",
+ ss->slot, status);
+
+ sprintf (templ, "reader_%d.status", ss->slot);
+ fname = make_filename (opt.homedir, templ, NULL );
+ fp = fopen (fname, "w");
+ if (fp)
+ {
+ fprintf (fp, "%s\n",
+ (status & 1)? "USABLE":
+ (status & 4)? "ACTIVE":
+ (status & 2)? "PRESENT": "NOCARD");
+ fclose (fp);
+ }
+ xfree (fname);
+
+ /* Set the card removed flag for all current sessions. We
+ will set this on any card change because a reset or
+ SERIALNO request must be done in any case. */
+ if (ss->any)
+ update_card_removed (ss->slot, 1);
+
+ ss->any = 1;
+ ss->status = status;
+ ss->changed = changed;
-
- /* Send a signal to all clients who applied for it. */
- for (sl=session_list; sl; sl = sl->next_session)
- if (sl->event_signal && sl->assuan_ctx)
- {
- pid_t pid = assuan_get_pid (sl->assuan_ctx);
- int signo = sl->event_signal;
-
- log_info ("client pid is %d, sending signal %d\n",
- pid, signo);
+ /* Send a signal to all clients who applied for it. */
+ for (sl=session_list; sl; sl = sl->next_session)
+ if (sl->event_signal && sl->assuan_ctx)
+ {
+ pid_t pid = assuan_get_pid (sl->assuan_ctx);
+ int signo = sl->event_signal;
+
+ log_info ("client pid is %d, sending signal %d\n",
+ pid, signo);
#ifndef HAVE_W32_SYSTEM
- if (pid != (pid_t)(-1) && pid && signo > 0)
- kill (pid, signo);
+ if (pid != (pid_t)(-1) && pid && signo > 0)
+ kill (pid, signo);
#endif
- }
- }
- }
+ }
+ }
+ }
}
+
+/* This function is called by the ticker thread to check for changes
+ of the reader stati. It updates the reader status files and if
+ requested by the caller also send a signal to the caller. */
+void
+scd_update_reader_status_file (void)
+{
+ if (!pth_mutex_acquire (&status_file_update_lock, 1, NULL))
+ return; /* locked - give up. */
+ update_reader_status_file ();
+ if (!pth_mutex_release (&status_file_update_lock))
+ log_error ("failed to release status_file_update lock\n");
+}
More information about the Gnupg-commits
mailing list