- Wine-GitLab - list.winehq.org

[PATCH v4 0/2] MR2752: combase: Prevent use-after-free due to a race with proxy_manager_destroy.
by Jinoh Kang (＠iamahuman) May 16, 2023

May 16, 2023

Today, find_proxy_manager() may return a proxy manager that is about to be freed. This happens when the proxy manager's reference count reaches zero, but proxy_manager_destroy() has not removed the proxy manager from the apartment proxy list. Fix this by only incrementing the reference count if it was nonzero. If the reference count is zero, proxy_manager_destroy() will proceed to destroy the proxy manager after the current thread releases the apartment lock (apt->cs). Granted, this is fragile and far from elegant, but it seems like a safe approach for the time being. Also, the critical section and "safe" refcount increment will prevent the following race condition scenario: > 1. Thread X: A proxy manager's refcount reaches 0 and is about to be released. > 2. Thread Y tries to grab it, calls AddRef, notices it returns 1, and drops it. > 3. Thread Z tries to grab it, calls AddRef, notices it returns 2, and returns it. > 4. Thread X then proceeds to remove it from the list and free the object. -- v4: combase: Prevent use-after-free due to a race with proxy_manager_destroy. https://gitlab.winehq.org/wine/wine/-/merge_requests/2752

4 4

[PATCH 0/4] MR2830: Reimplement dbghelp.EnumerateLoadedModules.
by eric pouech (＠epo) May 16, 2023

May 16, 2023

The aim of this serie is to (re)implement dbghelp.EnumerateLoadedModules in order to return correct 32bit & 64bit modules information for a 32bit debuggee from a 64bit debugger. - In this case, Wine ntdll (incorrectly) stores the paths in LdrData (for system modules) within the syswow64 directory while native stores them within the system32 directory (except for 32bit ntdll which Wine correctly stores within system32). - So now dbghelp implements the system32 => syswow64 path rewrite (for the relevant cases), which is currently used only for ntdll. - This allows: - to be transparent in dbghelp if ntdll is fixed to store the same path as native - the paths returned by dbghelp are consistent with native (in the 64bit debugger / 32bit debuggee case mentionned above) It must be noted that the remaining todo:s in tests are only for the case of a 32bit debugger / 32bit debuggee in (old|new) wow setup, where the returned paths are the (untouched from ntdll) paths, hence in syswow64 when native reports them from system32. - this patch doesn't modify the returned path, and we haven't noticed any issue so far - with redirection in place, access to the image files should be tranparent - so we can keep it as is (not fixing ntdll). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2830

3 5

[PATCH 0/3] MR206: vkd3d-shader/hlsl: Implement SampleCmp() method.
by Nikolay Sivov (＠nsivov) May 16, 2023

May 16, 2023

-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/206

3 9

[PATCH v8 0/7] MR193: vkd3d-shader/hlsl: Add support for writing RWStructuredBuffer declarations.
by Nikolay Sivov (＠nsivov) May 16, 2023

May 16, 2023

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- v8: tests: Add RWBuffer writing test. tests: Add support for UAV buffers to d3d12 runner. tests: Add support for UAV buffers in Vulkan runner. vkd3d-shader/hlsl: Improve UAV format type checking for buffer types. vkd3d-shader/hlsl: Add support for writing RWStructuredBuffer declarations. vkd3d-shader/hlsl: Add support for RWBuffer object. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/193

3 10

[PATCH v2 0/1] MR2818: quartz: Avoid reading past the end of a buffer (Valgrind).
by Rémi Bernon May 16, 2023

May 16, 2023

-- v2: quartz: Avoid reading past the end of a buffer (Valgrind). https://gitlab.winehq.org/wine/wine/-/merge_requests/2818

2 3

Re: [PATCH v2 0/1] MR2827: ntdll: Correctly identify hidden files
by David Kahurani (＠kahurani) May 16, 2023

May 16, 2023

On Tue May 16 14:05:22 2023 +0000, **** wrote: > Marvin replied on the mailing list: > ``` > Hi, > It looks like your patch introduced the new failures shown below. > Please investigate and fix them before resubmitting your patch. > If they are not new, fixing them anyway would help a lot. Otherwise > please ask for the known failures list to be updated. > The full results can be found at: > https://testbot.winehq.org/JobDetails.pl?Key=132727 > Your paranoid android. > === debian11 (build log) === > error: corrupt patch at line 38 > Task: Patch failed to apply > === debian11b (build log) === > error: corrupt patch at line 38 > Task: Patch failed to apply > ``` Interesting indeed, the issue is gone after adjusting the patch to fix the issue with directories. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2827#note_32894

1 0

Re: [PATCH v2 0/1] MR2827: ntdll: Correctly identify hidden files
by Sven Baars (＠sbaars) May 16, 2023

May 16, 2023

