Fix PKCS#11 RPC handling of multi-threaded clients

In a multi-threaded client which makes concurrent calls into the PKCS#11
module (which is permitted as long as it's for separate sessions), we
might end up with multiple RPC connections to the dæmon.

These are currently being treated as independent client applications,
which leads to spurious CKR_USER_NOT_LOGGED_IN and other errors as
described at https://bugzilla.redhat.com/show_bug.cgi?id=1173279

Fix this by moving the CK_G_APPLICATION state out of the CallState and
into a separate refcounted ClientInstance data structure, and looking up
the appropriate ClientInstance for new connections before creating a new
one.

The ClientInstance is accessed without locking during certain calls
because we know it *cannot* go away while a refcount is still held on it.

1 files changed, 66 insertions, 11 deletions

diff --git a/pkcs11/rpc-layer/gkm-rpc-dispatch.c b/pkcs11/rpc-layer/gkm-rpc-dispatch.c
index 0f0cb8a4..72d2ced1 100644
--- a/pkcs11/rpc-layer/gkm-rpc-dispatch.c
+++ b/pkcs11/rpc-layer/gkm-rpc-dispatch.c
@@ -77,16 +77,29 @@ gkm_rpc_log (const char *line)
  * CALL STRUCTURES
  */
 
+typedef struct _ClientInstance {
+	struct _ClientInstance *next;
+	CK_G_APPLICATION application;
+	pid_t pid;
+	uid_t uid;
+	int refcount;
+} ClientInstance;
+
 typedef struct _CallState {
 	GkmRpcMessage *req;
 	GkmRpcMessage *resp;
 	void *allocated;
-	CK_G_APPLICATION application;
+	ClientInstance *client;
 } CallState;
 
+static ClientInstance *clients = NULL;
+static pthread_mutex_t clients_mutex = PTHREAD_MUTEX_INITIALIZER;
+
 static int
-call_init (CallState *cs)
+call_init (CallState *cs, uid_t uid, pid_t pid)
 {
+	ClientInstance *cl;
+
 	assert (cs);
 
 	cs->req = gkm_rpc_message_new ((EggBufferAllocator)realloc);
@@ -97,9 +110,34 @@ call_init (CallState *cs)
 		return 0;
 	}
 
-	memset (&cs->application, 0, sizeof (cs->application));
-	cs->application.applicationData = cs;
+	pthread_mutex_lock (&clients_mutex);
 
+	for (cl = clients; cl; cl = cl->next) {
+		if (cl->uid == uid && cl->pid == pid) {
+			cl->refcount++;
+			break;
+		}
+	}
+	if (!cl) {
+		cl = malloc (sizeof (*cl));
+		if (!cl) {
+			pthread_mutex_unlock (&clients_mutex);
+			return 0;
+		}
+		cl->next = clients;
+		clients = cl;
+
+		cl->uid = uid;
+		cl->pid = pid;
+		cl->refcount = 1;
+
+		memset (&cl->application, 0, sizeof (cl->application));
+		cl->application.applicationData = cl;
+	}
+
+	pthread_mutex_unlock (&clients_mutex);
+
+	cs->client = cl;
 	cs->allocated = NULL;
 	return 1;
 }
@@ -798,7 +836,7 @@ rpc_C_Finalize (CallState *cs)
 	 * we call C_CloseAllSessions for each slot for this client application.
 	 */
 
-	if (cs->application.applicationId) {
+	if (cs->client->application.applicationId) {
 		ret = (pkcs11_module->C_GetSlotList) (TRUE, NULL, &n_slots);
 		if (ret == CKR_OK) {
 			slots = calloc (n_slots, sizeof (CK_SLOT_ID));
@@ -807,7 +845,7 @@ rpc_C_Finalize (CallState *cs)
 			} else {
 				ret = (pkcs11_module->C_GetSlotList) (TRUE, slots, &n_slots);
 				for (i = 0; ret == CKR_OK && i < n_slots; ++i)
-					ret = (pkcs11_module->C_CloseAllSessions) (slots[i] | cs->application.applicationId);
+					ret = (pkcs11_module->C_CloseAllSessions) (slots[i] | cs->client->application.applicationId);
 				free (slots);
 			}
 		}
@@ -961,7 +999,7 @@ rpc_C_OpenSession (CallState *cs)
 		IN_ULONG (slot_id);
 		IN_ULONG (flags);
 		flags |= CKF_G_APPLICATION_SESSION;
-	PROCESS_CALL ((slot_id, flags, &cs->application, NULL, &session));
+	PROCESS_CALL ((slot_id, flags, &cs->client->application, NULL, &session));
 		OUT_ULONG (session);
 	END_CALL;
 }
@@ -987,7 +1025,7 @@ rpc_C_CloseAllSessions (CallState *cs)
 
 	BEGIN_CALL (C_CloseAllSessions);
 		IN_ULONG (slot_id);
-		slot_id |= cs->application.applicationId;
+		slot_id |= cs->client->application.applicationId;
 	PROCESS_CALL ((slot_id));
 	END_CALL;
 }
@@ -2093,7 +2131,7 @@ run_dispatch_loop (int sock)
 	}
 
 	/* Setup our buffers */
-	if (!call_init (&cs)) {
+	if (!call_init (&cs, uid, pid)) {
 		gkm_rpc_warn ("out of memory");
 		return;
 	}
@@ -2149,8 +2187,25 @@ run_dispatch_loop (int sock)
 	 * as slot ids. Calling with an application identifier closes all
 	 * sessions for just that application identifier.
 	 */
-	if (cs.application.applicationId)
-		(pkcs11_module->C_CloseAllSessions) (cs.application.applicationId);
+	pthread_mutex_lock (&clients_mutex);
+
+	if (!--cs.client->refcount) {
+		ClientInstance **cl = &clients;
+
+		while (*cl) {
+			if (*cl == cs.client) {
+				*cl = cs.client->next;
+				break;
+			}
+			cl = &(*cl)->next;
+		}
+		if (cs.client->application.applicationId)
+			(pkcs11_module->C_CloseAllSessions) (cs.client->application.applicationId);
+		free (cs.client);
+		cs.client = NULL;
+	}
+
+	pthread_mutex_unlock (&clients_mutex);
 
 	call_uninit (&cs);
 }