Bug 170912 - [iOS, macOS] Guard against passing nullptr to vImagePremultiplyData
Summary: [iOS, macOS] Guard against passing nullptr to vImagePremultiplyData
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-04-17 13:35 PDT by Brent Fulgham
Modified: 2017-04-19 09:17 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.38 KB, patch)
2017-04-17 13:38 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (2.92 KB, patch)
2017-04-18 17:17 PDT, Brent Fulgham
beidson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2017-04-17 13:35:27 PDT
If the system is under heavy memory pressure, it is possible for a calloc to fail, resulting in an ImageBuffer with a nullptr data member.

We should return from an attempt to perform ImageBuffer::putData on a nullptr without taking any action, or perhaps RELEASE_ASSERT that we encountered this case.

Since the memory pressure may be transient, it seems reasonable to just bail out early; a future attempt may succeed.
Comment 1 Brent Fulgham 2017-04-17 13:38:05 PDT
Created attachment 307292 [details]
Patch
Comment 2 Brent Fulgham 2017-04-18 13:12:55 PDT
<rdar://problem/30565800>
Comment 3 Brady Eidson 2017-04-18 16:59:42 PDT
Comment on attachment 307292 [details]
Patch

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

> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:444
> +        ASSERT(data);
> +        if (!data)
> +            return;

We can't really both expect that is' impossible to have a null data (like the assert says) but also expect it is possible to have a null data (like the early return says)

If it's truly impossible, we need to know why.

If it's truly possible (such as we commonly get into the case where memory can't be allocated), then the ASSERT should be dropped.
Comment 4 Brent Fulgham 2017-04-18 17:16:58 PDT
Comment on attachment 307292 [details]
Patch

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

>> Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:444
>> +            return;
> 
> We can't really both expect that is' impossible to have a null data (like the assert says) but also expect it is possible to have a null data (like the early return says)
> 
> If it's truly impossible, we need to know why.
> 
> If it's truly possible (such as we commonly get into the case where memory can't be allocated), then the ASSERT should be dropped.

Good point. I'll revise.
Comment 5 Brent Fulgham 2017-04-18 17:17:02 PDT
Created attachment 307439 [details]
Patch
Comment 6 Brent Fulgham 2017-04-19 09:17:55 PDT
Committed r215514: <http://trac.webkit.org/changeset/215514>