riched20: Avoid releasing a non-existent interface.
Nikolay Sivov
bunglehead at gmail.com
Wed Apr 16 22:54:58 CDT 2014
On 4/16/2014 17:37, Jactry Zeng wrote:
> Hi Nikolay,
>
> Thanks for your review!
>
> The crash can be reproduced follow this:
> - first release the ITextSelection or IOleClientSite
> interfaces completely;
> - release ITextDocument interface;
> - try to release the IRichEditOle (crash happen)
>
> And this patch try to fix it.
>
> + /* hres = ITextDocument_GetSelection(txtDoc, &txtSel); */
> + /* ok(hres == S_OK, "ITextDocument_GetSelection\n"); */
> + /* while(ITextSelection_Release(txtSel)); */
What you're doing is a violation of refcount handling. The rule is to
release what you got, without relying on internals like that. In this
case GetSelection() returns interface pointer and you're responsible in
exactly one Release() on it.
Interesting thing to test would be to check if GetSelection() returns
new instance every time it's called. If this is a case it will justify
some code changes to support this, right now patch is wrong.
If it actually returns same interface pointer you can't protect from
use-after-free because I can grab multiple references with several
GetSelection() calls, and when I'll try to release them it will be freed
already by a loop like that.
More information about the wine-devel
mailing list