Signed-off-by: Daniel Lehman dlehman25@gmail.com
--- matches sequence in NtClose
i can submit a test that deliberately fails if preferred, but results vary it takes ~4 seconds for 32 threads to hit it on my box
There's a race condition with DuplicateHandle and the fd_cache: - Thread 1 calls DuplicateHandle(CLOSE_SOURCE), which is 3 operations: - req_dup_handle - remove_fd_from_cache - close - Thread 2 calls CreateFile, then ReadFile (which calls server_get_unix_fd) - CreateFile - req_create_file - ReadFile - server_get_unix_fd - get_cached_fd (before lock) - if uncached: - get_cached_fd (inside lock) - req_get_handle_fd - add_fd_to_cache - read/pread
The problem is when a thread caches a handle in the fd_cache, then duplicates and closes it. The closing of the handle is done on the wineserver during duplication, before it is removed from the fd_cache on the client side. This allows another thread opening a file to use the just-closed handle for another file. Because the fd_cache entry for that handle hasn't been invalidated by the first thread, another thread can see the old fd entry. This can lead to accessing the wrong fd or seeing an invalid fd if the first thread calls close().
T1 T2 fd_cache Notes --------------------- --------------------------- ----------- ------------------------ (T1 opened 'foo.txt' as 0x100) - 0x100 -> 4 T1 has already mapped 0x100 as fd 4 in fd_cache req_dup_handle(0x100, CLOSE_SOURCE) - 0x100 -> 4 T1 duplicates handle closing source - - 0x100 -> 4 wineserver closes 0x100, handle now available - req_create_file(bar.txt) 0x100 -> 4 T2 opens 'bar.txt', gets 0x100, but fd_cache still has 0x100 -> 4 - get_cached_fd(0x100) -> 4 0x100 -> 4 T2 sees existing 0x100 in fd_cache, gets fd 4 (foo.txt) remove_fd_from_cache(0x100) - 0x100 -> X T1 removes handle from cache _after_ T2 fetched it - pread(4 -> 'foo.txt) 0x100 -> X T2 reads from fd 4 ('foo.txt') instead of 'bar.txt' close(4) - T1 finally closes fd 4. this could also happen before T2's read and lead to EBADF --- dlls/ntdll/unix/server.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index db09d7759da..fc555a7f5f1 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -1639,7 +1639,10 @@ NTSTATUS WINAPI NtDuplicateObject( HANDLE source_process, HANDLE source, HANDLE ACCESS_MASK access, ULONG attributes, ULONG options ) { NTSTATUS ret; + int fd = -1;
+ if (options & DUPLICATE_CLOSE_SOURCE) + fd = remove_fd_from_cache( source ); SERVER_START_REQ( dup_handle ) { req->src_process = wine_server_obj_handle( source_process ); @@ -1651,11 +1654,8 @@ NTSTATUS WINAPI NtDuplicateObject( HANDLE source_process, HANDLE source, HANDLE if (!(ret = wine_server_call( req ))) { if (dest) *dest = wine_server_ptr_handle( reply->handle ); - if (reply->closed && reply->self) - { - int fd = remove_fd_from_cache( source ); - if (fd != -1) close( fd ); - } + if (reply->closed && reply->self && (fd != -1)) + close( fd ); } } SERVER_END_REQ;