[newlib-cygwin] Cygwin: fix a race in the FAST_CWD fallback code

[Corinna Vinschen]

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=698d93c4b4edb442ca6ebae13c29e2d14feb047e

commit 698d93c4b4edb442ca6ebae13c29e2d14feb047e
Author: Corinna Vinschen <***@vinschen.de>
Date: Tue Jul 10 14:13:15 2018 +0200

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 <***@vinschen.de>

Diff:
---
winsup/cygwin/path.cc | 39 +++++++++++++++++----------------------
1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index d54ea1a..d56f22e 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -4390,43 +4390,38 @@ cwdstuff::override_win32_cwd (bool init, ULONG old_dismount_count)
}
else
{
- /* This is more a hack, and it's only used if we failed to find the
- 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.
+ /* Fallback if we failed to find the fast_cwd_ptr value:

- Note that the call to RtlSetCurrentDirectory_U also closes our
- old dir handle, so there won't be any handle left open.
+ - Call RtlSetCurrentDirectory_U.
+ - 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
- handle, the directory is locked against deletion or renaming
- between the RtlSetCurrentDirectory_U and the subsequent NtClose
- call.
+ Unfortunately this method is still prone to a directory usage
+ race condition:

- - When another thread calls SetCurrentDirectory at exactly the
- same time, a crash might occur, or worse, unrelated data could
- be overwritten or NtClose could be called on an unrelated handle.
-
- Therefore, use this *only* as a fallback. */
+ - The directory is locked against deletion or renaming between the
+ RtlSetCurrentDirectory_U and the subsequent NtClose call. */
+ if (unlikely (upp_cwd_hdl == NULL) && init)
+ return;
+ RtlEnterCriticalSection (peb.FastPebLock);
if (!init)
{
NTSTATUS status =
RtlSetCurrentDirectory_U (error ? &ro_u_pipedir : &win32);
if (!NT_SUCCESS (status))
{
+ RtlLeaveCriticalSection (peb.FastPebLock);
debug_printf ("RtlSetCurrentDirectory_U(%S) failed, %y",
error ? &ro_u_pipedir : &win32, status);
return;
}
}
- else if (upp_cwd_hdl == NULL)
- return;
- RtlEnterCriticalSection (peb.FastPebLock);
fcwd_access_t::SetDirHandleFromBufferPointer(upp_cwd_str.Buffer, dir);
h = upp_cwd_hdl;
upp_cwd_hdl = dir;