On Tue May 16 13:04:00 2023 +0000, David Kahurani wrote: > Yeah, I noticed that my changes alienate directories which should be > part of the "dot files" but I think I have come across a different > problem that might be harder to solve. > The tests don't seem to like these changes.... > ``` > GetTempPathA( MAX_PATH, temppath ); > GetTempFileNameA( temppath, ".foo", 0, filename ); > h = CreateFileA( filename, GENERIC_READ | GENERIC_WRITE, 0, NULL, > CREATE_ALWAYS, FILE_FLAG_DELETE_ON_CLOSE, 0 ); > ok( h != INVALID_HANDLE_VALUE, "failed to create temp file\n" ); > status = nt_get_file_attrs(filename, &attrs); > ok( status == STATUS_SUCCESS, "got %#lx\n", status ); > ok( !(attrs & FILE_ATTRIBUTE_HIDDEN), "got attributes %#lx\n", attrs ); > ``` > This test creates a temporary file(hidden as per its unix path but of > course not hidden as per Windows functioning). The test expects that the > file is not hidden but our changes make it so that the file is reported > as hidden. > I suspect the old code was carefully crafted so as to avoid marking a > file as hidden unless it is absolutely necessary. If I just do this, I don't seem to get any additional failures. We should add a test demonstrating the current problem though. ``` diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 0263d53716b..a740408fc58 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -1305,8 +1305,7 @@ static BOOL is_hidden_file( const char *name ) while (p > name && p[-1] != '/') p--; if (*p++ != '.') return FALSE; if (!*p || *p == '/') return FALSE; /* "." directory */ - if (*p++ != '.') return FALSE; - if (!*p || *p == '/') return FALSE; /* ".." directory */ + if (*p++ == '.' && (!*p || *p == '/')) return FALSE; /* ".." directory */ return TRUE; } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2827#note_32892

2 1

[PATCH v3 0/2] MR2752: combase: Prevent use-after-free due to a race with proxy_manager_destroy.
by Jinoh Kang (＠iamahuman) May 16, 2023

May 16, 2023

Today, find_proxy_manager() may return a proxy manager that is about to be freed. This happens when the proxy manager's reference count reaches zero, but proxy_manager_destroy() has not removed the proxy manager from the apartment proxy list. Fix this by only incrementing the reference count if it was nonzero. If the reference count is zero, proxy_manager_destroy() will proceed to destroy the proxy manager after the current thread releases the apartment lock (apt->cs). Granted, this is fragile and far from elegant, but it seems like a safe approach for the time being. Also, the critical section and "safe" refcount increment will prevent the following race condition scenario: > 1. Thread X: A proxy manager's refcount reaches 0 and is about to be released. > 2. Thread Y tries to grab it, calls AddRef, notices it returns 1, and drops it. > 3. Thread Z tries to grab it, calls AddRef, notices it returns 2, and returns it. > 4. Thread X then proceeds to remove it from the list and free the object. -- v3: sombase: Prevent use-after-free due to a race with proxy_manager_destroy. https://gitlab.winehq.org/wine/wine/-/merge_requests/2752

3 3

[PATCH v2 0/2] MR2752: combase: Prevent use-after-free due to a race with proxy_manager_destroy.
by Jinoh Kang (＠iamahuman) May 16, 2023

May 16, 2023

Today, find_proxy_manager() may return a proxy manager that is about to be freed. This happens when the proxy manager's reference count reaches zero, but proxy_manager_destroy() has not removed the proxy manager from the apartment proxy list. Fix this by only incrementing the reference count if it was nonzero. If the reference count is zero, proxy_manager_destroy() will proceed to destroy the proxy manager after the current thread releases the apartment lock (apt->cs). Granted, this is fragile and far from elegant, but it seems like a safe approach for the time being. Also, the critical section and "safe" refcount increment will prevent the following race condition scenario: > 1. Thread X: A proxy manager's refcount reaches 0 and is about to be released. > 2. Thread Y tries to grab it, calls AddRef, notices it returns 1, and drops it. > 3. Thread Z tries to grab it, calls AddRef, notices it returns 2, and returns it. > 4. Thread X then proceeds to remove it from the list and free the object. -- v2: combase: Prevent use-after-free due to a race with proxy_manager_destroy. https://gitlab.winehq.org/wine/wine/-/merge_requests/2752

3 4

Re: [PATCH v2 0/1] MR2827: ntdll: Correctly identify hidden files
by David Kahurani (＠kahurani) May 16, 2023

May 16, 2023

On Tue May 16 12:34:25 2023 +0000, Sven Baars wrote: > This line and the following line should probably be combined into one to > fix the issue. Yeah, I noticed that my changes alienate directories which should be part of the "dot files" but I think I have come across a different problem that might be harder to solve. The tests don't seem to like these changes.... ``` GetTempPathA( MAX_PATH, temppath ); GetTempFileNameA( temppath, ".foo", 0, filename ); h = CreateFileA( filename, GENERIC_READ | GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_FLAG_DELETE_ON_CLOSE, 0 ); ok( h != INVALID_HANDLE_VALUE, "failed to create temp file\n" ); status = nt_get_file_attrs(filename, &attrs); ok( status == STATUS_SUCCESS, "got %#lx\n", status ); ok( !(attrs & FILE_ATTRIBUTE_HIDDEN), "got attributes %#lx\n", attrs ); ``` This test creates a temporary file(hidden as per its unix path but of course not hidden as per Windows functioning). The test expects that the file is not hidden but our changes make it so that the file is reported as hidden. I suspect the old code was carefully crafted so as to avoid marking a file as hidden unless it is absolutely necessary. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2827#note_32883

1 0