Cygwin: fix a race in the FAST_CWD fallback code

Guard the entire operation with the FastPebLock critical section
used by RtlSetCurrentDirectory_U as well, thus eliminating the
race between concurrent chdir/fchdir/SetCurrentDirectory calls.

Streamline comment explaining the fallback method.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
This commit is contained in:
Corinna Vinschen 2018-07-10 14:13:15 +02:00
parent 3a6833e3c4
commit 698d93c4b4
1 changed files with 17 additions and 22 deletions

View File

@ -4390,43 +4390,38 @@ cwdstuff::override_win32_cwd (bool init, ULONG old_dismount_count)
} }
else else
{ {
/* This is more a hack, and it's only used if we failed to find the /* Fallback if we failed to find the fast_cwd_ptr value:
fast_cwd_ptr value. We call RtlSetCurrentDirectory_U and let it
set up a new FAST_CWD structure. Afterwards, compute the address
of that structure utilizing the fact that the buffer address in
the user process parameter block is actually pointing to the buffer
in that FAST_CWD structure. Then replace the directory handle in
that structure with our own handle and close the original one.
Note that the call to RtlSetCurrentDirectory_U also closes our - Call RtlSetCurrentDirectory_U.
old dir handle, so there won't be any handle left open. - Compute new FAST_CWD struct address from buffer pointer in the
user process parameter block.
- Replace the directory handle in the struct with our own handle.
- Close the original handle. RtlSetCurrentDirectory_U already
closed our former dir handle -> no handle leak.
This method is prone to two race conditions: Guard the entire operation with FastPebLock to avoid races
accessing the PEB and FAST_CWD struct.
- Due to the way RtlSetCurrentDirectory_U opens the directory Unfortunately this method is still prone to a directory usage
handle, the directory is locked against deletion or renaming race condition:
between the RtlSetCurrentDirectory_U and the subsequent NtClose
call.
- When another thread calls SetCurrentDirectory at exactly the - The directory is locked against deletion or renaming between the
same time, a crash might occur, or worse, unrelated data could RtlSetCurrentDirectory_U and the subsequent NtClose call. */
be overwritten or NtClose could be called on an unrelated handle. if (unlikely (upp_cwd_hdl == NULL) && init)
return;
Therefore, use this *only* as a fallback. */ RtlEnterCriticalSection (peb.FastPebLock);
if (!init) if (!init)
{ {
NTSTATUS status = NTSTATUS status =
RtlSetCurrentDirectory_U (error ? &ro_u_pipedir : &win32); RtlSetCurrentDirectory_U (error ? &ro_u_pipedir : &win32);
if (!NT_SUCCESS (status)) if (!NT_SUCCESS (status))
{ {
RtlLeaveCriticalSection (peb.FastPebLock);
debug_printf ("RtlSetCurrentDirectory_U(%S) failed, %y", debug_printf ("RtlSetCurrentDirectory_U(%S) failed, %y",
error ? &ro_u_pipedir : &win32, status); error ? &ro_u_pipedir : &win32, status);
return; return;
} }
} }
else if (upp_cwd_hdl == NULL)
return;
RtlEnterCriticalSection (peb.FastPebLock);
fcwd_access_t::SetDirHandleFromBufferPointer(upp_cwd_str.Buffer, dir); fcwd_access_t::SetDirHandleFromBufferPointer(upp_cwd_str.Buffer, dir);
h = upp_cwd_hdl; h = upp_cwd_hdl;
upp_cwd_hdl = dir; upp_cwd_hdl = dir;