[PATCH] Add test case for bug #90289

[David Woodhouse]

As reported in bug 90289, the p11-kit-proxy module will deadlock if a
child calls C_Initialize() after forking — which is bizarrely
recommended by the PKCS#11 Usage Guide despite the fact that it's a
violation of POSIX to do so in a multi-threaded process.

This patch adds a check for that problem, which is currently causing
p11-kit-proxy not to work with OpenVPN.

---
There is some strangeness here. It looks like any test failure actually
causes all *later* tests to fail, when C_Finalize() and
p11_proxy_module_cleanup() don't get called.

This problem was probably masked by the fact that most instances of
assert() in the tests don't actually use the version in common/test.h
which is equivalent to assert_true(). Instead they use a glibc version
of assert() which ends up aborting the whole process. Why do these
files include <assert.h> after "test.h"? Or at all, for that matter.

---
 p11-kit/test-proxy.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 65 insertions(+), 1 deletion(-)

diff --git a/p11-kit/test-proxy.c b/p11-kit/test-proxy.c
index e4998be..b00be05 100644
--- a/p11-kit/test-proxy.c
+++ b/p11-kit/test-proxy.c
@@ -52,6 +52,9 @@
 #include <string.h>
 #include <time.h>
 #include <unistd.h>
+#ifndef _WIN32
+#include <sys/wait.h>
+#endif
 
 /* This is the proxy module entry point in proxy.c, and linked to this test */
 CK_RV C_GetFunctionList (CK_FUNCTION_LIST_PTR_PTR list);
@@ -110,6 +113,64 @@ test_initialize_multiple (void)
        p11_proxy_module_cleanup ();
 }
 
+#ifndef _WIN32
+static void
+test_initialize_child (void)
+{
+       CK_FUNCTION_LIST_PTR proxy;
+       CK_RV rv;
+       pid_t pid;
+       int st;
+
+       rv = C_GetFunctionList (&proxy);
+       assert (rv == CKR_OK);
+
+       assert (p11_proxy_module_check (proxy));
+
+       rv = proxy->C_Initialize(NULL);
+       assert_num_eq (rv, CKR_OK);
+
+       pid = fork ();
+       if (!pid) {
+               /* The PKCS#11 Usage Guide (v2.40) advocates in §2.5.2 that
+                * a child should call C_Initialize() after forking, and
+                * then immediately C_Finalize() if it's not going to do
+                * anything more with the PKCS#11 token. In a multi-threaded
+                * program this is a violation of the POSIX standard, which
+                * puts strict limits on what you're allowed to do between
+                * fork and an eventual exec or exit. But some things (like
+                * pkcs11-helper and thus OpenVPN) do it anyway, and we
+                * need to cope... */
+
+               /* https://bugs.freedesktop.org/show_bug.cgi?id=90289 reports
+                * a deadlock when this happens. Catch it with SIGALRM... */
+               alarm(1);
+
+               rv = proxy->C_Initialize(NULL);
+               assert_num_eq (rv, CKR_OK);
+
+               rv = proxy->C_Finalize (NULL);
+               assert_num_eq (rv, CKR_OK);
+
+               exit(0);
+       }
+       assert (pid != -1);
+       waitpid(pid, &st, 0);
+
+       rv = proxy->C_Finalize (NULL);
+       assert_num_eq (rv, CKR_OK);
+
+       p11_proxy_module_cleanup ();
+
+       /* If the assertion fails, p11_kit_failed() doesn't return. So make
+        * sure we do all the cleanup before the (expected) failure, or it
+        * causes all the *later* tests to fail too! */
+       if (!WIFEXITED (st) || WEXITSTATUS(st) != 0)
+               assert_fail("Child failed to C_Initialize() and C_Finalize()", NULL);
+
+}
+#endif
+
 static CK_FUNCTION_LIST_PTR
 setup_mock_module (CK_SESSION_HANDLE *session)
 {
@@ -188,6 +249,9 @@ main (int argc,
 
        p11_test (test_initialize_finalize, "/proxy/initialize-finalize");
        p11_test (test_initialize_multiple, "/proxy/initialize-multiple");
+#ifndef _WIN32
+       p11_test (test_initialize_child, "/proxy/initialize-child");
+#endif
 
        test_mock_add_tests ("/proxy");
 
-- 
2.4.2

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse at intel.com                              Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5691 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/p11-glue/attachments/20150603/d27f2b32/attachment.bin>