[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: [PATCH 1/1] qedit: Correct MediaDet_Release() crash.
From:       Zebediah Figura <z.figura12 () gmail ! com>
Date:       2019-10-31 23:49:06
Message-ID: 02163631-ec35-6a0d-3a56-94e96d9ad8cf () gmail ! com
[Download RAW message or body]

On 10/30/19 12:58 AM, Roberto Pungartnik wrote:
> qedit: Correct MediaDet_Release() crash.
> 
> MediaDet_Release() crash after calling MediaDet_put_Filename() with an unsupported \
> media type. 
> The crash is at MediaDet_Release(), but the problem is at MediaDet_put_Filename(), \
> in case of a failure in GetSplitter(). 
> Signed-off-by: Roberto Pungartnik <rpungartnik@gmail.com>
> ---
> dlls/qedit/mediadet.c | 4 ++++
> 1 file changed, 4 insertions(+)
> 
> diff --git a/dlls/qedit/mediadet.c b/dlls/qedit/mediadet.c
> index b7a5abb80c..a602166462 100644
> --- a/dlls/qedit/mediadet.c
> +++ b/dlls/qedit/mediadet.c
> @@ -509,7 +509,11 @@ static HRESULT WINAPI MediaDet_put_Filename(IMediaDet* iface, \
> BSTR newVal) This->source = bf;
> hr = GetSplitter(This);
> if (FAILED(hr))
> +    {
> +        This->graph = NULL;
> +        This->source = NULL;
> return hr;
> +    }
> 
> return MediaDet_put_CurrentStream(iface, 0);
> }

Hello Roberto, thanks for the patch.

I don't think this looks quite right as-is, however. I do think it's
probably better to reset these variables to NULL if GetSplitter() fails,
but then we also need to release them here, or the objects will be leaked.

I'm also not sure how this could cause a crash in MediaDet_Release(). As
far as I can tell our reference counting logic is valid (if a little
confusing). Can you elaborate on what causes the crash?

ἔρρωσο,
Zeb

> -- 
> 2.17.1
> 


[prev in list] [next in list] [prev in thread] [next in thread]