Bug 144809 - Fix possible race condition in BitmapImage::cacheFrame()
Summary: Fix possible race condition in BitmapImage::cacheFrame()
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-08 13:10 PDT by Said Abou-Hallawa
Modified: 2015-05-14 09:02 PDT (History)
0 users

See Also:


Attachments
Patch (4.11 KB, patch)
2015-05-08 13:13 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (4.70 KB, patch)
2015-05-08 13:24 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (4.69 KB, patch)
2015-05-08 14:55 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2015-05-08 13:10:04 PDT
In BitmapImage::cacheFrame(), we should check the frame metadata loading status before, asking the image source to provide information about its metadata. A crash was reported on iOS with the following call stack:

CGImageSourceCopyPropertiesAtIndex()
WebCore::ImageSource::frameDurationAtIndex(unsigned long)
WebCore::BitmapImage::cacheFrameInfo(unsigned long)

The crash happens in CGImageSourceCopyPropertiesAtIndex() when it is called from ImageSource::frameDurationAtIndex(). But before BitmapImage::cacheFrameInfo() calls ImageSource::frameDurationAtIndex(), it calls ImageSource::orientationAtIndex() which calls CGImageSourceCopyPropertiesAtIndex() with exactly the same parameters. Two possible scenarios may have happened here:

1) The frame metadata was not complete when CGImageSourceCopyPropertiesAtIndex() was called the first time so it returns null and we fall back to the default value case in ImageSource::orientationAtIndex(). But in the second time, the image metadata loading was in progress but not complete, so we end up reading from still not allocated memory buffer.
2) The frame metadata was complete when CGImageSourceCopyPropertiesAtIndex() was called the first time but the frame itself was freed when the second call was made, so end up reading from a freed memory block.
Comment 1 Said Abou-Hallawa 2015-05-08 13:13:22 PDT
Created attachment 252738 [details]
Patch
Comment 2 Said Abou-Hallawa 2015-05-08 13:24:16 PDT
Created attachment 252741 [details]
Patch
Comment 3 Said Abou-Hallawa 2015-05-08 14:55:02 PDT
Created attachment 252749 [details]
Patch
Comment 4 Said Abou-Hallawa 2015-05-14 09:02:13 PDT
The crash I suspected to be the result of this race condition has been fixed in one of the MacOS/iOS components. So this is not an issue anymore.