June 2022 - Wine-Devel - list.winehq.org

Re: [PATCH 0/1] MR186: secur32/schannel: Avoid use-after-free in AcquireClientCredentials() (Coverity). - approved

by Hans Leidekker (＠hans)

This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/186

June 6, 2022

Re: [PATCH v2 0/9] MR180: misc: Fixes and workarounds for UAF warnings. (GCC12)

by Alexandre Julliard (＠julliard)

On Mon Jun 6 06:09:19 2022 +0000, Rémi Bernon wrote: > You're expected to approve the MR if you're okay with the changes, but I > see you're not in the reviewers so I think you need to first request > project access on https://gitlab.winehq.org/wine/wine to get the > permission to review. Also if you don't mind, could you please use your full name in your gitlab profile? This is used to match with the MAINTAINERS file so that you can be assigned as reviewer automatically. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/180#note_1600

June 6, 2022

[PATCH v2] include: Define atomic read/write helpers for 32-bit integers.

by Jinoh Kang

Based on the corresponding functions from Windows SDK header files. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- v1 -> v2: Remove *ULong* wrappers (not needed immediately) include/winnt.h | 92 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/include/winnt.h b/include/winnt.h index 87c4b4da92d..370ed3adce6 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -6475,6 +6475,61 @@ static FORCEINLINE void MemoryBarrier(void) #endif /* __i386__ */ +/* Since Visual Studio 2012, volatile accesses do not always imply acquire and + * release semantics. We explicitly use ISO volatile semantics, manually + * placing barriers as appropriate. + */ +#if _MSC_VER >= 1700 +#pragma intrinsic(__iso_volatile_load32) +#pragma intrinsic(__iso_volatile_store32) +#define __WINE_ISO_VOLATILE_LOAD_32(src) (__iso_volatile_load32(src)) +#define __WINE_ISO_VOLATILE_STORE_32(dest, value) (__iso_volatile_store32(dest, value)) +#else /* _MSC_VER >= 1700 */ +#define __WINE_ISO_VOLATILE_LOAD_32(src) (*(src)) +#define __WINE_ISO_VOLATILE_STORE_32(dest, value) ((void)(*(dest) = (value))) +#endif /* _MSC_VER >= 1700 */ + +#if defined(__i386__) || defined(__x86_64__) +#pragma intrinsic(_ReadWriteBarrier) +void _ReadWriteBarrier(void); +#endif /* defined(__i386__) || defined(__x86_64__) */ + +static void __wine_memory_barrier_acq_rel(void) +{ +#if defined(__i386__) || defined(__x86_64__) +#pragma warning(suppress:4996) + _ReadWriteBarrier(); +#elif defined(__arm__) + __dmb(_ARM_BARRIER_ISH); +#elif defined(__aarch64__) + __dmb(_ARM64_BARRIER_ISH); +#endif /* defined(__i386__) || defined(__x86_64__) */ +} + +static FORCEINLINE LONG ReadAcquire( LONG const volatile *src ) +{ + LONG value = __WINE_ISO_VOLATILE_LOAD_32( (int const volatile *)src ); + __wine_memory_barrier_acq_rel(); + return value; +} + +static FORCEINLINE LONG ReadNoFence( LONG const volatile *src ) +{ + LONG value = __WINE_ISO_VOLATILE_LOAD_32( (int const volatile *)src ); + return value; +} + +static FORCEINLINE void WriteRelease( LONG volatile *dest, LONG value ) +{ + __wine_memory_barrier_acq_rel(); + __WINE_ISO_VOLATILE_STORE_32( (int volatile *)dest, value ); +} + +static FORCEINLINE void WriteNoFence( LONG volatile *dest, LONG value ) +{ + __WINE_ISO_VOLATILE_STORE_32( (int volatile *)dest, value ); +} + #elif defined(__GNUC__) static FORCEINLINE BOOLEAN WINAPI BitScanForward(DWORD *index, DWORD mask) @@ -6588,6 +6643,43 @@ static FORCEINLINE void MemoryBarrier(void) __sync_synchronize(); } +#if defined(__x86_64__) || defined(__i386__) +/* On x86, Support old GCC with either no or buggy (GCC BZ#81316) __atomic_* support */ +#define __WINE_ATOMIC_LOAD_ACQUIRE(ptr, ret) do { *(ret) = *(ptr); __asm__ __volatile__( "" ::: "memory" ); } while (0) +#define __WINE_ATOMIC_LOAD_RELAXED(ptr, ret) do { *(ret) = *(ptr); } while (0) +#define __WINE_ATOMIC_STORE_RELEASE(ptr, val) do { __asm__ __volatile__( "" ::: "memory" ); *(ptr) = *(val); } while (0) +#define __WINE_ATOMIC_STORE_RELAXED(ptr, val) do { *(ptr) = *(val); } while (0) +#else +#define __WINE_ATOMIC_LOAD_ACQUIRE(ptr, ret) __atomic_load(ptr, ret, __ATOMIC_ACQUIRE) +#define __WINE_ATOMIC_LOAD_RELAXED(ptr, ret) __atomic_load(ptr, ret, __ATOMIC_RELAXED) +#define __WINE_ATOMIC_STORE_RELEASE(ptr, val) __atomic_store(ptr, val, __ATOMIC_RELEASE) +#define __WINE_ATOMIC_STORE_RELAXED(ptr, val) __atomic_store(ptr, val, __ATOMIC_RELAXED) +#endif /* defined(__x86_64__) || defined(__i386__) */ + +static FORCEINLINE LONG ReadAcquire( LONG const volatile *src ) +{ + LONG value; + __WINE_ATOMIC_LOAD_ACQUIRE( src, &value ); + return value; +} + +static FORCEINLINE LONG ReadNoFence( LONG const volatile *src ) +{ + LONG value; + __WINE_ATOMIC_LOAD_RELAXED( src, &value ); + return value; +} + +static FORCEINLINE void WriteRelease( LONG volatile *dest, LONG value ) +{ + __WINE_ATOMIC_STORE_RELEASE( dest, &value ); +} + +static FORCEINLINE void WriteNoFence( LONG volatile *dest, LONG value ) +{ + __WINE_ATOMIC_STORE_RELAXED( dest, &value ); +} + static FORCEINLINE DECLSPEC_NORETURN void __fastfail(unsigned int code) { #if defined(__x86_64__) || defined(__i386__) -- 2.35.3

