locked
ATLTRACE macro issues and Windows 7 (possible OpenThreadToken bug) RRS feed

  • Question

  • Hello everyone,

    First, the pertinent info:

    OS: Windows 7 64-bit Ultimate N and Professional
    Compiler: Visual Studio 2008 SP1 / Visual Studio 2010
    Application:  Win32 app (so will run in 32-bit mode).
    Issue: ATLTRACE macro does not work (causes DebugBreak() to be called when application is run in debug mode (_DEBUG is defined in preprocessor).

    After tracing into ATLTRACE, I have discovered a possible bug in the OpenThreadToken( ) API call for Windows 7, 64-bit OS when running 32-bit apps.  The issue can be duplicated with the code sample below:

    #include <windows.h>

    int main()
    {
       HANDLE m_hThreadToken = INVALID_HANDLE_VALUE;
       if ( OpenThreadToken( GetCurrentProcess( ), TOKEN_IMPERSONATE | TOKEN_DUPLICATE, FALSE, &m_hThreadToken))
       {
               // success
       }
       else
       {
             // failure
       }
    }

    To run this test, you need to be running Windows 7 - 64-bit OS, but create a Win32 app.

    Now the issue is *not* the failure when OpenThreadToken returns-- that is all well and good.  The issue is that the value of m_hThreadToken gets changed when the function returns a failure.  You will see that before the function is called, m_hThreadToken is set to INVALID_HANDLE_VALUE.  However, after the call to OpenThreadToken( ) gets executed, if a failure occurs, the m_hThreadToken value gets changed to NULL.

    What does this have to do with ATLTRACE?  It just so happens that internally ATLTRACE calls OpenThreadToken( ) with the exact same parameters.   Then there is an internal class that is used called CRevertThreadToken that has a HANDLE has an internal member.  On construction of CRevertThreadToken, this handle is set to INVALID_HANDLE_VALUE.  This handle member is sent to the OpenThreadToken API function, and when OpenThreadToken returns failure (again, this is not the issue), that handle value has changed (this *is* the issue). 

    What happens after that is the CRevertThreadToken object is destroyed, and the destructor checks if the handle has changed.  Since it has changed, CRevertThreadToken assumes this is an error, and boom -- the DebugBreak( ) is executed.  To see the source code, open atlutil.h and search for CRevertThreadToken.  You will see exactly the code I've described.  To be exact, the handle value goes from 0xffffffff to 0x0 on failure of OpenThreadToken( ). 

    I tested on two different computers running Windows 7-64Bit, and the issue is the same.  This problem does not occur if you are running Windows 7-32 bit OS or running Windows 7-64-bit mode and the app is 64-bit.  Also, Windows Server doesn't suffer from this issue, regardless of whether we are running in 64-bit or 32-bit mode.  Windows XP/32-bit also works fine.  The value of the handle remains untouched if OpenThreadToken( ) returns FALSE, as I believe it should -- not so with Win 7/64 running a 32-bit app.

    So the problem seems to only exist when running Windows 7/64 and the app is 32-bit.  Both Windows 7 systems I've tested on have various versions of Windows updates installed, so it doesn't seem to be an issue with any Windows updates causing the problem.

    Can anyone run the code above on a Win 7 / 64 bit system, and confirm my findings?  If it is a bug, then either OpenThreadToken( ) needs to be fixed for Windows 7/64 when running 32-bit apps, or atlutil.h needs to be fixed (to be exact, the CRevertThreadToken class needs to be fixed to do a better check of the handle on destruction).

    Thanks in advance

    Tuesday, December 21, 2010 10:55 AM

All replies

  • >Can anyone run the code above on a Win 7 / 64 bit system, and confirm my findings?

    I can confirm your findings - guess it must be an implementation bug
    in the W7 Wow64 layer :(

    Other than a paid for support call to MS (though you shouldn't get
    charged as this is clearly a bug IMHO) I suggest that you report it on
    the MS Connect site against VS2010 (the version doesn't matter since
    its an OS bug): https://connect.microsoft.com/VisualStudio and hope
    that someone can route it to the correct person in MS to get it fixed
    in the OS.

    Just supply the excellent details and example you have here and I'm
    sure someone there will be able to repro it.

    If you do report it on Connect, please post a link to your report back
    here and I'll vote/validate it.

    Dave

    Tuesday, December 21, 2010 11:14 PM
  • Hello Dave,

    My company has opened a bug ticket with Microsoft on this issue today.  I'll keep everyone posted on any progress, as this clearly is a bug in the Wow64 layer that other developers should be aware of.

    Wednesday, December 22, 2010 1:16 AM
  • >My company has opened a bug ticket with Microsoft on this issue today.  I'll keep everyone posted on any progress, as this clearly is a bug in the Wow64 layer that other developers should be aware of.

    Great - indeed, it should be fixed in the OS code. If the incorrect
    behaviour has affected MS code, it'll potentially be affecting many
    others too.

    Dave

    Wednesday, December 22, 2010 7:59 AM
  • Hello Dave,

    We finally got the issue resolved with Microsoft.  They sent me an updated version of atlutil.h with the fix.  The fix is simple (in the Initialize() function):

        if ( OpenThreadToken (/*params*/,  &m_hThreadToken) )
        {
              // OpenThreadToken returns success, so no problem
        }

        // Here is the fix ...
        else
        {
             if ( !m_hThreadToken )
                  m_hThreadToken = INVALID_HANDLE_VALUE;
        }
        // end of fix ...
    }

    I could have fixed this myself, but it's better that MS know about this and officially provide a fix in any future version of ATL.

    Tuesday, January 18, 2011 4:39 PM
  • Nice to have an official solution - though it's really only a work-around for this specific use in ATL. There's still an incompatibility in the W7 Wow64 implementation which may affect others code.

    Sunday, January 23, 2011 3:19 PM