[PATCH 1/4] mfplat: Fix use-after-free when releasing sample allocator.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/mfplat/sample.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/dlls/mfplat/sample.c b/dlls/mfplat/sample.c index 4ecce4fbe84..10d2b7bc2a8 100644 --- a/dlls/mfplat/sample.c +++ b/dlls/mfplat/sample.c @@ -1174,8 +1174,6 @@ static ULONG WINAPI sample_allocator_Release(IMFVideoSampleAllocatorEx *iface) IDirect3DDeviceManager9_Release(allocator->d3d9_device_manager); if (allocator->dxgi_device_manager) IMFDXGIDeviceManager_Release(allocator->dxgi_device_manager); - if (allocator->attributes) - IMFAttributes_Release(allocator->attributes); sample_allocator_set_media_type(allocator, NULL); sample_allocator_set_attributes(allocator, NULL); sample_allocator_release_samples(allocator); -- 2.30.0
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/mfplat/sample.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/dlls/mfplat/sample.c b/dlls/mfplat/sample.c index 10d2b7bc2a8..82e7c8680dd 100644 --- a/dlls/mfplat/sample.c +++ b/dlls/mfplat/sample.c @@ -184,18 +184,21 @@ static ULONG WINAPI sample_tracked_Release(IMFSample *iface) HRESULT hr; EnterCriticalSection(&sample->attributes.cs); - refcount = InterlockedDecrement(&sample->attributes.ref); - if (sample->tracked_result && sample->tracked_refcount == refcount) + if (sample->tracked_result && sample->tracked_refcount == (sample->attributes.ref - 1)) { - /* Call could fail if queue system is not initialized, it's not critical. */ - if (FAILED(hr = RtwqInvokeCallback(sample->tracked_result))) - WARN("Failed to invoke tracking callback, hr %#x.\n", hr); - IRtwqAsyncResult_Release(sample->tracked_result); + IRtwqAsyncResult *tracked_result = sample->tracked_result; sample->tracked_result = NULL; sample->tracked_refcount = 0; + + /* Call could fail if queue system is not initialized, it's not critical. */ + if (FAILED(hr = RtwqInvokeCallback(tracked_result))) + WARN("Failed to invoke tracking callback, hr %#x.\n", hr); + IRtwqAsyncResult_Release(tracked_result); } LeaveCriticalSection(&sample->attributes.cs); + refcount = InterlockedDecrement(&sample->attributes.ref); + TRACE("%p, refcount %u.\n", iface, refcount); if (!refcount) -- 2.30.0
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/mfplat/sample.c | 33 ++++++++++++--- dlls/mfplat/tests/mfplat.c | 82 +++++++++++++++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/dlls/mfplat/sample.c b/dlls/mfplat/sample.c index 82e7c8680dd..cdd3d897169 100644 --- a/dlls/mfplat/sample.c +++ b/dlls/mfplat/sample.c @@ -72,6 +72,8 @@ struct sample_allocator unsigned int height; D3DFORMAT d3d9_format; DXGI_FORMAT dxgi_format; + unsigned int usage; + unsigned int bindflags; unsigned int buffer_count; } frame_desc; @@ -1346,8 +1348,12 @@ static HRESULT sample_allocator_allocate_sample(struct sample_allocator *allocat desc.Format = allocator->frame_desc.dxgi_format; desc.SampleDesc.Count = 1; desc.SampleDesc.Quality = 0; - desc.Usage = 0; - desc.BindFlags = D3D11_BIND_SHADER_RESOURCE | D3D11_BIND_RENDER_TARGET; + desc.Usage = allocator->frame_desc.usage; + desc.BindFlags = allocator->frame_desc.bindflags; + if (desc.Usage == D3D11_USAGE_DYNAMIC) + desc.CPUAccessFlags = D3D11_CPU_ACCESS_WRITE; + else if (desc.Usage == D3D11_USAGE_STAGING) + desc.CPUAccessFlags = D3D11_CPU_ACCESS_WRITE | D3D11_CPU_ACCESS_READ; if (SUCCEEDED(hr = ID3D11Device_CreateTexture2D(service->d3d11_device, &desc, NULL, &texture))) { @@ -1396,15 +1402,32 @@ static HRESULT sample_allocator_initialize(struct sample_allocator *allocator, u if (sample_count > max_sample_count) return E_INVALIDARG; + allocator->frame_desc.usage = D3D11_USAGE_DEFAULT; + if (attributes) + { + IMFAttributes_GetUINT32(attributes, &MF_SA_BUFFERS_PER_SAMPLE, &allocator->frame_desc.buffer_count); + IMFAttributes_GetUINT32(attributes, &MF_SA_D3D11_USAGE, &allocator->frame_desc.usage); + } + + if (allocator->frame_desc.usage == D3D11_USAGE_IMMUTABLE || allocator->frame_desc.usage > D3D11_USAGE_STAGING) + return E_INVALIDARG; + + if (allocator->frame_desc.usage == D3D11_USAGE_DEFAULT) + allocator->frame_desc.bindflags = D3D11_BIND_SHADER_RESOURCE | D3D11_BIND_RENDER_TARGET; + else if (allocator->frame_desc.usage == D3D11_USAGE_DYNAMIC) + allocator->frame_desc.bindflags = D3D11_BIND_SHADER_RESOURCE; + else + allocator->frame_desc.bindflags = 0; + + if (attributes) + IMFAttributes_GetUINT32(attributes, &MF_SA_D3D11_BINDFLAGS, &allocator->frame_desc.bindflags); + sample_allocator_set_media_type(allocator, media_type); sample_allocator_set_attributes(allocator, attributes); sample_count = max(1, sample_count); max_sample_count = max(1, max_sample_count); - if (attributes) - IMFAttributes_GetUINT32(attributes, &MF_SA_BUFFERS_PER_SAMPLE, &allocator->frame_desc.buffer_count); - allocator->frame_desc.d3d9_format = subtype.Data1; allocator->frame_desc.dxgi_format = MFMapDX9FormatToDXGIFormat(allocator->frame_desc.d3d9_format); allocator->frame_desc.width = frame_size >> 32; diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 9e3a3da08ea..fdfcf8a5d4b 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -6295,7 +6295,7 @@ static void test_sample_allocator(void) IMFVideoSampleAllocatorEx *allocatorex; IDirect3DDeviceManager9 *d3d9_manager; IMFVideoSampleAllocator *allocator; - unsigned int buffer_count, token; + unsigned int i, buffer_count, token; IDirect3DDevice9 *d3d9_device; IMFDXGIDeviceManager *manager; IMFSample *sample, *sample2; @@ -6311,6 +6311,14 @@ static void test_sample_allocator(void) HRESULT hr; BYTE *data; HWND window; + static const unsigned int usage[] = + { + D3D11_USAGE_DEFAULT, + D3D11_USAGE_IMMUTABLE, + D3D11_USAGE_DYNAMIC, + D3D11_USAGE_STAGING, + D3D11_USAGE_STAGING + 1, + }; if (!pMFCreateVideoSampleAllocatorEx) { @@ -6600,6 +6608,78 @@ todo_wine IMFSample_Release(sample); IMFVideoSampleAllocator_Release(allocator); + + /* MF_SA_D3D11_USAGE */ + hr = MFCreateAttributes(&attributes, 1); + ok(hr == S_OK, "Failed to create attributes, hr %#x.\n", hr); + + for (i = 0; i < ARRAY_SIZE(usage); ++i) + { + hr = pMFCreateVideoSampleAllocatorEx(&IID_IMFVideoSampleAllocatorEx, (void **)&allocatorex); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + + hr = IMFVideoSampleAllocatorEx_SetDirectXManager(allocatorex, (IUnknown *)manager); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + + hr = IMFAttributes_SetUINT32(attributes, &MF_SA_D3D11_USAGE, usage[i]); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + + hr = IMFVideoSampleAllocatorEx_InitializeSampleAllocatorEx(allocatorex, 0, 0, attributes, video_type); + if (usage[i] == D3D11_USAGE_IMMUTABLE || usage[i] > D3D11_USAGE_STAGING) + { + ok(hr == E_INVALIDARG, "Unexpected hr %#x.\n", hr); + IMFVideoSampleAllocatorEx_Release(allocatorex); + continue; + } + ok(hr == S_OK, "%u: Unexpected hr %#x.\n", usage[i], hr); + + hr = IMFAttributes_SetUINT32(attributes, &MF_SA_D3D11_USAGE, D3D11_USAGE_DEFAULT); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + + hr = IMFVideoSampleAllocatorEx_AllocateSample(allocatorex, &sample); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + + hr = IMFSample_GetBufferByIndex(sample, 0, &buffer); + ok(hr == S_OK, "Unexpected hr %#x.\n", hr); + + hr = IMFMediaBuffer_QueryInterface(buffer, &IID_IMFDXGIBuffer, (void **)&dxgi_buffer); + ok(hr == S_OK, "Failed to get interface, hr %#x.\n", hr); + + hr = IMFDXGIBuffer_GetResource(dxgi_buffer, &IID_ID3D11Texture2D, (void **)&texture); + ok(hr == S_OK, "Failed to get resource, hr %#x.\n", hr); + + ID3D11Texture2D_GetDesc(texture, &desc); + ok(desc.Usage == usage[i], "Unexpected usage %u.\n", desc.Usage); + if (usage[i] == D3D11_USAGE_DEFAULT) + { + ok(desc.BindFlags == (D3D11_BIND_SHADER_RESOURCE | D3D11_BIND_RENDER_TARGET), "Unexpected bind flags %#x.\n", + desc.BindFlags); + ok(desc.CPUAccessFlags == 0, "Unexpected CPU access flags %#x.\n", desc.CPUAccessFlags); + } + else if (usage[i] == D3D11_USAGE_DYNAMIC) + { + ok(desc.BindFlags == D3D11_BIND_SHADER_RESOURCE, "Unexpected bind flags %#x.\n", desc.BindFlags); + ok(desc.CPUAccessFlags == D3D11_CPU_ACCESS_WRITE, "Unexpected CPU access flags %#x.\n", desc.CPUAccessFlags); + } + else if (usage[i] == D3D11_USAGE_STAGING) + { + ok(desc.BindFlags == 0, "Unexpected bind flags %#x.\n", desc.BindFlags); + ok(desc.CPUAccessFlags == (D3D11_CPU_ACCESS_WRITE | D3D11_CPU_ACCESS_READ), "Unexpected CPU access flags %#x.\n", + desc.CPUAccessFlags); + } + ok(desc.MiscFlags == 0, "Unexpected misc flags %#x.\n", desc.MiscFlags); + + ID3D11Texture2D_Release(texture); + IMFDXGIBuffer_Release(dxgi_buffer); + IMFMediaBuffer_Release(buffer); + + IMFSample_Release(sample); + + IMFVideoSampleAllocatorEx_Release(allocatorex); + } + + IMFAttributes_Release(attributes); + IMFDXGIDeviceManager_Release(manager); ID3D11Device_Release(device); -- 2.30.0
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=85604 Your paranoid android. === w7u_el (32 bit report) === mfplat: 0a58:mfplat: unhandled exception c0000005 at 6D467D1C
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/mfplat/main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c index b78cfc33e60..99afbaf8a46 100644 --- a/dlls/mfplat/main.c +++ b/dlls/mfplat/main.c @@ -8599,7 +8599,7 @@ static HRESULT dxgi_device_manager_get_handle_index(struct dxgi_device_manager * static HRESULT WINAPI dxgi_device_manager_QueryInterface(IMFDXGIDeviceManager *iface, REFIID riid, void **obj) { - TRACE("(%p, %s, %p).\n", iface, debugstr_guid(riid), obj); + TRACE("%p, %s, %p.\n", iface, debugstr_guid(riid), obj); if (IsEqualIID(riid, &IID_IMFDXGIDeviceManager) || IsEqualGUID(riid, &IID_IUnknown)) @@ -8619,7 +8619,7 @@ static ULONG WINAPI dxgi_device_manager_AddRef(IMFDXGIDeviceManager *iface) struct dxgi_device_manager *manager = impl_from_IMFDXGIDeviceManager(iface); ULONG refcount = InterlockedIncrement(&manager->refcount); - TRACE("(%p) ref=%u.\n", iface, refcount); + TRACE("%p, refcount %u.\n", iface, refcount); return refcount; } @@ -8629,7 +8629,7 @@ static ULONG WINAPI dxgi_device_manager_Release(IMFDXGIDeviceManager *iface) struct dxgi_device_manager *manager = impl_from_IMFDXGIDeviceManager(iface); ULONG refcount = InterlockedDecrement(&manager->refcount); - TRACE("(%p) ref=%u.\n", iface, refcount); + TRACE("%p, refcount %u.\n", iface, refcount); if (!refcount) { -- 2.30.0
participants (2)
-
Marvin -
Nikolay Sivov