From: Jinoh Kang jinoh.kang.kr@gmail.com
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). --- dlls/combase/marshal.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/dlls/combase/marshal.c b/dlls/combase/marshal.c index 360c2c3e987..607ac93a569 100644 --- a/dlls/combase/marshal.c +++ b/dlls/combase/marshal.c @@ -75,7 +75,7 @@ struct proxy_manager OXID_INFO oxid_info; /* string binding, ipid of rem unknown and other information (RO) */ OID oid; /* object ID (RO) */ struct list interfaces; /* imported interfaces (CS cs) */ - LONG refs; /* proxy reference count (LOCK) */ + LONG refs; /* proxy reference count (LOCK); 0 if about to be removed from list */ CRITICAL_SECTION cs; /* thread safety for this object and children */ ULONG sorflags; /* STDOBJREF flags (RO) */ IRemUnknown *remunk; /* proxy to IRemUnknown used for lifecycle management (CS cs) */ @@ -958,6 +958,7 @@ HRESULT marshal_object(struct apartment *apt, STDOBJREF *stdobjref, REFIID riid, /* Client-side identity of the server object */
static HRESULT proxy_manager_get_remunknown(struct proxy_manager * This, IRemUnknown **remunk); +static LONG proxy_manager_addref_if_alive(struct proxy_manager * This); static void proxy_manager_destroy(struct proxy_manager * This); static HRESULT proxy_manager_find_ifproxy(struct proxy_manager * This, REFIID riid, struct ifproxy ** ifproxy_found); static HRESULT proxy_manager_query_local_interface(struct proxy_manager * This, REFIID riid, void ** ppv); @@ -1887,6 +1888,32 @@ static HRESULT proxy_manager_get_remunknown(struct proxy_manager * This, IRemUnk return hr; }
+/* + * Safely increment the reference count of a proxy manager obtained from an + * apartment proxy list. + * + * This function shall be called inside the apartment's critical section. + */ +static LONG proxy_manager_addref_if_alive(struct proxy_manager * This) +{ + LONG refs = ReadNoFence(&This->refs); + LONG old_refs, new_refs; + + do + { + if (refs == 0) + { + /* This proxy manager is about to be destroyed */ + return 0; + } + + old_refs = refs; + new_refs = refs + 1; + } while ((refs = InterlockedCompareExchange(&This->refs, new_refs, old_refs)) != old_refs); + + return new_refs; +} + /* destroys a proxy manager, freeing the memory it used. * Note: this function should not be called from a list iteration in the * apartment, due to the fact that it removes itself from the apartment and @@ -1949,7 +1976,7 @@ static BOOL find_proxy_manager(struct apartment * apt, OXID oxid, OID oid, struc /* be careful of a race with ClientIdentity_Release, which would * cause us to return a proxy which is in the process of being * destroyed */ - if (IMultiQI_AddRef(&proxy->IMultiQI_iface) != 1) + if (proxy_manager_addref_if_alive(proxy) != 0) { *proxy_found = proxy; found = TRUE;