scd: Simplify saving application context.
authorNIIBE Yutaka <gniibe@fsij.org>
Fri, 4 Dec 2015 05:13:23 +0000 (14:13 +0900)
committerNIIBE Yutaka <gniibe@fsij.org>
Tue, 15 Dec 2015 00:49:29 +0000 (09:49 +0900)
* scd/app.c (lock_table): Remove LAST_APP field.
(lock_reader, app_dump_state, application_notify_card_reset)
(release_application): Follow the change.
(check_conflict): New.
(check_application_conflict): Lock the slot and call check_conflict.
(select_application): Call check_conflict and not use LAST_APP.

--

We don't need LAST_APP field but just keep the application context by
APP field.  Since we have a reference counter, it is possible if we
can deallocate or not.

(backport of commit 9639af5f16a7ed908cbce2415330b9fcd88edc90)

scd/app.c

index 380a347..cbcd3e4 100644 (file)
--- a/scd/app.c
+++ b/scd/app.c
@@ -39,7 +39,6 @@ static struct
   int initialized;
   pth_mutex_t lock;
   app_t app;        /* Application context in use or NULL. */
-  app_t last_app;   /* Last application object used as this slot or NULL. */
 } lock_table[10];
 
 
@@ -87,7 +86,6 @@ lock_reader (int slot, ctrl_t ctrl)
         }
       lock_table[slot].initialized = 1;
       lock_table[slot].app = NULL;
-      lock_table[slot].last_app = NULL;
     }
 
   if (!pth_mutex_acquire (&lock_table[slot].lock, 0, NULL))
@@ -154,12 +152,6 @@ app_dump_state (void)
             if (lock_table[slot].app->apptype)
               log_printf (" type=`%s'", lock_table[slot].app->apptype);
           }
-        if (lock_table[slot].last_app)
-          {
-            log_printf (" lastapp=%p", lock_table[slot].last_app);
-            if (lock_table[slot].last_app->apptype)
-              log_printf (" type=`%s'", lock_table[slot].last_app->apptype);
-          }
         log_printf ("\n");
       }
 }
@@ -182,8 +174,6 @@ is_app_allowed (const char *name)
 void
 application_notify_card_reset (int slot)
 {
-  app_t app;
-
   if (slot < 0 || slot >= DIM (lock_table))
     return;
 
@@ -197,20 +187,35 @@ application_notify_card_reset (int slot)
       lock_table[slot].app = NULL;
     }
 
-  /* Deallocate a saved application for that slot, so that we won't
-     try to reuse it.  If there is no saved application, set a flag so
-     that we won't save the current state. */
-  app = lock_table[slot].last_app;
+  unlock_reader (slot);
+}
+
 
-  if (app)
+/*
+ * This function is called with lock held.
+ */
+static gpg_error_t
+check_conflict (int slot, const char *name)
+{
+  app_t app = lock_table[slot].app;
+
+  if (!app || !name || (app->apptype && !ascii_strcasecmp (app->apptype, name)))
+    return 0;
+
+  if (!app->ref_count)
     {
-      lock_table[slot].last_app = NULL;
+      lock_table[slot].app = NULL;
       deallocate_app (app);
+      return 0;
+    }
+  else
+    {
+      log_info ("application '%s' in use by reader %d - can't switch\n",
+                app->apptype? app->apptype : "<null>", slot);
+      return gpg_error (GPG_ERR_CONFLICT);
     }
-  unlock_reader (slot);
 }
 
-
 /* This function is used by the serialno command to check for an
    application conflict which may appear if the serialno command is
    used to request a specific application and the connection has
@@ -218,17 +223,18 @@ application_notify_card_reset (int slot)
 gpg_error_t
 check_application_conflict (ctrl_t ctrl, const char *name)
 {
-  int slot = ctrl->reader_slot;
-  app_t app;
+  gpg_error_t err;
 
   if (slot < 0 || slot >= DIM (lock_table))
     return gpg_error (GPG_ERR_INV_VALUE);
 
-  app = lock_table[slot].initialized ? lock_table[slot].app : NULL;
-  if (app && app->apptype && name)
-    if ( ascii_strcasecmp (app->apptype, name))
-        return gpg_error (GPG_ERR_CONFLICT);
-  return 0;
+  err = lock_reader (slot, ctrl);
+  if (err)
+    return err;
+
+  err = check_conflict (slot, name);
+  unlock_reader (slot);
+  return err;
 }
 
 
@@ -257,41 +263,15 @@ select_application (ctrl_t ctrl, int slot, const char *name, app_t *r_app)
     return err;
 
   /* First check whether we already have an application to share. */
-  app = lock_table[slot].initialized ? lock_table[slot].app : NULL;
-  if (app && name)
-    if (!app->apptype || ascii_strcasecmp (app->apptype, name))
-      {
-        unlock_reader (slot);
-        if (app->apptype)
-          log_info ("application `%s' in use by reader %d - can't switch\n",
-                    app->apptype, slot);
-        return gpg_error (GPG_ERR_CONFLICT);
-      }
-
-  /* If we don't have an app, check whether we have a saved
-     application for that slot.  This is useful so that a card does
-     not get reset even if only one session is using the card - this
-     way the PIN cache and other cached data are preserved.  */
-  if (!app && lock_table[slot].initialized && lock_table[slot].last_app)
+  err = check_conflict (slot, name);
+  if (err)
     {
-      app = lock_table[slot].last_app;
-      if (!name || (app->apptype && !ascii_strcasecmp (app->apptype, name)) )
-        {
-          /* Yes, we can reuse this application - either the caller
-             requested an unspecific one or the requested one matches
-             the saved one. */
-          lock_table[slot].app = app;
-          lock_table[slot].last_app = NULL;
-        }
-      else
-        {
-          /* No, this saved application can't be used - deallocate it. */
-          lock_table[slot].last_app = NULL;
-          deallocate_app (app);
-          app = NULL;
-        }
+      unlock_reader (slot);
+      return err;
     }
 
+  app = lock_table[slot].app;
+
   /* If we can reuse an application, bump the reference count and
      return it.  */
   if (app)
@@ -497,10 +477,10 @@ release_application (app_t app)
       return;
     }
 
-  if (lock_table[slot].last_app)
-    deallocate_app (lock_table[slot].last_app);
-  lock_table[slot].last_app = lock_table[slot].app;
-  lock_table[slot].app = NULL;
+  /* We don't deallocate app here.  Instead, we keep it.  This is
+     useful so that a card does not get reset even if only one session
+     is using the card - this way the PIN cache and other cached data
+     are preserved.  */
   unlock_reader (slot);
 }