REGRESSION(r207182): [iOS] Crash because of passing freed CFDictionaryRef to CG when decoding a sub-sampled large image

[Said Abou-Hallawa]

On iOS image sub-sampling is enabled by default, WebCore::imageSourceOptions() creates a new dictionary for the image options every time it is called.

Because ImageDecoder::createFrameImageAtIndex() takes the raw pointer of the output of imageSourceOptions(), CFRelease() is called for the returned retain pointer. This happens immediately after assigning the raw pointer to the local variable.  This causes the CFDictionaryRef to be freed before passing it to CGImageSourceCreateImageAtIndex().

    CFDictionaryRef options = imageSourceOptions(subsamplingLevel, decodingMode).get();
    RetainPtr<CGImageRef> image = adoptCF(CGImageSourceCreateImageAtIndex(m_nativeDecoder.get(), index, options));

The original code was not getting the raw pointer into a local variable. Instead it was passing it directly to  CGImageSourceCreateImageAtIndex(). This prevents calling CFRelease() before CGImageSourceCreateImageAtIndex returns.

    RetainPtr<CGImageRef> image = adoptCF(CGImageSourceCreateImageAtIndex(m_nativeDecoder.get(), index, imageSourceOptions(subsamplingLevel).get()));

[Said Abou-Hallawa]

<rdar://problem/28907947>

[Said Abou-Hallawa]

Created attachment 294443 [details]
Patch

[Tim Horton]

Comment on attachment 294443 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=294443&action=review

> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:65
> +    CFDictionaryAddValue(options.get(), kCGImageSourceShouldCache, kCFBooleanTrue);

Why swap to Add? Set is a tiny bit cheaper (I think?) and we know that each key will only be used once.

[Said Abou-Hallawa]

Created attachment 294444 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 294444 [details]
Patch

Clearing flags on attachment: 294444

Committed r208575: <http://trac.webkit.org/changeset/208575>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.