This is the mail archive of the cygwin-developers mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Wed, Aug 11, 2010 at 03:55:28PM +0200, Corinna Vinschen wrote:
>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.

4) Break path into convenient chunks and first change to an absolute path
whose length is <= MAXPATHLEN and then change to relative paths whose
lenghts are also <= MAXPATHLEN.

I thought that this technique worked but maybe I'm mistaken.  If it does,
it would be a foolproof but slow way of cd'ing to a long path.

cgf


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]