[PATCH 1/3] ole32: Fix IMalloc::DidAlloc() return value to reflect block validity.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/ole32/ifs.c | 59 ++++++++++++++++++++++++-------------- dlls/ole32/tests/compobj.c | 21 ++++++++++++++ 2 files changed, 58 insertions(+), 22 deletions(-) diff --git a/dlls/ole32/ifs.c b/dlls/ole32/ifs.c index 9c7cb0977f..e747cfc0fe 100644 --- a/dlls/ole32/ifs.c +++ b/dlls/ole32/ifs.c @@ -114,6 +114,20 @@ static BOOL AddMemoryLocation(LPVOID * pMem) return TRUE; } +static void** mallocspy_is_allocation_spyed(const void *mem) +{ + void **current = Malloc32.SpyedBlocks; + + while (*current != mem) + { + current++; + if (current >= Malloc32.SpyedBlocks + Malloc32.SpyedBlockTableLength) + return NULL; + } + + return current; +} + static BOOL RemoveMemoryLocation(LPCVOID pMem) { LPVOID * Current; @@ -122,14 +136,8 @@ static BOOL RemoveMemoryLocation(LPCVOID pMem) if (!Malloc32.SpyedBlockTableLength && !SetSpyedBlockTableLength(0x1000)) return FALSE; - Current = Malloc32.SpyedBlocks; - - /* find the location */ - while (*Current != pMem) { - Current++; - if (Current >= Malloc32.SpyedBlocks + Malloc32.SpyedBlockTableLength) - return FALSE; /* not found */ - } + if (!(Current = mallocspy_is_allocation_spyed(pMem))) + return FALSE; /* location found */ Malloc32.SpyedAllocationsLeft--; @@ -313,25 +321,32 @@ static SIZE_T WINAPI IMalloc_fnGetSize(IMalloc *iface, void *pv) /****************************************************************************** * IMalloc32_DidAlloc [VTABLE] */ -static INT WINAPI IMalloc_fnDidAlloc(IMalloc *iface, void *pv) +static INT WINAPI IMalloc_fnDidAlloc(IMalloc *iface, void *mem) { - BOOL fSpyed = FALSE; - int didAlloc; + BOOL spyed_block = FALSE; + int did_alloc; - TRACE("(%p)\n",pv); + TRACE("(%p)\n", mem); - if(Malloc32.pSpy) { - EnterCriticalSection(&IMalloc32_SpyCS); - pv = IMallocSpy_PreDidAlloc(Malloc32.pSpy, pv, fSpyed); - } + if (!mem) + return -1; + + if (Malloc32.pSpy) + { + EnterCriticalSection(&IMalloc32_SpyCS); + spyed_block = !!mallocspy_is_allocation_spyed(mem); + mem = IMallocSpy_PreDidAlloc(Malloc32.pSpy, mem, spyed_block); + } - didAlloc = -1; + did_alloc = HeapValidate(GetProcessHeap(), 0, mem); - if(Malloc32.pSpy) { - didAlloc = IMallocSpy_PostDidAlloc(Malloc32.pSpy, pv, fSpyed, didAlloc); - LeaveCriticalSection(&IMalloc32_SpyCS); - } - return didAlloc; + if (Malloc32.pSpy) + { + did_alloc = IMallocSpy_PostDidAlloc(Malloc32.pSpy, mem, spyed_block, did_alloc); + LeaveCriticalSection(&IMalloc32_SpyCS); + } + + return did_alloc; } /****************************************************************************** diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c index cc2737bdd1..b0a6ba1ea7 100644 --- a/dlls/ole32/tests/compobj.c +++ b/dlls/ole32/tests/compobj.c @@ -3069,6 +3069,8 @@ static void test_CoGetMalloc(void) { IMalloc *imalloc; HRESULT hr; + char *ptr; + int ret; if (0) /* crashes on native */ hr = CoGetMalloc(0, NULL); @@ -3102,6 +3104,25 @@ static void test_CoGetMalloc(void) hr = CoGetMalloc(MEMCTX_TASK, &imalloc); ok(hr == S_OK, "got 0x%08x\n", hr); ok(imalloc != NULL, "got %p\n", imalloc); + + /* DidAlloc() */ + ptr = IMalloc_Alloc(imalloc, 16); + ok(!!ptr, "Failed to allocate block.\n"); + + ret = IMalloc_DidAlloc(imalloc, ptr); + ok(ret == 1, "Unexpected return value %d.\n", ret); + + ret = IMalloc_DidAlloc(imalloc, NULL); + ok(ret == -1, "Unexpected return value %d.\n", ret); + + ret = IMalloc_DidAlloc(imalloc, (void *)0x1); + ok(ret == 0, "Unexpected return value %d.\n", ret); + + ret = IMalloc_DidAlloc(imalloc, ptr + 4); + ok(ret == 0, "Unexpected return value %d.\n", ret); + + IMalloc_Free(imalloc, ptr); + IMalloc_Release(imalloc); } -- 2.25.1
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- If spy is set after initial null check, it will touch critical section it didn't acquire. dlls/ole32/ifs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dlls/ole32/ifs.c b/dlls/ole32/ifs.c index e747cfc0fe..410691b2a8 100644 --- a/dlls/ole32/ifs.c +++ b/dlls/ole32/ifs.c @@ -323,7 +323,7 @@ static SIZE_T WINAPI IMalloc_fnGetSize(IMalloc *iface, void *pv) */ static INT WINAPI IMalloc_fnDidAlloc(IMalloc *iface, void *mem) { - BOOL spyed_block = FALSE; + BOOL spyed_block = FALSE, spy_active = FALSE; int did_alloc; TRACE("(%p)\n", mem); @@ -335,12 +335,13 @@ static INT WINAPI IMalloc_fnDidAlloc(IMalloc *iface, void *mem) { EnterCriticalSection(&IMalloc32_SpyCS); spyed_block = !!mallocspy_is_allocation_spyed(mem); + spy_active = TRUE; mem = IMallocSpy_PreDidAlloc(Malloc32.pSpy, mem, spyed_block); } did_alloc = HeapValidate(GetProcessHeap(), 0, mem); - if (Malloc32.pSpy) + if (spy_active) { did_alloc = IMallocSpy_PostDidAlloc(Malloc32.pSpy, mem, spyed_block, did_alloc); LeaveCriticalSection(&IMalloc32_SpyCS); -- 2.25.1
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/ole32/ifs.c | 35 +++++++++++++++++++++-------------- dlls/ole32/tests/compobj.c | 8 ++++++++ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/dlls/ole32/ifs.c b/dlls/ole32/ifs.c index 410691b2a8..1b19be424c 100644 --- a/dlls/ole32/ifs.c +++ b/dlls/ole32/ifs.c @@ -296,26 +296,33 @@ static void WINAPI IMalloc_fnFree(IMalloc *iface, void *pv) * win95: size allocated (4 byte boundaries) * win2k: size originally requested !!! (allocated on 8 byte boundaries) */ -static SIZE_T WINAPI IMalloc_fnGetSize(IMalloc *iface, void *pv) +static SIZE_T WINAPI IMalloc_fnGetSize(IMalloc *iface, void *mem) { - SIZE_T cb; - BOOL fSpyed = FALSE; + BOOL spyed_block = FALSE, spy_active = FALSE; + SIZE_T size; - TRACE("(%p)\n",pv); + TRACE("(%p)\n", mem); - if(Malloc32.pSpy) { - EnterCriticalSection(&IMalloc32_SpyCS); - pv = IMallocSpy_PreGetSize(Malloc32.pSpy, pv, fSpyed); - } + if (!mem) + return (SIZE_T)-1; + + if (Malloc32.pSpy) + { + EnterCriticalSection(&IMalloc32_SpyCS); + spyed_block = !!mallocspy_is_allocation_spyed(mem); + spy_active = TRUE; + mem = IMallocSpy_PreGetSize(Malloc32.pSpy, mem, spyed_block); + } - cb = HeapSize(GetProcessHeap(),0,pv); + size = HeapSize(GetProcessHeap(), 0, mem); - if(Malloc32.pSpy) { - cb = IMallocSpy_PostGetSize(Malloc32.pSpy, cb, fSpyed); - LeaveCriticalSection(&IMalloc32_SpyCS); - } + if (spy_active) + { + size = IMallocSpy_PostGetSize(Malloc32.pSpy, size, spyed_block); + LeaveCriticalSection(&IMalloc32_SpyCS); + } - return cb; + return size; } /****************************************************************************** diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c index b0a6ba1ea7..d6ccbc5882 100644 --- a/dlls/ole32/tests/compobj.c +++ b/dlls/ole32/tests/compobj.c @@ -3068,6 +3068,7 @@ static void test_CoWaitForMultipleHandles(void) static void test_CoGetMalloc(void) { IMalloc *imalloc; + SIZE_T size; HRESULT hr; char *ptr; int ret; @@ -3121,6 +3122,13 @@ static void test_CoGetMalloc(void) ret = IMalloc_DidAlloc(imalloc, ptr + 4); ok(ret == 0, "Unexpected return value %d.\n", ret); + /* GetSize() */ + size = IMalloc_GetSize(imalloc, NULL); + ok(size == (SIZE_T)-1, "Unexpected return value.\n"); + + size = IMalloc_GetSize(imalloc, ptr); + ok(size == 16, "Unexpected return value.\n"); + IMalloc_Free(imalloc, ptr); IMalloc_Release(imalloc); -- 2.25.1
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=69821 Your paranoid android. === w1064v1809_ja (32 bit report) === ole32: compobj.c:2670: Test failed: WaitForSingleObject failed compobj.c:2585: Test failed: QueryInterface failed: 80010108 19d8:compobj: unhandled exception c0000005 at 0040A321
On 4/16/20 10:28 PM, Marvin wrote:
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=69821
Your paranoid android.
=== w1064v1809_ja (32 bit report) ===
ole32: compobj.c:2670: Test failed: WaitForSingleObject failed compobj.c:2585: Test failed: QueryInterface failed: 80010108 19d8:compobj: unhandled exception c0000005 at 0040A321 I don't think it's related to this patch.
participants (2)
-
Marvin -
Nikolay Sivov