Commit 1ae891c6 authored by David Benjamin's avatar David Benjamin
Browse files

Save and restore the Windows error around TlsGetValue.



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: default avatarRich Salz <rsalz@openssl.org>

(cherry picked from commit 2de108df)

(Merged from https://github.com/openssl/openssl/pull/6349)
parent 926b2111
Loading
Loading
Loading
Loading
+20 −1
Original line number Diff line number Diff line
@@ -98,7 +98,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)
+5 −1
Original line number Diff line number Diff line
@@ -18,7 +18,7 @@ IF[{- !$disabled{tests} -}]
          dtlsv1listentest ct_test threadstest afalgtest d2i_test \
          ssl_test_ctx_test ssl_test x509aux cipherlist_test asynciotest \
          bioprinttest sslapitest dtlstest sslcorrupttest bio_enc_test \
          ocspapitest fatalerrtest x509_time_test
          ocspapitest fatalerrtest x509_time_test errtest

  SOURCE[versions]=versions.c
  INCLUDE[versions]=../include
@@ -306,6 +306,10 @@ IF[{- !$disabled{tests} -}]
    SOURCE[shlibloadtest]=shlibloadtest.c
    INCLUDE[shlibloadtest]=../include
  ENDIF

  SOURCE[errtest]=errtest.c testutil.c
  INCLUDE[errtest]=../include
  DEPEND[errtest]=../libcrypto
ENDIF

{-

test/errtest.c

0 → 100644
+40 −0
Original line number Diff line number Diff line
/*
 * 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 GetLastError() == ERROR_INVALID_FUNCTION;
#else
    errno = EINVAL;
    ERR_get_error();
    return errno == EINVAL;
#endif
}

int main(int argc, char **argv)
{
    ADD_TEST(preserves_system_error);

    return run_tests(argv[0]);
}
+12 −0
Original line number Diff line number Diff line
#! /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");