June 6, 2022

Re: [PATCH v2 0/9] MR180: misc: Fixes and workarounds for UAF warnings. (GCC12) - approved

by Hans Leidekker (＠hans)

This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/180

June 6, 2022

Re: [PATCH 4/5] d2d1: Implement RegisterEffectFromString().

by Nikolay Sivov

On 6/6/22 10:35, Ziqing Hui wrote: > + size = sizeof(*property_xml) * (wcslen(property_xml) + 1); > + hglobal = GlobalAlloc(0, size); > + data = GlobalLock(hglobal); > + memcpy(data, property_xml, size); > + GlobalUnlock(hglobal); > + > + if (FAILED(hr = CreateStreamOnHGlobal(hglobal, TRUE, &stream))) > + return hr; Seems easier to write whole string in a stream, and seek back.

June 6, 2022

Re: [PATCH 3/5] d2d1: Implement property parsing for RegisterEffectFromStream().

by Nikolay Sivov

On 6/6/22 10:35, Ziqing Hui wrote: > struct d2d_effect_info > { > const CLSID *clsid; > UINT32 default_input_count; > UINT32 min_inputs; > UINT32 max_inputs; > + WCHAR *display_name; > + WCHAR *author; > + WCHAR *category; > + WCHAR *description; > }; Shouldn't these also be expressed as properties? > +static D2D1_PROPERTY_TYPE str_to_property_type(const WCHAR *str) > +{ > + unsigned int i; > + > + static const WCHAR *type_str[] = > + { > + L"", L"string", L"bool", L"uint32", L"int32", L"float", L"vector2", L"vector3", > + L"vector4", L"blob", L"iunknown", L"enum", L"array", L"clsid", L"matrix3x2", > + L"matrix4x3", L"matrix4x4", L"matrix5x4", L"colorcontext", > + }; > + > + for (i = 1; i < ARRAY_SIZE(type_str); ++i) > + { > + if (!wcscmp(str, type_str[i])) > + return i; > + } > + > + return D2D1_PROPERTY_TYPE_UNKNOWN; > +} This uses numeric values of D2D1_PROPERTY_TYPE, that deserves at least a comment.

June 6, 2022

Re: [PATCH 2/5] d2d1: Partially implement RegisterEffectFromStream().

by Nikolay Sivov

