From: Thomas Faber Date: Thu, 26 Nov 2015 22:04:34 +0000 (+0000) Subject: [QUARTZ] X-Git-Tag: ReactOS-0.4.0~59^2~203 X-Git-Url: https://git.reactos.org/?p=reactos.git;a=commitdiff_plain;h=1708c275b3efbdbcceddceb3ebf3fa55fbe4e32c [QUARTZ] - Don't cache IFilterMapper2 interface in FilterGraph2. Fixes use after free when playing videos with MPC HC. - Fix a reference leak CORE-7671 svn path=/trunk/; revision=70118 --- diff --git a/reactos/dll/directx/wine/quartz/filtergraph.c b/reactos/dll/directx/wine/quartz/filtergraph.c index 9b1707de634..9121a215e02 100644 --- a/reactos/dll/directx/wine/quartz/filtergraph.c +++ b/reactos/dll/directx/wine/quartz/filtergraph.c @@ -155,7 +155,6 @@ typedef struct _IFilterGraphImpl { IUnknown *outer_unk; LONG ref; IUnknown *punkFilterMapper2; - IFilterMapper2 * pFilterMapper2; IBaseFilter ** ppFiltersInGraph; LPWSTR * pFilterNames; ULONG nFilters; @@ -243,11 +242,11 @@ static HRESULT WINAPI FilterGraphInner_QueryInterface(IUnknown *iface, REFIID ri TRACE(" requesting IFilterMapper interface from aggregated filtermapper (%p)\n", *ppvObj); return IUnknown_QueryInterface(This->punkFilterMapper2, riid, ppvObj); } else if (IsEqualGUID(&IID_IFilterMapper2, riid)) { - *ppvObj = This->pFilterMapper2; TRACE(" returning IFilterMapper2 interface from aggregated filtermapper (%p)\n", *ppvObj); + return IUnknown_QueryInterface(This->punkFilterMapper2, riid, ppvObj); } else if (IsEqualGUID(&IID_IFilterMapper3, riid)) { - *ppvObj = This->pFilterMapper2; TRACE(" returning IFilterMapper3 interface from aggregated filtermapper (%p)\n", *ppvObj); + return IUnknown_QueryInterface(This->punkFilterMapper2, riid, ppvObj); } else if (IsEqualGUID(&IID_IGraphVersion, riid)) { *ppvObj = &This->IGraphConfig_iface; TRACE(" returning IGraphConfig interface (%p)\n", *ppvObj); @@ -297,9 +296,6 @@ static ULONG WINAPI FilterGraphInner_Release(IUnknown *iface) IUnknown_Release(This->ItfCacheEntries[i].iface); } - /* AddRef on controlling IUnknown, to compensate for Release of cached IFilterMapper2 */ - IUnknown_AddRef(This->outer_unk); - IFilterMapper2_Release(This->pFilterMapper2); IUnknown_Release(This->punkFilterMapper2); if (This->pSite) IUnknown_Release(This->pSite); @@ -888,6 +884,7 @@ static HRESULT WINAPI FilterGraph2_Connect(IFilterGraph2 *iface, IPin *ppinOut, CLSID FilterCLSID; PIN_DIRECTION dir; unsigned int i = 0; + IFilterMapper2 *pFilterMapper2 = NULL; TRACE("(%p/%p)->(%p, %p)\n", This, iface, ppinOut, ppinIn); @@ -977,10 +974,16 @@ static HRESULT WINAPI FilterGraph2_Connect(IFilterGraph2 *iface, IPin *ppinOut, TRACE("MajorType %s\n", debugstr_guid(&mt->majortype)); TRACE("SubType %s\n", debugstr_guid(&mt->subtype)); + hr = IUnknown_QueryInterface(This->punkFilterMapper2, &IID_IFilterMapper2, (void**)&pFilterMapper2); + if (FAILED(hr)) { + WARN("Unable to get IFilterMapper2 (%x)\n", hr); + goto out; + } + /* Try to find a suitable filter that can connect to the pin to render */ tab[0] = mt->majortype; tab[1] = mt->subtype; - hr = IFilterMapper2_EnumMatchingFilters(This->pFilterMapper2, &pEnumMoniker, 0, FALSE, MERIT_UNLIKELY, TRUE, 1, tab, NULL, NULL, FALSE, FALSE, 0, NULL, NULL, NULL); + hr = IFilterMapper2_EnumMatchingFilters(pFilterMapper2, &pEnumMoniker, 0, FALSE, MERIT_UNLIKELY, TRUE, 1, tab, NULL, NULL, FALSE, FALSE, 0, NULL, NULL, NULL); if (FAILED(hr)) { WARN("Unable to enum filters (%x)\n", hr); goto out; @@ -1147,7 +1150,11 @@ error: CoTaskMemFree(ppins); } + IEnumMoniker_Release(pEnumMoniker); + out: + if (pFilterMapper2) + IFilterMapper2_Release(pFilterMapper2); if (penummt) IEnumMediaTypes_Release(penummt); if (mt) @@ -1243,6 +1250,7 @@ static HRESULT WINAPI FilterGraph2_Render(IFilterGraph2 *iface, IPin *ppinOut) ULONG nb; IMoniker* pMoniker; INT x; + IFilterMapper2 *pFilterMapper2 = NULL; TRACE("(%p/%p)->(%p)\n", This, iface, ppinOut); @@ -1352,10 +1360,20 @@ static HRESULT WINAPI FilterGraph2_Render(IFilterGraph2 *iface, IPin *ppinOut) continue; } + if (pFilterMapper2 == NULL) + { + hr = IUnknown_QueryInterface(This->punkFilterMapper2, &IID_IFilterMapper2, (void**)&pFilterMapper2); + if (FAILED(hr)) + { + WARN("Unable to query IFilterMapper2 (%x)\n", hr); + break; + } + } + /* Try to find a suitable renderer with the same media type */ tab[0] = mt->majortype; tab[1] = mt->subtype; - hr = IFilterMapper2_EnumMatchingFilters(This->pFilterMapper2, &pEnumMoniker, 0, FALSE, MERIT_UNLIKELY, TRUE, 1, tab, NULL, NULL, FALSE, FALSE, 0, NULL, NULL, NULL); + hr = IFilterMapper2_EnumMatchingFilters(pFilterMapper2, &pEnumMoniker, 0, FALSE, MERIT_UNLIKELY, TRUE, 1, tab, NULL, NULL, FALSE, FALSE, 0, NULL, NULL, NULL); if (FAILED(hr)) { WARN("Unable to enum filters (%x)\n", hr); @@ -1469,6 +1487,9 @@ error: hr = S_OK; } + if (pFilterMapper2) + IFilterMapper2_Release(pFilterMapper2); + IEnumMediaTypes_Release(penummt); return hr; } @@ -5684,14 +5705,6 @@ HRESULT FilterGraph_create(IUnknown *pUnkOuter, LPVOID *ppObj) hr = CoCreateInstance(&CLSID_FilterMapper2, fimpl->outer_unk, CLSCTX_INPROC_SERVER, &IID_IUnknown, (void**)&fimpl->punkFilterMapper2); - if (SUCCEEDED(hr)) - hr = IUnknown_QueryInterface(fimpl->punkFilterMapper2, &IID_IFilterMapper2, - (void**)&fimpl->pFilterMapper2); - - if (SUCCEEDED(hr)) - /* Release controlling IUnknown - compensate refcount increase from caching IFilterMapper2 interface. */ - IUnknown_Release(fimpl->outer_unk); - if (FAILED(hr)) { ERR("Unable to create filter mapper (%x)\n", hr); if (fimpl->punkFilterMapper2) IUnknown_Release(fimpl->punkFilterMapper2); diff --git a/rostests/winetests/quartz/filtergraph.c b/rostests/winetests/quartz/filtergraph.c index b42aea8d35d..39d6e88a321 100644 --- a/rostests/winetests/quartz/filtergraph.c +++ b/rostests/winetests/quartz/filtergraph.c @@ -1886,6 +1886,98 @@ static void test_render_filter_priority(void) ok(hr == S_OK, "CoRevokeClassObject failed with %08x\n", hr); } +typedef struct IUnknownImpl +{ + IUnknown IUnknown_iface; + int AddRef_called; + int Release_called; +} IUnknownImpl; + +static IUnknownImpl *IUnknownImpl_from_iface(IUnknown * iface) +{ + return CONTAINING_RECORD(iface, IUnknownImpl, IUnknown_iface); +} + +static HRESULT WINAPI IUnknownImpl_QueryInterface(IUnknown * iface, REFIID riid, LPVOID * ppv) +{ + ok(0, "QueryInterface should not be called for %s\n", wine_dbgstr_guid(riid)); + return E_NOINTERFACE; +} + +static ULONG WINAPI IUnknownImpl_AddRef(IUnknown * iface) +{ + IUnknownImpl *This = IUnknownImpl_from_iface(iface); + This->AddRef_called++; + return 2; +} + +static ULONG WINAPI IUnknownImpl_Release(IUnknown * iface) +{ + IUnknownImpl *This = IUnknownImpl_from_iface(iface); + This->Release_called++; + return 1; +} + +static CONST_VTBL IUnknownVtbl IUnknownImpl_Vtbl = +{ + IUnknownImpl_QueryInterface, + IUnknownImpl_AddRef, + IUnknownImpl_Release +}; + +static void test_aggregate_filter_graph(void) +{ + HRESULT hr; + IUnknown *pgraph; + IUnknown *punk; + IUnknownImpl unk_outer = { { &IUnknownImpl_Vtbl }, 0, 0 }; + + hr = CoCreateInstance(&CLSID_FilterGraph, &unk_outer.IUnknown_iface, CLSCTX_INPROC_SERVER, + &IID_IUnknown, (void **)&pgraph); + ok(hr == S_OK, "CoCreateInstance returned %x\n", hr); + ok(pgraph != &unk_outer.IUnknown_iface, "pgraph = %p, expected not %p\n", pgraph, &unk_outer.IUnknown_iface); + + hr = IUnknown_QueryInterface(pgraph, &IID_IUnknown, (void **)&punk); + ok(hr == S_OK, "CoCreateInstance returned %x\n", hr); + ok(punk != &unk_outer.IUnknown_iface, "punk = %p, expected not %p\n", punk, &unk_outer.IUnknown_iface); + IUnknown_Release(punk); + + ok(unk_outer.AddRef_called == 0, "IUnknownImpl_AddRef called %d times\n", unk_outer.AddRef_called); + ok(unk_outer.Release_called == 0, "IUnknownImpl_Release called %d times\n", unk_outer.Release_called); + unk_outer.AddRef_called = 0; + unk_outer.Release_called = 0; + + hr = IUnknown_QueryInterface(pgraph, &IID_IFilterMapper, (void **)&punk); + ok(hr == S_OK, "CoCreateInstance returned %x\n", hr); + ok(punk != &unk_outer.IUnknown_iface, "punk = %p, expected not %p\n", punk, &unk_outer.IUnknown_iface); + IUnknown_Release(punk); + + ok(unk_outer.AddRef_called == 1, "IUnknownImpl_AddRef called %d times\n", unk_outer.AddRef_called); + ok(unk_outer.Release_called == 1, "IUnknownImpl_Release called %d times\n", unk_outer.Release_called); + unk_outer.AddRef_called = 0; + unk_outer.Release_called = 0; + + hr = IUnknown_QueryInterface(pgraph, &IID_IFilterMapper2, (void **)&punk); + ok(hr == S_OK, "CoCreateInstance returned %x\n", hr); + ok(punk != &unk_outer.IUnknown_iface, "punk = %p, expected not %p\n", punk, &unk_outer.IUnknown_iface); + IUnknown_Release(punk); + + ok(unk_outer.AddRef_called == 1, "IUnknownImpl_AddRef called %d times\n", unk_outer.AddRef_called); + ok(unk_outer.Release_called == 1, "IUnknownImpl_Release called %d times\n", unk_outer.Release_called); + unk_outer.AddRef_called = 0; + unk_outer.Release_called = 0; + + hr = IUnknown_QueryInterface(pgraph, &IID_IFilterMapper3, (void **)&punk); + ok(hr == S_OK, "CoCreateInstance returned %x\n", hr); + ok(punk != &unk_outer.IUnknown_iface, "punk = %p, expected not %p\n", punk, &unk_outer.IUnknown_iface); + IUnknown_Release(punk); + + ok(unk_outer.AddRef_called == 1, "IUnknownImpl_AddRef called %d times\n", unk_outer.AddRef_called); + ok(unk_outer.Release_called == 1, "IUnknownImpl_Release called %d times\n", unk_outer.Release_called); + + IUnknown_Release(pgraph); +} + START_TEST(filtergraph) { HRESULT hr; @@ -1904,5 +1996,6 @@ START_TEST(filtergraph) test_mediacontrol(); test_filter_graph2(); test_render_filter_priority(); + test_aggregate_filter_graph(); CoUninitialize(); } diff --git a/rostests/winetests/quartz/filtermapper.c b/rostests/winetests/quartz/filtermapper.c index 52298fb3623..63b31bb54d5 100644 --- a/rostests/winetests/quartz/filtermapper.c +++ b/rostests/winetests/quartz/filtermapper.c @@ -541,6 +541,98 @@ out: IFilterMapper2_Release(pMapper); } +typedef struct IUnknownImpl +{ + IUnknown IUnknown_iface; + int AddRef_called; + int Release_called; +} IUnknownImpl; + +static IUnknownImpl *IUnknownImpl_from_iface(IUnknown * iface) +{ + return CONTAINING_RECORD(iface, IUnknownImpl, IUnknown_iface); +} + +static HRESULT WINAPI IUnknownImpl_QueryInterface(IUnknown * iface, REFIID riid, LPVOID * ppv) +{ + ok(0, "QueryInterface should not be called for %s\n", wine_dbgstr_guid(riid)); + return E_NOINTERFACE; +} + +static ULONG WINAPI IUnknownImpl_AddRef(IUnknown * iface) +{ + IUnknownImpl *This = IUnknownImpl_from_iface(iface); + This->AddRef_called++; + return 2; +} + +static ULONG WINAPI IUnknownImpl_Release(IUnknown * iface) +{ + IUnknownImpl *This = IUnknownImpl_from_iface(iface); + This->Release_called++; + return 1; +} + +static CONST_VTBL IUnknownVtbl IUnknownImpl_Vtbl = +{ + IUnknownImpl_QueryInterface, + IUnknownImpl_AddRef, + IUnknownImpl_Release +}; + +static void test_aggregate_filter_mapper(void) +{ + HRESULT hr; + IUnknown *pmapper; + IUnknown *punk; + IUnknownImpl unk_outer = { { &IUnknownImpl_Vtbl }, 0, 0 }; + + hr = CoCreateInstance(&CLSID_FilterMapper2, &unk_outer.IUnknown_iface, CLSCTX_INPROC_SERVER, + &IID_IUnknown, (void **)&pmapper); + ok(hr == S_OK, "CoCreateInstance returned %x\n", hr); + ok(pmapper != &unk_outer.IUnknown_iface, "pmapper = %p, expected not %p\n", pmapper, &unk_outer.IUnknown_iface); + + hr = IUnknown_QueryInterface(pmapper, &IID_IUnknown, (void **)&punk); + ok(hr == S_OK, "CoCreateInstance returned %x\n", hr); + ok(punk != &unk_outer.IUnknown_iface, "punk = %p, expected not %p\n", punk, &unk_outer.IUnknown_iface); + IUnknown_Release(punk); + + ok(unk_outer.AddRef_called == 0, "IUnknownImpl_AddRef called %d times\n", unk_outer.AddRef_called); + ok(unk_outer.Release_called == 0, "IUnknownImpl_Release called %d times\n", unk_outer.Release_called); + unk_outer.AddRef_called = 0; + unk_outer.Release_called = 0; + + hr = IUnknown_QueryInterface(pmapper, &IID_IFilterMapper, (void **)&punk); + ok(hr == S_OK, "CoCreateInstance returned %x\n", hr); + ok(punk != &unk_outer.IUnknown_iface, "punk = %p, expected not %p\n", punk, &unk_outer.IUnknown_iface); + IUnknown_Release(punk); + + ok(unk_outer.AddRef_called == 1, "IUnknownImpl_AddRef called %d times\n", unk_outer.AddRef_called); + ok(unk_outer.Release_called == 1, "IUnknownImpl_Release called %d times\n", unk_outer.Release_called); + unk_outer.AddRef_called = 0; + unk_outer.Release_called = 0; + + hr = IUnknown_QueryInterface(pmapper, &IID_IFilterMapper2, (void **)&punk); + ok(hr == S_OK, "CoCreateInstance returned %x\n", hr); + ok(punk != &unk_outer.IUnknown_iface, "punk = %p, expected not %p\n", punk, &unk_outer.IUnknown_iface); + IUnknown_Release(punk); + + ok(unk_outer.AddRef_called == 1, "IUnknownImpl_AddRef called %d times\n", unk_outer.AddRef_called); + ok(unk_outer.Release_called == 1, "IUnknownImpl_Release called %d times\n", unk_outer.Release_called); + unk_outer.AddRef_called = 0; + unk_outer.Release_called = 0; + + hr = IUnknown_QueryInterface(pmapper, &IID_IFilterMapper3, (void **)&punk); + ok(hr == S_OK, "CoCreateInstance returned %x\n", hr); + ok(punk != &unk_outer.IUnknown_iface, "punk = %p, expected not %p\n", punk, &unk_outer.IUnknown_iface); + IUnknown_Release(punk); + + ok(unk_outer.AddRef_called == 1, "IUnknownImpl_AddRef called %d times\n", unk_outer.AddRef_called); + ok(unk_outer.Release_called == 1, "IUnknownImpl_Release called %d times\n", unk_outer.Release_called); + + IUnknown_Release(pmapper); +} + START_TEST(filtermapper) { CoInitialize(NULL); @@ -550,6 +642,7 @@ START_TEST(filtermapper) test_ifiltermapper_from_filtergraph(); test_register_filter_with_null_clsMinorType(); test_parse_filter_data(); + test_aggregate_filter_mapper(); CoUninitialize(); }