Save and restore the Windows error around TlsGetValue.
authorDavid Benjamin <davidben@google.com>
Sun, 20 May 2018 21:24:30 +0000 (17:24 -0400)
committerDavid Benjamin <davidben@google.com>
Wed, 23 May 2018 21:34:54 +0000 (17:34 -0400)
TlsGetValue clears the last error even on success, so that callers may
distinguish it successfully returning NULL or failing. This error-mangling
behavior interferes with the caller's use of GetLastError. In particular
SSL_get_error queries the error queue to determine whether the caller should
look at the OS's errors. To avoid destroying state, save and restore the
Windows error.

Fixes #6299.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6316)

crypto/threads_win.c
test/build.info
test/errtest.c [new file with mode: 0644]
test/recipes/04-test_err.t [new file with mode: 0644]

index 1e5cf82073e0787dddb88956882a9a62747dec12..7fdbc1f67fe5505c9f8eaf2563dc18a8837215e9 100644 (file)
@@ -101,7 +101,26 @@ int CRYPTO_THREAD_init_local(CRYPTO_THREAD_LOCAL *key, void (*cleanup)(void *))
 
 void *CRYPTO_THREAD_get_local(CRYPTO_THREAD_LOCAL *key)
 {
-    return TlsGetValue(*key);
+    DWORD last_error;
+    void *ret;
+
+    /*
+     * TlsGetValue clears the last error even on success, so that callers may
+     * distinguish it successfully returning NULL or failing. It is documented
+     * to never fail if the argument is a valid index from TlsAlloc, so we do
+     * not need to handle this.
+     *
+     * However, this error-mangling behavior interferes with the caller's use of
+     * GetLastError. In particular SSL_get_error queries the error queue to
+     * determine whether the caller should look at the OS's errors. To avoid
+     * destroying state, save and restore the Windows error.
+     *
+     * https://msdn.microsoft.com/en-us/library/windows/desktop/ms686812(v=vs.85).aspx
+     */
+    last_error = GetLastError();
+    ret = TlsGetValue(*key);
+    SetLastError(last_error);
+    return ret;
 }
 
 int CRYPTO_THREAD_set_local(CRYPTO_THREAD_LOCAL *key, void *val)
index c3a09048b55d1da5070a35d54c797f855a18c096..000153d510bccc5bd4bd3b44171fc5591a9507b8 100644 (file)
@@ -51,7 +51,7 @@ INCLUDE_MAIN___test_libtestutil_OLB = /INCLUDE=MAIN
           recordlentest drbgtest drbg_cavs_test sslbuffertest \
           time_offset_test pemtest ssl_cert_table_internal_test ciphername_test \
           servername_test ocspapitest rsa_mp_test fatalerrtest tls13ccstest \
-          sysdefaulttest
+          sysdefaulttest errtest
 
   SOURCE[versions]=versions.c
   INCLUDE[versions]=../include
@@ -535,6 +535,10 @@ INCLUDE_MAIN___test_libtestutil_OLB = /INCLUDE=MAIN
   SOURCE[sysdefaulttest]=sysdefaulttest.c
   INCLUDE[sysdefaulttest]=../include
   DEPEND[sysdefaulttest]=../libcrypto ../libssl libtestutil.a
+
+  SOURCE[errtest]=errtest.c
+  INCLUDE[errtest]=../include
+  DEPEND[errtest]=../libcrypto libtestutil.a
 ENDIF
 
 {-
diff --git a/test/errtest.c b/test/errtest.c
new file mode 100644 (file)
index 0000000..e464d08
--- /dev/null
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2018 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the OpenSSL license (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+#include <openssl/opensslconf.h>
+#include <openssl/err.h>
+
+#include "testutil.h"
+
+#if defined(OPENSSL_SYS_WINDOWS)
+# include <windows.h>
+#else
+# include <errno.h>
+#endif
+
+/* Test that querying the error queue preserves the OS error. */
+static int preserves_system_error(void)
+{
+#if defined(OPENSSL_SYS_WINDOWS)
+    SetLastError(ERROR_INVALID_FUNCTION);
+    ERR_get_error();
+    return TEST_int_eq(GetLastError(), ERROR_INVALID_FUNCTION);
+#else
+    errno = EINVAL;
+    ERR_get_error();
+    return TEST_int_eq(errno, EINVAL);
+#endif
+}
+
+int setup_tests(void)
+{
+    ADD_TEST(preserves_system_error);
+    return 1;
+}
diff --git a/test/recipes/04-test_err.t b/test/recipes/04-test_err.t
new file mode 100644 (file)
index 0000000..dd7681a
--- /dev/null
@@ -0,0 +1,12 @@
+#! /usr/bin/env perl
+# Copyright 2018 The OpenSSL Project Authors. All Rights Reserved.
+#
+# Licensed under the OpenSSL license (the "License").  You may not use
+# this file except in compliance with the License.  You can obtain a copy
+# in the file LICENSE in the source distribution or at
+# https://www.openssl.org/source/license.html
+
+
+use OpenSSL::Test::Simple;
+
+simple_test("test_err", "errtest");