Attachment 107222 Details for Bug 84567 – p11-kit: Use getpid() to detect when process has forked

[patch] p11-kit: Use getpid() to detect when process has forked

p11-kit-Use-getpid-to-detect-when-process-has-fork.patch (text/plain), 7.65 KB, created by Stef Walter on 2014-10-02 10:56:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stef Walter

Created: 2014-10-02 10:56:01 UTC

Size: 7.65 KB

patch

obsolete

>From 07395497492a1ba72663ccdafa1aaaa3c5acff2a Mon Sep 17 00:00:00 2001
>From: Stef Walter <stefw@redhat.com>
>Date: Thu, 2 Oct 2014 12:20:34 +0200
>Subject: [PATCH] p11-kit: Use getpid() to detect when process has forked
>
>pthread_atfork() is notoriously hard to use cleanly. In particular
>since we don't have portable atomics, it becomes nearly impossible.
>
>https://bugs.freedesktop.org/show_bug.cgi?id=84567
>---
> p11-kit/modules.c    | 45 +++++++++-----------------------------
> p11-kit/proxy.c      | 62 ++++++++++++++++------------------------------------
> p11-kit/proxy.h      |  2 --
> p11-kit/test-proxy.c |  2 +-
> 4 files changed, 30 insertions(+), 81 deletions(-)
>
>diff --git a/p11-kit/modules.c b/p11-kit/modules.c
>index 8aaa769..eab854a 100644
>--- a/p11-kit/modules.c
>+++ b/p11-kit/modules.c
>@@ -158,7 +158,7 @@ typedef struct _Module {
> 
> 	/* Initialization, mutex must be held */
> 	p11_mutex_t initialize_mutex;
>-	bool initialize_called;
>+	pid_t initialize_called;
> 	p11_thread_id_t initialize_thread;
> } Module;
> 
>@@ -247,7 +247,6 @@ free_module_unlocked (void *data)
> 		p11_debug_precond ("module unloaded without C_Finalize having been "
> 		                   "called for each C_Initialize");
> 	} else {
>-		assert (!mod->initialize_called);
> 		assert (mod->initialize_thread == 0);
> 	}
> 
>@@ -612,6 +611,7 @@ initialize_module_inlock_reentrant (Module *mod)
> {
> 	CK_RV rv = CKR_OK;
> 	p11_thread_id_t self;
>+	pid_t pid;
> 
> 	assert (mod);
> 
>@@ -633,7 +633,8 @@ initialize_module_inlock_reentrant (Module *mod)
> 	p11_unlock ();
> 	p11_mutex_lock (&mod->initialize_mutex);
> 
>-	if (!mod->initialize_called) {
>+	pid = getpid ();
>+	if (mod->initialize_called != pid) {
> 		p11_debug ("C_Initialize: calling");
> 
> 		rv = mod->virt.funcs.C_Initialize (&mod->virt.funcs,
>@@ -643,10 +644,12 @@ initialize_module_inlock_reentrant (Module *mod)
> 
> 		/* Module was initialized and C_Finalize should be called */
> 		if (rv == CKR_OK)
>-			mod->initialize_called = true;
>+			mod->initialize_called = pid;
>+		else
>+			mod->initialize_called = 0;
> 
> 		/* Module was already initialized, we don't call C_Finalize */
>-		else if (rv == CKR_CRYPTOKI_ALREADY_INITIALIZED)
>+		if (rv == CKR_CRYPTOKI_ALREADY_INITIALIZED)
> 			rv = CKR_OK;
> 	}
> 
>@@ -665,31 +668,6 @@ initialize_module_inlock_reentrant (Module *mod)
> 	return rv;
> }
> 
>-#ifdef OS_UNIX
>-
>-static void
>-reinitialize_after_fork (void)
>-{
>-	p11_dictiter iter;
>-	Module *mod;
>-
>-	p11_debug ("forked");
>-
>-	p11_lock ();
>-
>-		if (gl.modules) {
>-			p11_dict_iterate (gl.modules, &iter);
>-			while (p11_dict_next (&iter, (void **)&mod, NULL))
>-				mod->initialize_called = false;
>-		}
>-
>-	p11_unlock ();
>-
>-	p11_proxy_after_fork ();
>-}
>-
>-#endif /* OS_UNIX */
>-
> static CK_RV
> init_globals_unlocked (void)
> {
>@@ -719,9 +697,6 @@ init_globals_unlocked (void)
> 	if (once)
> 		return CKR_OK;
> 
>-#ifdef OS_UNIX
>-	pthread_atfork (NULL, NULL, reinitialize_after_fork);
>-#endif
> 	once = true;
> 
> 	return CKR_OK;
>@@ -777,9 +752,9 @@ finalize_module_inlock_reentrant (Module *mod)
> 	p11_unlock ();
> 	p11_mutex_lock (&mod->initialize_mutex);
> 
>-	if (mod->initialize_called) {
>+	if (mod->initialize_called == getpid ()) {
> 		mod->virt.funcs.C_Finalize (&mod->virt.funcs, NULL);
>-		mod->initialize_called = false;
>+		mod->initialize_called = 0;
> 	}
> 
> 	p11_mutex_unlock (&mod->initialize_mutex);
>diff --git a/p11-kit/proxy.c b/p11-kit/proxy.c
>index 3e76f15..3656d33 100644
>--- a/p11-kit/proxy.c
>+++ b/p11-kit/proxy.c
>@@ -82,6 +82,7 @@ typedef struct {
> 	unsigned int n_mappings;
> 	p11_dict *sessions;
> 	CK_FUNCTION_LIST **inited;
>+	pid_t pid;
> } Proxy;
> 
> typedef struct _State {
>@@ -96,6 +97,8 @@ static CK_FUNCTION_LIST **all_modules = NULL;
> static State *all_instances = NULL;
> static State global = { { { { -1, -1 }, NULL, }, }, NULL, NULL, FIRST_HANDLE, NULL };
> 
>+#define PROXY_VALID(px) ((px) && (px)->pid == getpid ())
>+
> #define MANUFACTURER_ID         "PKCS#11 Kit                     "
> #define LIBRARY_DESCRIPTION     "PKCS#11 Kit Proxy Module        "
> #define LIBRARY_VERSION_MAJOR   1
>@@ -137,7 +140,7 @@ map_slot_to_real (Proxy *px,
> 
> 	p11_lock ();
> 
>-		if (!px)
>+		if (!PROXY_VALID (px))
> 			rv = CKR_CRYPTOKI_NOT_INITIALIZED;
> 		else
> 			rv = map_slot_unlocked (px, *slot, mapping);
>@@ -163,7 +166,7 @@ map_session_to_real (Proxy *px,
> 
> 	p11_lock ();
> 
>-		if (!px) {
>+		if (!PROXY_VALID (px)) {
> 			rv = CKR_CRYPTOKI_NOT_INITIALIZED;
> 		} else {
> 			assert (px->sessions);
>@@ -195,40 +198,6 @@ proxy_free (Proxy *py)
> 	}
> }
> 
>-void
>-p11_proxy_after_fork (void)
>-{
>-	p11_array *array;
>-	State *state;
>-	unsigned int i;
>-
>-	/*
>-	 * After a fork the callers are supposed to call C_Initialize and all.
>-	 * In addition the underlying libraries may change their state so free
>-	 * up any mappings and all
>-	 */
>-
>-	array = p11_array_new (NULL);
>-
>-	p11_lock ();
>-
>-		if (global.px)
>-			p11_array_push (array, global.px);
>-		global.px = NULL;
>-
>-		for (state = all_instances; state != NULL; state = state->next) {
>-			if (state->px)
>-				p11_array_push (array, state->px);
>-			state->px = NULL;
>-		}
>-
>-	p11_unlock ();
>-
>-	for (i = 0; i < array->num; i++)
>-		proxy_free (array->elem[i]);
>-	p11_array_free (array);
>-}
>-
> static CK_RV
> proxy_C_Finalize (CK_X_FUNCTION_LIST *self,
>                   CK_VOID_PTR reserved)
>@@ -247,8 +216,10 @@ proxy_C_Finalize (CK_X_FUNCTION_LIST *self,
> 	} else {
> 		p11_lock ();
> 
>-			if (!state->px) {
>+			if (!PROXY_VALID (state->px)) {
> 				rv = CKR_CRYPTOKI_NOT_INITIALIZED;
>+				py = state->px;
>+				state->px = NULL;
> 			} else if (state->px->refs-- == 1) {
> 				py = state->px;
> 				state->px = NULL;
>@@ -287,6 +258,8 @@ proxy_create (Proxy **res)
> 	py = calloc (1, sizeof (Proxy));
> 	return_val_if_fail (py != NULL, CKR_HOST_MEMORY);
> 
>+	py->pid = getpid ();
>+
> 	py->inited = modules_dup (all_modules);
> 	return_val_if_fail (py->inited != NULL, CKR_HOST_MEMORY);
> 
>@@ -357,10 +330,13 @@ proxy_C_Initialize (CK_X_FUNCTION_LIST *self,
> 
> 	p11_lock ();
> 
>-		if (state->px == NULL)
>+		if (!PROXY_VALID (state->px)) {
> 			initialize = true;
>-		else
>+			if (state->px)
>+				proxy_free (state->px);
>+		} else {
> 			state->px->refs++;
>+		}
> 
> 	p11_unlock ();
> 
>@@ -402,7 +378,7 @@ proxy_C_GetInfo (CK_X_FUNCTION_LIST *self,
> 
> 	p11_lock ();
> 
>-		if (!state->px)
>+		if (!PROXY_VALID (state->px))
> 			rv = CKR_CRYPTOKI_NOT_INITIALIZED;
> 
> 	p11_unlock ();
>@@ -438,7 +414,7 @@ proxy_C_GetSlotList (CK_X_FUNCTION_LIST *self,
> 
> 	p11_lock ();
> 
>-		if (!state->px) {
>+		if (!PROXY_VALID (state->px)) {
> 			rv = CKR_CRYPTOKI_NOT_INITIALIZED;
> 		} else {
> 			index = 0;
>@@ -586,7 +562,7 @@ proxy_C_OpenSession (CK_X_FUNCTION_LIST *self,
> 	if (rv == CKR_OK) {
> 		p11_lock ();
> 
>-			if (!state->px) {
>+			if (!PROXY_VALID (state->px)) {
> 				/*
> 				 * The underlying module should have returned an error, so this
> 				 * code should never be reached with properly behaving modules.
>@@ -650,7 +626,7 @@ proxy_C_CloseAllSessions (CK_X_FUNCTION_LIST *self,
> 
> 	p11_lock ();
> 
>-		if (!state->px) {
>+		if (!PROXY_VALID (state->px)) {
> 			rv = CKR_CRYPTOKI_NOT_INITIALIZED;
> 		} else {
> 			assert (state->px->sessions != NULL);
>diff --git a/p11-kit/proxy.h b/p11-kit/proxy.h
>index df05be0..f3d56d7 100644
>--- a/p11-kit/proxy.h
>+++ b/p11-kit/proxy.h
>@@ -35,8 +35,6 @@
> #ifndef __P11_PROXY_H__
> #define __P11_PROXY_H__
> 
>-void       p11_proxy_after_fork                      (void);
>-
> bool       p11_proxy_module_check                    (CK_FUNCTION_LIST_PTR module);
> 
> void       p11_proxy_module_cleanup                  (void);
>diff --git a/p11-kit/test-proxy.c b/p11-kit/test-proxy.c
>index bf5007d..e4998be 100644
>--- a/p11-kit/test-proxy.c
>+++ b/p11-kit/test-proxy.c
>@@ -76,7 +76,7 @@ test_initialize_finalize (void)
> 	assert (rv == CKR_OK);
> 
> 	rv = proxy->C_Finalize (NULL);
>-	assert (rv == CKR_OK);
>+	assert_num_eq (rv, CKR_OK);
> 
> 	p11_proxy_module_cleanup ();
> }
>-- 
>1.9.3

Actions: View | Splinter Review

Attachments on bug 84567: 107220 | 107222 | 107246 | 107280 | 109773

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.