From 87dc6a7b2473a0c3326bc4b83d5f88798b88892b Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 27 Nov 2025 12:17:03 +0100 Subject: [PATCH] win: fix race in uv_fs_event_start() Fetch file metadata by handle instead of by path, otherwise there is a race window between fetching and acting on said metadata where another process can replace the file with another one. There is still a potential race when upgrading the short path to a long path but that that one is intrinsic. It's not something libuv can solve except by refraining from calling GetLongPathName. Fixes: https://github.com/libuv/libuv/issues/4568 --- src/win/fs-event.c | 108 +++++++++++++++++++++++++-------------------- 1 file changed, 59 insertions(+), 49 deletions(-) diff --git a/src/win/fs-event.c b/src/win/fs-event.c index 1bbb8c52b..fd77f2a49 100644 --- a/src/win/fs-event.c +++ b/src/win/fs-event.c @@ -158,14 +158,14 @@ int uv_fs_event_start(uv_fs_event_t* handle, const char* path, unsigned int flags) { int is_path_dir; - size_t size; - DWORD attr, last_error; - WCHAR* dir = NULL, *dir_to_watch, *pathw = NULL; + DWORD last_error; + WCHAR* dir, *pathw = NULL; DWORD short_path_buffer_len; WCHAR *short_path_buffer; - WCHAR* short_path, *long_path; + WCHAR* short_path = NULL; + HANDLE file_handle = INVALID_HANDLE_VALUE; + BY_HANDLE_FILE_INFORMATION info; - short_path = NULL; if (uv__is_active(handle)) return UV_EINVAL; @@ -181,43 +181,26 @@ int uv_fs_event_start(uv_fs_event_t* handle, if (last_error) goto error_uv; - /* Determine whether path is a file or a directory. */ - attr = GetFileAttributesW(pathw); - if (attr == INVALID_FILE_ATTRIBUTES) { + file_handle = CreateFileW(pathw, + FILE_LIST_DIRECTORY, + FILE_SHARE_READ|FILE_SHARE_DELETE|FILE_SHARE_WRITE, + NULL, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS|FILE_FLAG_OVERLAPPED, + NULL); + if (file_handle == INVALID_HANDLE_VALUE) { last_error = GetLastError(); goto error; } - is_path_dir = (attr & FILE_ATTRIBUTE_DIRECTORY) ? 1 : 0; + if (!GetFileInformationByHandle(file_handle, &info)) { + last_error = GetLastError(); + goto error; + } - if (is_path_dir) { - /* path is a directory, so that's the directory that we will watch. */ + is_path_dir = (info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ? 1 : 0; - /* Convert to long path. */ - size = GetLongPathNameW(pathw, NULL, 0); - - if (size) { - long_path = (WCHAR*)uv__malloc(size * sizeof(WCHAR)); - if (!long_path) { - uv_fatal_error(ERROR_OUTOFMEMORY, "uv__malloc"); - } - - size = GetLongPathNameW(pathw, long_path, size); - if (size) { - long_path[size] = '\0'; - } else { - uv__free(long_path); - long_path = NULL; - } - - if (long_path) { - uv__free(pathw); - pathw = long_path; - } - } - - dir_to_watch = pathw; - } else { + if (!is_path_dir) { /* * path is a file. So we split path into dir & file parts, and * watch the dir directory. @@ -252,28 +235,50 @@ short_path_done: goto error; } - dir_to_watch = dir; uv__free(short_path); short_path = NULL; uv__free(pathw); pathw = NULL; - } - handle->dir_handle = CreateFileW(dir_to_watch, - FILE_LIST_DIRECTORY, - FILE_SHARE_READ | FILE_SHARE_DELETE | - FILE_SHARE_WRITE, - NULL, - OPEN_EXISTING, - FILE_FLAG_BACKUP_SEMANTICS | - FILE_FLAG_OVERLAPPED, - NULL); - - if (dir) { + /* Open the containing directory and watch that instead. Events for + * other files are filtered out in uv__process_fs_event_req(). + * Not super efficient but c'est ça. + */ + CloseHandle(file_handle); + file_handle = CreateFileW(dir, + FILE_LIST_DIRECTORY, + FILE_SHARE_READ|FILE_SHARE_DELETE|FILE_SHARE_WRITE, + NULL, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS|FILE_FLAG_OVERLAPPED, + NULL); uv__free(dir); dir = NULL; + if (file_handle == INVALID_HANDLE_VALUE) { + last_error = GetLastError(); + goto error; + } + + if (!GetFileInformationByHandle(file_handle, &info)) { + last_error = GetLastError(); + goto error; + } + + /* Race with another process: directory foo in foo/bar was replaced + * with a file. Bail out with an error, we're not recursing upwards. + */ + if (!(info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) { + /* TODO(bnoordhuis) ERROR_DIRECTORY is translated to UV_ENOENT, + * there's currently nothing that maps to UV_ENOTDIR. + */ + last_error = ERROR_DIRECTORY; + goto error; + } } + handle->dir_handle = file_handle; + file_handle = INVALID_HANDLE_VALUE; + if (handle->dir_handle == INVALID_HANDLE_VALUE) { last_error = GetLastError(); goto error; @@ -325,6 +330,11 @@ error: last_error = uv_translate_sys_error(last_error); error_uv: + if (file_handle != INVALID_HANDLE_VALUE) { + CloseHandle(file_handle); + file_handle = INVALID_HANDLE_VALUE; + } + if (handle->path) { uv__free(handle->path); handle->path = NULL;