Rob Shearman : ole32: Fix some races in the global interface table implementation.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: eebf8df6015472de393c049c4daea6f7be06b905
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=eebf8df6015472de393c049c4d...

Author: Rob Shearman <rob(a)codeweavers.com>
Date:   Tue May 22 10:31:11 2007 +0100

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 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/dlls/ole32/git.c b/dlls/ole32/git.c
index 222eb6f..b51990d 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");