Bug 119327 - Replace CRASH() on overflow with Checked<>
Summary: Replace CRASH() on overflow with Checked<>
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kwang Yul Seo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-31 00:20 PDT by Kwang Yul Seo
Modified: 2013-08-01 12:45 PDT (History)
7 users (show)

See Also:


Attachments
Patch (4.04 KB, patch)
2013-07-31 00:34 PDT, Kwang Yul Seo
no flags Details | Formatted Diff | Diff
Patch (4.54 KB, patch)
2013-07-31 16:46 PDT, Kwang Yul Seo
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kwang Yul Seo 2013-07-31 00:20:10 PDT
Use a checked type that allows us to automate bound checks. We use a non-recording Checked<> to keep the behavior.
Comment 1 Kwang Yul Seo 2013-07-31 00:34:42 PDT
Created attachment 207813 [details]
Patch
Comment 2 Oliver Hunt 2013-07-31 14:07:29 PDT
Comment on attachment 207813 [details]
Patch

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

> Source/WebCore/platform/audio/AudioArray.h:55
>      void allocate(size_t n)

Make n Checked<size_t>, otherwise the multiplication below may silently fail before the arithmetic becomes safe.
Comment 3 Kwang Yul Seo 2013-07-31 16:46:54 PDT
Created attachment 207886 [details]
Patch
Comment 4 Kwang Yul Seo 2013-07-31 16:47:57 PDT
(In reply to comment #2)
> Make n Checked<size_t>, otherwise the multiplication below may silently fail before the arithmetic becomes safe.

Done. Thanks for the review.
Comment 5 Kwang Yul Seo 2013-07-31 17:02:02 PDT
Committed r153548: <http://trac.webkit.org/changeset/153548>
Comment 6 Darin Adler 2013-08-01 12:45:49 PDT
Comment on attachment 207886 [details]
Patch

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

> Source/WebCore/platform/audio/AudioArray.h:55
> +    void allocate(Checked<size_t> n)

I think it’s not good to make <n> itself a Checked. There must be a better way to make the initialSize computation be checked.