On 6/6/22 10:35, Ziqing Hui wrote: > +struct d2d_effect_reg > +{ > + PD2D1_EFFECT_FACTORY factory; > + UINT32 count; > + > + struct d2d_effect_info *info; > + > + struct list entry; > +}; You don't need to allocate 'info' separately. > + memset(node_name, 0, sizeof(node_name)); > + while (IXmlReader_Read(xml_reader, &type) == S_OK) > + { > + if (FAILED(hr = IXmlReader_GetDepth(xml_reader, &depth))) > + goto done; > + if (depth >= 3) > + continue; > + if (FAILED(hr = IXmlReader_GetLocalName(xml_reader, &value, &len))) > + goto done; > + if (node_name[depth] && wcslen(node_name[depth]) <= len) > + { > + wcscpy(node_name[depth], value); > + } > + else > + { > + heap_free(node_name[depth]); > + node_name[depth] = heap_strdupW(value); > + } > + > + if (type != XmlNodeType_Element) > + continue; Depth should not be used for this. What you need is to first find "Effect" element. After that loop within it, until you run out of nested elements. You can then have helpers to process specific elements like Property or Inputs. > + if (!wcscmp(node_name[depth - 1], L"Effect") > + && !wcscmp(node_name[depth], L"Property")) Is it really case-sensitive? > + reg->info->default_input_count = input_count; > + reg->info->min_inputs = input_count; > + reg->info->max_inputs = input_count; Is this supposed to be subproperties of an Input? Maybe we should have basic property support first, and store it there. Is min/max/default expressible in effect xml? <Input> attributes maybe? > + entry->count = 1; > + entry->info->clsid = effect_id; > + entry->factory = effect_factory; > + list_add_tail(&factory->effect_regs, &entry->entry); This should store GUID itself, not a pointer.

June 6, 2022

Re: [PATCH 5/5] d2d1: Implement UnregisterEffect().

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=116285 Your paranoid android. === debian11 (64 bit WoW report) === d2d1: d2d1: Timeout

June 6, 2022

[PATCH 5/5] d2d1: Implement UnregisterEffect().

by Ziqing Hui

