From eebf8df6015472de393c049c4daea6f7be06b905 Mon Sep 17 00:00:00 2001 From: Rob Shearman Date: Tue, 22 May 2007 10:31:11 +0100 Subject: [PATCH] ole32: Fix some races in the global interface table implementation. Fix a race between RevokeInterfaceFromGlobal and GetInterfaceFromGlobal by only using the entry inside the critical section. Fix a race between two GetInterfaceFromGlobal by cloning the stream, instead of using it and setting the current position back to zero. --- dlls/ole32/git.c | 55 ++++++++++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/dlls/ole32/git.c b/dlls/ole32/git.c index 222eb6f375f..b51990dc7fe 100644 --- a/dlls/ole32/git.c +++ b/dlls/ole32/git.c @@ -96,7 +96,7 @@ static void StdGlobalInterfaceTable_Destroy(void* self) /*** * A helper function to traverse the list and find the entry that matches the cookie. - * Returns NULL if not found + * Returns NULL if not found. Must be called inside git_section critical section. */ static StdGITEntry* StdGlobalInterfaceTable_FindEntry(IGlobalInterfaceTable* iface, DWORD cookie) @@ -106,14 +106,10 @@ StdGlobalInterfaceTable_FindEntry(IGlobalInterfaceTable* iface, DWORD cookie) TRACE("iface=%p, cookie=0x%x\n", iface, (UINT)cookie); - EnterCriticalSection(&git_section); LIST_FOR_EACH_ENTRY(e, &self->list, StdGITEntry, entry) { - if (e->cookie == cookie) { - LeaveCriticalSection(&git_section); + if (e->cookie == cookie) return e; - } } - LeaveCriticalSection(&git_section); TRACE("Entry not found\n"); return NULL; @@ -232,12 +228,19 @@ StdGlobalInterfaceTable_RevokeInterfaceFromGlobal( HRESULT hr; TRACE("iface=%p, dwCookie=0x%x\n", iface, (UINT)dwCookie); - + + EnterCriticalSection(&git_section); + entry = StdGlobalInterfaceTable_FindEntry(iface, dwCookie); if (entry == NULL) { TRACE("Entry not found\n"); + LeaveCriticalSection(&git_section); return E_INVALIDARG; /* not found */ } + + list_remove(&entry->entry); + + LeaveCriticalSection(&git_section); /* Free the stream */ hr = CoReleaseMarshalData(entry->stream); @@ -248,11 +251,6 @@ StdGlobalInterfaceTable_RevokeInterfaceFromGlobal( } IStream_Release(entry->stream); - /* chop entry out of the list, and free the memory */ - EnterCriticalSection(&git_section); - list_remove(&entry->entry); - LeaveCriticalSection(&git_section); - HeapFree(GetProcessHeap(), 0, entry); return S_OK; } @@ -264,27 +262,38 @@ StdGlobalInterfaceTable_GetInterfaceFromGlobal( { StdGITEntry* entry; HRESULT hres; - LARGE_INTEGER move; LPUNKNOWN lpUnk; - + IStream *stream; + TRACE("dwCookie=0x%x, riid=%s, ppv=%p\n", dwCookie, debugstr_guid(riid), ppv); - + + EnterCriticalSection(&git_section); + entry = StdGlobalInterfaceTable_FindEntry(iface, dwCookie); - if (entry == NULL) return E_INVALIDARG; + if (entry == NULL) { + LeaveCriticalSection(&git_section); + return E_INVALIDARG; + } if (!IsEqualIID(&entry->iid, riid)) { + LeaveCriticalSection(&git_section); WARN("entry->iid (%s) != riid\n", debugstr_guid(&entry->iid)); return E_INVALIDARG; } TRACE("entry=%p\n", entry); - + + hres = IStream_Clone(entry->stream, &stream); + + LeaveCriticalSection(&git_section); + + if (hres) { + WARN("Failed to clone stream with error 0x%08x\n", hres); + return hres; + } + /* unmarshal the interface */ - hres = CoUnmarshalInterface(entry->stream, riid, ppv); - - /* rewind stream, in case it's used again */ - move.u.LowPart = 0; - move.u.HighPart = 0; - IStream_Seek(entry->stream, move, STREAM_SEEK_SET, NULL); + hres = CoUnmarshalInterface(stream, riid, ppv); + IStream_Release(stream); if (hres) { WARN("Failed to unmarshal stream\n");