It's an unfortunate choice of data type for the file system redirection cookie


If you want to disable 64-bit file system redirection, you call the Wow64­Disable­Wow64­Fs­Redirection function. This function gives you a cookie. When you are finished, you call Wow64­Revert­Wow64­Fs­Redirection, passing the cookie you received from the previous call. Like this:

void* cookie;
if (Wow64DisableWow64FsRedirection(&cookie)) {
   ... do stuff ...
   Wow64RevertWow64FsRedirection(cookie);
}

The unfortunate thing is that the data type for the cookie is an untyped pointer: void*. This means that the following mistake goes undetected:

// Remember: Code in italics is wrong.
void* cookie;
if (Wow64DisableWow64FsRedirection(&cookie)) {
   ... do stuff ...
   Wow64RevertWow64FsRedirection(&cookie);
}

The erroneous parameter to Wow64­Revert­Wow64­Fs­Redirection goes undetected because void** is implicitly convertible to void*. Because any pointer is implicitly convertible to void*, because void* is a generic pointer.

In retrospect, the type of the cookie used by the file system redirection functions should have been something other than void*. It could have used DECLARE_HANDLE, which declares a pointer to a dummy structure with a unique name. Or it could have been a pointer to a uniquely-named incomplete type.

Comments (21)
  1. Darran Rowe says:

    But that's the way the cookie crumbles, right?

  2. Mc says:

    Would the return value be false in the incorrect case? So would proper error handling flag the error when it was tested?

    1. Joshua says:

      Yeah, but why would you check the failure code here?

      1. Roman says:

        To catch errors like this, naturally.

        1. ZLB says:

          In practice though, people don't tend to bother with the return value from calls like these unless they are debugging a problem in the neighborhood. I mean what are you going to do if it fails? Call it again in a loop until it works?

          1. Darran Rowe says:

            I would say that this depends really. In all my debug and fully logged builds, I always check the return of close functions.
            While it is true that the close functions are normally call once and forget, and if it fails, then you mustn't do anything else, there is one important thing you can get from that failure, the fact that it failed.
            If you are not completely in the world of RAII, then a failure of a close function could seriously indicate a resource handling problem. So while granted, this is still debugging, keeping track of the return is always useful since it can surprisingly show an error is lurking inside your application without you realising it.

          2. Joshua says:

            @Darren Rowe: Ah, but in the UNIX world you must handle close() returning -1 with errno = EIO. It's not really all that recoverable (either flushing buffer to server failed which means you lost your disk, or you ran out of disk) but you need to abort whatever you were working on to avoid further data corruption.

  3. jeffrey cassar says:

    Wow64DisableWow64FsRedirection stores an address in the void* you gave it. Wow64RevertWow64FsRedirection takes a void*, so that it can have that value back. who would randomly give Wow64RevertWow64FsRedirection the address of a void* when it explicitly doesn't expect that? this being the windows api is irrelevant. this is a language issue here. I definitely agree.

  4. jeffrey cassar says:

    it definitely makes sense, and I'm shocked, that there wasn't a DECLARE_HANDLE(HCOOKIE) etc. like you mentioned, though.

  5. Dave Bacher says:

    Theoretically, it would not be too late to fix this one.

    If you added a defined type behind #if STRICT, that wouldn't break binary compatibility. If it breaks someone compiling something, they can always (100%) work around it. I wouldn't see it as a big deal to fix it, since it would be a compile time only problem and you're talking a minor change in limited places to impacted code. Compare it, for example, to the move from System.Security.Cryptography to Windows.Cryptography, that often requires significant code changes in Windows Phone apps being up-ported to UWP.

    1. kantos says:

      I know that some may disagree with me on this but if this was the ONLY API breaking change between SDK versions then I'd be fine doing it without guarding it behind STRICT on the basis that fixing it takes all of a few seconds and the use case for this function is.... minimal so the list of affected vendors should be short. What I would do in the immediate future is update the sample on MSDN to use an ifndef guard so that code that needs to fall back to void** can.

  6. Piotr Siódmak says:

    // remember: don't use this
    // this is a sample code! don't do that this way
    // this is intentionally wrong, don't do this!

    yeah, right, like that ever stopped anyone from copy-pasting samples from MSDN

    1. If you're going to intentionally copy code that advertises itself as broken and use it that way, then you're kind of asking for it. If you're copying a working example to get your prototype running quickly, that's what they're generally there for.

  7. Killer{R} says:

    Why not to return that cookie instead of redundant BOOL? NULL - error, not NULL - success and cookie in the hands. And greatly reduced risk of this copy-paste-induced error.. And only one drawback - is reduced amount of possible cookie values by one. But what is 1 in comparing with 2 ^ ( sizeof(void *) * 8 )...

    1. Dmitry says:

      I guess that because the main point of the function it to try to disable redirection, not to create a cookie. The cookie is provided to you as a convenient way to re-enable redirection if you would like to. In Windows API you can generally pass NULL pointer to say that you are not interested in a value. But if Windows gave you some handle you are obliged to keep care of it.

      1. GregM says:

        "I guess that because the main point of the function it to try to disable redirection, not to create a cookie. The cookie is provided to you as a convenient way to re-enable redirection if you would like to. In Windows API you can generally pass NULL pointer to say that you are not interested in a value."

        Nope, not here:

        https://msdn.microsoft.com/en-us/library/windows/desktop/aa365743(v=vs.85).aspx
        "Every successful call to the Wow64DisableWow64FsRedirection function must have a matching call to the Wow64RevertWow64FsRedirection function. This will ensure redirection is re-enabled and frees associated system resources."

        There's more information at that link about exactly why that is, and how this must be used over an extremely small region.

        1. So, what, if you just exit, the kernel leaks memory? Not bloody likely. (Though I do take your point that reverting should be the common case.)

          1. skSdnW says:

            No, the issue is that you cannot call LoadLibrary with redirection disabled (because it might try to load a 64-bit .dll and fail). Any API above Kernel32 might call LoadLibrary inside its functions. Shell32 for example has a bunch of delay-loaded function calls.

      2. Killer{R} says:

        First, as GregM said if you leave thread to live without reverting redirection - you just started great disaster, so you always should save the cookie and revert redirection state with it as soon as posisble.
        Second, Wow64RevertWow64FsRedirection pointer-to-cookie arg is not optional. So anyway you always should care about cookie. Furthermore, having cookie in return value instead would often allow you to save one line of code.

    2. that's actually a really great idea man. I totally agree. whenever I write a function that returns a pointer, I return it. null for failure non-null for success. instead of taking in a pointer to that pointer. it just makes more sense. the only downfall to this, is it doesn't really help the possible carelessness of Wow64RevertWow64FsRedirection at all. someone could still mistakenly pass the address of that to Wow64RevertWow64FsRedirection. the root problem is the untyped parameter. the ultimate solution would be to combine what both you and Raymond said. e.g. HWOW64COOKIE Wow64DisableWow64FsRedirection(); ...... HWOW64COOKIE cookie = Wow64DisableWow64FsRedirection(); ... yatta yatta .. Wow64RevertWow64FsRedirection(cookie);

  8. Sander Weerdenburg says:

    I like how the code comment is in italics too :)

Comments are closed.

Skip to main content