Signed-off-by: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/d2d1/factory.c | 21 +++++++++++++++++++-- dlls/d2d1/tests/d2d1.c | 28 ++++++++++++++-------------- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/dlls/d2d1/factory.c b/dlls/d2d1/factory.c index 1d7c8325077..aac00a50a2a 100644 --- a/dlls/d2d1/factory.c +++ b/dlls/d2d1/factory.c @@ -882,9 +882,26 @@ static HRESULT STDMETHODCALLTYPE d2d_factory_RegisterEffectFromString(ID2D1Facto static HRESULT STDMETHODCALLTYPE d2d_factory_UnregisterEffect(ID2D1Factory3 *iface, REFCLSID effect_id) { - FIXME("iface %p, effect_id %s stub!\n", iface, debugstr_guid(effect_id)); + struct d2d_factory *factory = impl_from_ID2D1Factory3(iface); + struct d2d_effect_reg *iter, *iter2; - return E_NOTIMPL; + TRACE("iface %p, effect_id %s.\n", iface, debugstr_guid(effect_id)); + + LIST_FOR_EACH_ENTRY_SAFE(iter, iter2, &factory->effect_regs, struct d2d_effect_reg, entry) + { + if (IsEqualGUID(effect_id, iter->info->clsid)) + { + --iter->count; + if (!iter->count) + { + list_remove(&iter->entry); + d2d_effect_reg_cleanup(iter); + } + return S_OK; + } + } + + return D2DERR_EFFECT_IS_NOT_REGISTERED; } static HRESULT STDMETHODCALLTYPE d2d_factory_GetRegisteredEffects(ID2D1Factory3 *iface, diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index 3e385b2b05f..15b106288a8 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -10694,7 +10694,8 @@ static void test_effect_register(BOOL d3d11) winetest_push_context("Test %u", i); hr = ID2D1Factory1_RegisterEffectFromString(factory, &CLSID_TestEffect, test->xml, NULL, 0, effect_impl_create); - todo_wine_if(test->hr != S_OK) + todo_wine_if(test->xml == effect_xml_without_version + || test->xml == effect_xml_without_inputs) ok(hr == test->hr, "Got unexpected hr %#lx, expected %#lx.\n", hr, test->hr); if (hr == S_OK) { @@ -10702,9 +10703,9 @@ static void test_effect_register(BOOL d3d11) hr = ID2D1DeviceContext_CreateEffect(device_context, &CLSID_TestEffect, &effect); todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == D2DERR_EFFECT_IS_NOT_REGISTERED, "Got unexpected hr %#lx.\n", hr); + ok(hr == D2DERR_EFFECT_IS_NOT_REGISTERED, "Got unexpected hr %#lx.\n", hr); if (effect) ID2D1Effect_Release(effect); } @@ -10727,11 +10728,11 @@ static void test_effect_register(BOOL d3d11) hr = ID2D1Factory1_RegisterEffectFromString(factory, &CLSID_TestEffect, test->xml, NULL, 0, effect_impl_create); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == D2DERR_EFFECT_IS_NOT_REGISTERED, "Got unexpected hr %#lx.\n", hr); + ok(hr == D2DERR_EFFECT_IS_NOT_REGISTERED, "Got unexpected hr %#lx.\n", hr); winetest_pop_context(); } @@ -10766,9 +10767,9 @@ static void test_effect_register(BOOL d3d11) } hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); /* Register effect with property binding */ for (i = 0; i < ARRAY_SIZE(binding_tests); ++i) @@ -10827,13 +10828,13 @@ static void test_effect_register(BOOL d3d11) } hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); /* Unregister builtin effect */ hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_D2D1Composite); - todo_wine ok(hr == D2DERR_EFFECT_IS_NOT_REGISTERED, "Got unexpected hr %#lx.\n", hr); + ok(hr == D2DERR_EFFECT_IS_NOT_REGISTERED, "Got unexpected hr %#lx.\n", hr); release_test_context(&ctx); } @@ -10911,7 +10912,6 @@ done: if (effect) ID2D1Effect_Release(effect); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); release_test_context(&ctx); } @@ -11068,7 +11068,7 @@ static void test_effect_properties(BOOL d3d11) if (effect) ID2D1Effect_Release(effect); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); winetest_pop_context(); } @@ -11161,7 +11161,7 @@ done: if (effect) ID2D1Effect_Release(effect); hr = ID2D1Factory1_UnregisterEffect(factory, &CLSID_TestEffect); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); release_test_context(&ctx); } -- 2.25.1

June 6, 2022

[PATCH 4/5] d2d1: Implement RegisterEffectFromString().

by Ziqing Hui

Signed-off-by: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/d2d1/factory.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/dlls/d2d1/factory.c b/dlls/d2d1/factory.c index b3b65b0cb92..1d7c8325077 100644 --- a/dlls/d2d1/factory.c +++ b/dlls/d2d1/factory.c @@ -856,10 +856,28 @@ static HRESULT STDMETHODCALLTYPE d2d_factory_RegisterEffectFromString(ID2D1Facto REFCLSID effect_id, const WCHAR *property_xml, const D2D1_PROPERTY_BINDING *bindings, UINT32 binding_count, PD2D1_EFFECT_FACTORY effect_factory) { - FIXME("iface %p, effect_id %s, property_xml %s, bindings %p, binding_count %u, effect_factory %p stub!\n", - iface, debugstr_guid(effect_id), debugstr_w(property_xml), bindings, binding_count, effect_factory); + HGLOBAL hglobal; + IStream *stream; + size_t size; + BYTE *data; + HRESULT hr; - return S_OK; + TRACE("iface %p, effect_id %s, property_xml %s, bindings %p, binding_count %u, effect_factory %p.\n", + iface, debugstr_guid(effect_id), debugstr_w(property_xml), bindings, binding_count, effect_factory); + + size = sizeof(*property_xml) * (wcslen(property_xml) + 1); + hglobal = GlobalAlloc(0, size); + data = GlobalLock(hglobal); + memcpy(data, property_xml, size); + GlobalUnlock(hglobal); + + if (FAILED(hr = CreateStreamOnHGlobal(hglobal, TRUE, &stream))) + return hr; + + hr = ID2D1Factory3_RegisterEffectFromStream(iface, effect_id, stream, bindings, binding_count, effect_factory); + + IStream_Release(stream); + return hr; } static HRESULT STDMETHODCALLTYPE d2d_factory_UnregisterEffect(ID2D1Factory3 *iface, REFCLSID effect_id) -- 2.25.1

June 6, 2022