1.7.5: Occasional failure of CreatePipe or signal handing due to thread-unsafe code in cwdstuff::set

Corinna Vinschen corinna-cygwin@cygwin.com
Wed Aug 11 13:55:00 GMT 2010


Hi silently suffering community,

On Aug 11 10:49, Corinna Vinschen wrote:
> On Aug 10 21:53, John Carey wrote:
> > Now, where is Windows stuffing the extra copy of the directory handle?
> > In those reference-counted heap-allocated directory objects.  Stepping
> > through the machine code under Windows 7 I see SetCurrentDirectory()
> > updating a global pointer ("ntdll!RtlpCurDirRef") to one such object,
> > under the protection of a critical section ("ntdll!FastPebLock").
> > Despite mentioning "Peb" in the name, neither global's address is
> > computed using "FS:".  The global pointer points to a heap-allocated
> > block that starts with a reference count followed by a copy of the
> > directory handle value, and apparently includes other data as well.
> > SetCurrentDirectory() swaps in a new such block, and decrements the
> > counter on the old block, and if that reaches 0, closes the handle.
> > Anyway, this block appears to be where the old current directory
> > handle value persists past the changes made by cwdstuff::set().
> 
> ...and that handle is used in subsequent calls and potentially is a
> handle to another object in the meantime.
> 
> I do basically agree with cgf that it's your own problem if you use
> Win32 calls in your Cygwin app.  OTOH, I see that this can trip up
> potential handle problems in the DLL itself.
> 
> Unfortunately that means there's no Win32-safe way to set a new
> directory handle as cwd in Vista and later anymore.  Since there's no
> official API to set the cwd using a directory handle, there's no way to
> set the Win32 cwd to a directory with restricted permissions.
> This *is* frustrating.
> 
> I'll look into another solution.  Probably we will have to call
> SetCurrentDirectory again and ignore any error.  I don't accept the
> aforementioned restriction for POSIX calls.

I'm wondering what you say about this.  From John's description I take
it that the construct really is unsafe, and we can't discount the chance
that this race can break Cygwin itself in some scenarios.

Since SetCurrentDirectory appears to be the only valid way to change the
Win32 CWD in Vista and later, I think we should do so as well.

This means, the Win32 CWD will be wrong as soon as

- The path is not accessible to admins w/o backup privileges.

- The pathlen is > 259.

The latter is already true anyway since no tinkering with internal
Win32 structures will allow that.  At least the pathname in the PEB
is wrong, just the handle was correct so far.

That also leads to the question what to do in these cases?

 1) Return with error

    --> Disallows these paths in Cygwin's POSIX API as well.  Not an
	option, IMHO.

 2) Just ignore that SetCurrentDirectory failed?

    --> Win32 CWD == previous CWD.

 3) If SetCurrentDirectory fails, call

       SetCurrentDirectory (GetSystemDirectory ())

    That's basically what CMD.EXE does if it can't handle the current
    CWD at startup.  You can easily test that by setting the CWD to a
    network UNC path and start CMD.EXE.

I think option 3) is what we should use, but I'm open to other
suggestions.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat



More information about the Cygwin-developers mailing list