From 4019792423f4f3f2eb9d4d3ca173ffe286f801d8 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Mon, 3 Nov 2008 19:09:34 +0000 Subject: [PATCH] Fixed the card removed with cached app bug. (Famous last fix). --- scd/ChangeLog | 15 +++++++++++++++ scd/app-common.h | 17 ++++++++--------- scd/app.c | 36 ++++++++++++++++++------------------ scd/ccid-driver.c | 3 ++- scd/command.c | 41 +++++++++++++++++++++++------------------ 5 files changed, 66 insertions(+), 46 deletions(-) diff --git a/scd/ChangeLog b/scd/ChangeLog index 53b01e9de..14eafd992 100644 --- a/scd/ChangeLog +++ b/scd/ChangeLog @@ -1,5 +1,20 @@ 2008-11-03 Werner Koch + * app-common.h (app_ctx_s): Remove INITIALIZED. Make REF_COUNT + unsigned. + * app.c (select_application): Remove INITIALIZED. + (app_write_learn_status, app_readcert, app_readkey, app_getattr) + (app_setattr, app_sign, app_decipher, app_writecert) + (app_writekey, app_get_challenge, app_change_pin, app_check_pin): + Replace INITIALIZED by REF_COUNT check. + (application_notify_card_removed): Rename to .. + (application_notify_card_reset): .. this. Change all callers. + * command.c (do_reset): Call application_notify_card_reset after + sending a reset. + (update_reader_status_file): Add arg SET_CARD_REMOVED. + (scd_update_reader_status_file): Pass true for new flag. + (do_reset): Pass false for new flag. + * app.c (app_get_serial_and_stamp): Use bin2hex. * app-help.c (app_help_get_keygrip_string): Ditto. * app-p15.c (send_certinfo, send_keypairinfo, do_getattr): Ditto. diff --git a/scd/app-common.h b/scd/app-common.h index 96fbb92aa..fe98bf832 100644 --- a/scd/app-common.h +++ b/scd/app-common.h @@ -38,14 +38,13 @@ struct app_local_s; /* Defined by all app-*.c. */ struct app_ctx_s { - int initialized; /* The application has been initialied and the - function pointers may be used. Note that for - unsupported operations the particular - function pointer is set to NULL */ - - int ref_count; /* Number of connections currently using this - application context. fixme: We might want to - merg this witghn INITIALIZED above. */ + unsigned int ref_count; /* Number of connections currently using + this application context. If this is + not 0 the application has been + initialized and the function pointers + may be used. Note that for unsupported + operations the particular function + pointer is set to NULL */ int slot; /* Used reader. */ @@ -138,7 +137,7 @@ size_t app_help_read_length_of_cert (int slot, int fid, size_t *r_certoff); /*-- app.c --*/ void app_dump_state (void); -void application_notify_card_removed (int slot); +void application_notify_card_reset (int slot); gpg_error_t check_application_conflict (ctrl_t ctrl, const char *name); gpg_error_t select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app); diff --git a/scd/app.c b/scd/app.c index b15c55e11..731f98326 100644 --- a/scd/app.c +++ b/scd/app.c @@ -161,9 +161,9 @@ is_app_allowed (const char *name) } -/* This may be called to tell this module about a removed card. */ +/* This may be called to tell this module about a removed or resetted card. */ void -application_notify_card_removed (int slot) +application_notify_card_reset (int slot) { app_t app; @@ -369,8 +369,8 @@ select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app) return err; } - app->initialized = 1; app->ref_count = 1; + log_debug ("USING application context (refcount=%u) (new)\n", app->ref_count); lock_table[slot].app = app; *r_app = app; unlock_reader (slot); @@ -405,7 +405,7 @@ release_application (app_t app) if (!app) return; - if (app->ref_count < 1) + if (!app->ref_count) log_bug ("trying to release an already released context\n"); if (--app->ref_count) return; @@ -500,7 +500,7 @@ app_write_learn_status (app_t app, ctrl_t ctrl) if (!app) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.learn_status) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); @@ -529,7 +529,7 @@ app_readcert (app_t app, const char *certid, if (!app) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.readcert) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); @@ -561,7 +561,7 @@ app_readkey (app_t app, const char *keyid, unsigned char **pk, size_t *pklen) if (!app || !keyid || !pk || !pklen) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.readkey) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); @@ -582,7 +582,7 @@ app_getattr (app_t app, ctrl_t ctrl, const char *name) if (!app || !name || !*name) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (app->apptype && name && !strcmp (name, "APPTYPE")) @@ -626,7 +626,7 @@ app_setattr (app_t app, const char *name, if (!app || !name || !*name || !value) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.setattr) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); @@ -652,7 +652,7 @@ app_sign (app_t app, const char *keyidstr, int hashalgo, if (!app || !indata || !indatalen || !outdata || !outdatalen || !pincb) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.sign) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); @@ -684,7 +684,7 @@ app_auth (app_t app, const char *keyidstr, if (!app || !indata || !indatalen || !outdata || !outdatalen || !pincb) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.auth) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); @@ -716,7 +716,7 @@ app_decipher (app_t app, const char *keyidstr, if (!app || !indata || !indatalen || !outdata || !outdatalen || !pincb) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.decipher) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); @@ -746,7 +746,7 @@ app_writecert (app_t app, ctrl_t ctrl, if (!app || !certidstr || !*certidstr || !pincb) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.writecert) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); @@ -774,7 +774,7 @@ app_writekey (app_t app, ctrl_t ctrl, if (!app || !keyidstr || !*keyidstr || !pincb) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.writekey) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); @@ -801,7 +801,7 @@ app_genkey (app_t app, ctrl_t ctrl, const char *keynostr, unsigned int flags, if (!app || !keynostr || !*keynostr || !pincb) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.genkey) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); @@ -827,7 +827,7 @@ app_get_challenge (app_t app, size_t nbytes, unsigned char *buffer) if (!app || !nbytes || !buffer) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); err = lock_reader (app->slot); if (err) @@ -849,7 +849,7 @@ app_change_pin (app_t app, ctrl_t ctrl, const char *chvnostr, int reset_mode, if (!app || !chvnostr || !*chvnostr || !pincb) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.change_pin) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); @@ -877,7 +877,7 @@ app_check_pin (app_t app, const char *keyidstr, if (!app || !keyidstr || !*keyidstr || !pincb) return gpg_error (GPG_ERR_INV_VALUE); - if (!app->initialized) + if (!app->ref_count) return gpg_error (GPG_ERR_CARD_NOT_INITIALIZED); if (!app->fnc.check_pin) return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); diff --git a/scd/ccid-driver.c b/scd/ccid-driver.c index b2c39cd6d..57e617ab5 100644 --- a/scd/ccid-driver.c +++ b/scd/ccid-driver.c @@ -1707,6 +1707,8 @@ ccid_slot_status (ccid_driver_t handle, int *statusbits) } +/* Return the ATR of the card. This is not a cached value and thus an + actual reset is done. */ int ccid_get_atr (ccid_driver_t handle, unsigned char *atr, size_t maxatrlen, size_t *atrlen) @@ -1730,7 +1732,6 @@ ccid_get_atr (ccid_driver_t handle, if (statusbits == 2) return CCID_DRIVER_ERR_NO_CARD; - /* For an inactive and also for an active card, issue the PowerOn command to get the ATR. */ again: diff --git a/scd/command.c b/scd/command.c index 849c9e33d..9385cbd9c 100644 --- a/scd/command.c +++ b/scd/command.c @@ -81,7 +81,7 @@ struct slot_status_s 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. */ + unsigned int changed; /* Last change counter of the slot. */ }; @@ -134,7 +134,7 @@ static pth_mutex_t status_file_update_lock; /*-- Local prototypes --*/ -static void update_reader_status_file (void); +static void update_reader_status_file (int set_card_removed_flag); @@ -171,7 +171,7 @@ update_card_removed (int slot, int value) } /* Let the card application layer know about the removal. */ if (value) - application_notify_card_removed (slot); + application_notify_card_reset (slot); } @@ -256,7 +256,8 @@ hex_to_buffer (const char *string, size_t *r_length) /* Reset the card and free the application context. With SEND_RESET - set to true actually send a RESET to the reader. */ + set to true actually send a RESET to the reader; this is the normal + way of calling the function. */ static void do_reset (ctrl_t ctrl, int send_reset) { @@ -265,18 +266,22 @@ do_reset (ctrl_t ctrl, int send_reset) if (!(slot == -1 || (slot >= 0 && slot < DIM(slot_table)))) BUG (); + /* If there is an active application, release it. */ if (ctrl->app_ctx) { release_application (ctrl->app_ctx); ctrl->app_ctx = NULL; } + /* If we want a real reset for the card, send the reset APDU and + tell the application layer about it. */ if (slot != -1 && send_reset && !IS_LOCKED (ctrl) ) { if (apdu_reset (slot)) { slot_table[slot].reset_failed = 1; } + application_notify_card_reset (slot); } /* If we hold a lock, unlock now. */ @@ -286,23 +291,23 @@ do_reset (ctrl_t ctrl, int send_reset) 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. */ + /* Reset the 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. Calling update_reader_status_file is + required to get hold of the new status of the card in the slot + table. */ if (!pth_mutex_acquire (&status_file_update_lock, 0, NULL)) { log_error ("failed to acquire status_fle_update lock\n"); ctrl->reader_slot = -1; return; } - update_reader_status_file (); - update_card_removed (slot, 0); + update_reader_status_file (0); /* Update slot status table. */ + update_card_removed (slot, 0); /* Clear card_removed flag. */ if (!pth_mutex_release (&status_file_update_lock)) log_error ("failed to release status_file_update lock\n"); - /* Do this last, so that update_card_removed does its job. */ + /* Do this last, so that the update_card_removed above does its job. */ ctrl->reader_slot = -1; } @@ -1875,7 +1880,7 @@ scd_command_handler (ctrl_t ctrl, int fd) } } - /* Cleanup. */ + /* Cleanup. We don't send an explicit reset to the card. */ do_reset (ctrl, 0); /* Release the server object. */ @@ -1951,9 +1956,9 @@ send_status_info (ctrl_t ctrl, const char *keyword, ...) /* This is the core of scd_update_reader_status_file but the caller - needs to take care of the locking. */ + needs to take care of the locking. */ static void -update_reader_status_file (void) +update_reader_status_file (int set_card_removed_flag) { int idx; unsigned int status, changed; @@ -1990,7 +1995,7 @@ update_reader_status_file (void) /* FIXME: Should this be IDX instead of ss->slot? This depends on how client sessions will associate the reader status with their session. */ - sprintf (templ, "reader_%d.status", ss->slot); + snprintf (templ, sizeof templ, "reader_%d.status", ss->slot); fname = make_filename (opt.homedir, templ, NULL ); fp = fopen (fname, "w"); if (fp) @@ -2047,7 +2052,7 @@ update_reader_status_file (void) /* 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) + if (ss->any && set_card_removed_flag) update_card_removed (idx, 1); ss->any = 1; @@ -2090,7 +2095,7 @@ scd_update_reader_status_file (void) { if (!pth_mutex_acquire (&status_file_update_lock, 1, NULL)) return; /* locked - give up. */ - update_reader_status_file (); + update_reader_status_file (1); if (!pth_mutex_release (&status_file_update_lock)) log_error ("failed to release status_file_update lock\n"); }