Windows: ACE_TSS_Cleanup incorrectly uses ::TlsAlloc value as an array index causing asserts/crashes about ace_tao

[jwillemsen]

From which ACE version is this trace? On which Windows version do you see this runtime behavior? What is your ace/config.h file contents?

[malwoden]

Hello,
Sorry about the omitted details, added to the original ticket:
Win 10 Enterprise / 64bit. Ace-6.0.3b.

Config:

#ifndef ACE_CONFIG_H

#define ACE_LACKS_IOSTREAM_TOTALLY
#define ACE_HAS_REACTOR_NOTIFICATION_QUEUE 1
#define ACE_AS_STATIC_LIBS 1
#ifdef WIN32
#define ACE_HAS_STANDARD_CPP_LIBRARY 1
#define ACE_HAS_TOKENS_LIBRARY
#define ACE_USES_WCHAR
#define ACE_DEFAULT_THREAD_KEYS 1088
#include "ace/config-win32.h"
#elif defined (WINCE)
#define ACE_HAS_STANDARD_CPP_LIBRARY 1
#define ACE_HAS_TOKENS_LIBRARY
#define ACE_USES_WCHAR
#include "ace/config-win32.h"
#elif defined (APPLE_CC)
#define ACE_HAS_STRPTIME 1
#define ACE_LACKS_UNSETENV
#define ACE_LACKS_ISCTYPE
#define ACE_NEEDS_DL_UNDERSCORE
#include "ace/config-macosx-tiger.h"
#else
#include "ace/config-linux.h"
#endif

#endif

Looks like we do define ACE_DEFAULT_THREAD_KEYS to 1088 here, but it would be defined to the same value in config-win32-common anyway.

[jwillemsen]

We haven't seen runtime problems on Windows 10 in any of our runtime testing. Can you try a more recent ACE release, a lot has changed between the versions.

[malwoden]

I would like to but I am a bit short on time at the moment. I think it is quite easy to see from the source though that the return value of ::TlsAlloc is being used as the index into an array on Windows.

Also found this mailing list entry which is indicating the same problem.

[jwillemsen]

This is in the core of ACE and changing this would require some work. Maybe you can propose a fix in ACE and make a pull request with the necessary changes and test extensions.

If not, see http://www.dre.vanderbilt.edu/~schmidt/commercial-support.html for the companies providing support, including Remedy IT the company I work for.

[vbaderks]

This problem still exist in ACE 6.5.1. As stated by @malwoden, the return value of TlsAlloc is used as an index, which is not allowed.

In normal conditions TlsAlloc will almost always return low values, but there is there no guarantee for it.

The problem is very easy to reproduce:

1. Create a basic ACE application
2. Use the Windows Application Verifier (installed in "C:\Windows\System32\appverif.exe") and add the application to the list of application that should be checked by the OS. Application Verifier enables by default the "Basics\TLS" test for new added applications.
3. Start the ACE application.

On my machine (Windows 10 x64 1803) TlsAlloc would return 13 if TLS checking is disabled and 895930 when checking is enabled.

As 895930 is larger then ACE_DEFAULT_THREAD_KEYS (1088 on win32), ACE will call __ace_assert, which will try to log the error.

void __ace_assert(const char *file, int line, const ACE_TCHAR *expression)
{
  int error = ACE_Log_Msg::last_error_adapter ();
  ACE_Log_Msg *log = ACE_Log_Msg::instance ();

__ace_assert calls ACE_Log_Msg::instance that will trigger a new __ace_assert, etc.. causing the stack overflow.

Workaround:
To use the Windows Application Verifier in combination with applications that use ACE, the TLS test should be disabled.

Minor remark:
The thr_keycreate_native function compares the return value of TlsAlloc with ACE_SYSCALL_FAILED.

ACE_OS::thr_keycreate_native (ACE_OS_thread_key_t *key,
{
...
    ACE_UNUSED_ARG (dest);
    *key = ::TlsAlloc ();
    if (*key == ACE_SYSCALL_FAILED)
      ACE_FAIL_RETURN (-1);
...
}

The Win32 documentation states that the return value of TlsAlloc should be compared with the macro TLS_OUT_OF_INDEXES. As the macros ACE_SYSCALL_FAILED and TLS_OUT_OF_INDEXES are both defined as 0xFFFFFFFF this is not a practical problem.