Fixed the card removed with cached app bug. (Famous last fix).

scd/ChangeLog

@@ -1,5 +1,20 @@
 2008-11-03  Werner Koch  <wk@g10code.com>
 
+	* 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.

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
+  unsigned int ref_count;  /* Number of connections currently using
-                       function pointers may be used.  Note that for
+                              this application context.  If this is
-                       unsupported operations the particular
+                              not 0 the application has been
-                       function pointer is set to NULL */
+                              initialized and the function pointers
-
+                              may be used.  Note that for unsupported
-  int ref_count;    /* Number of connections currently using this
+                              operations the particular function
-                       application context.  fixme: We might want to
+                              pointer is set to NULL */
-                       merg this witghn INITIALIZED above. */
 
   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);

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);

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:

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
+  /* Reset the card removed flag for the current reader.  We need to
-     the lock here so that the ticker thread won't concurrently try to
+     take the lock here so that the ticker thread won't concurrently
-     update the file.  Note that the update function will set the card
+     try to update the file.  Calling update_reader_status_file is
-     removed flag and we will later reset it - not a particualar nice
+     required to get hold of the new status of the card in the slot
-     way of implementing it but it works. */
+     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_reader_status_file (0);  /* Update slot status table.  */
-  update_card_removed (slot, 0);
+  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.  */
@@ -1953,7 +1958,7 @@ 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.  */
 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");
 }