Bug 189419 - [GStreamer] Fix overflow in buffered ranges
Summary: [GStreamer] Fix overflow in buffered ranges
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alicia Boya García
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-09-07 10:54 PDT by Alicia Boya García
Modified: 2018-09-10 07:04 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.28 KB, patch)
2018-09-07 10:59 PDT, Alicia Boya García
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alicia Boya García 2018-09-07 10:54:26 PDT
I found one case of overflow when scaling the results of a GstQuery. The code was like this:

rangeStart * toGstUnsigned64Time(mediaDuration) / GST_FORMAT_PERCENT_MAX

In this case mediaDuration was 24 hours. rangeStart was a signed integer. Computing the multiplication:

rangeStart = (int64_t) 999999
toGstUnsigned64Time(mediaDuration) = (uint64_t) 86408208000000

> (int64_t) 999999 * (uint64_t) 86408208000000
12621145296953793536

Which does not look that strange on first sight, but if you actually run the calculation on a calculator supporting big ints, like Python's:

In [1]: 999999 * 86408208000000
Out[1]: 86408121591792000000

It's now clear that the previous value was wrong: the result overflew.  This caused an inconsistency that was fortunately caught by an assertion:

ASSERTION FAILED: start <= end
#0  WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:267
#1  0x00007ff7f03cd413 in WebCore::PlatformTimeRanges::add (this=0x21ff490, start=..., end=...) at ../../Source/WebCore/platform/graphics/PlatformTimeRanges.cpp:141
#2  0x00007ff7f0d80d3a in WebCore::MediaPlayerPrivateGStreamer::buffered (this=0x7ff760856a80) at ../../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1127
#3  0x00007ff7f03bdf6b in WebCore::MediaPlayer::buffered (this=0x7ff7cd168410) at ../../Source/WebCore/platform/graphics/MediaPlayer.cpp:818
#4  0x00007ff7efcf9a73 in WebCore::HTMLMediaElement::buffered (this=0x7ff760c00778) at ../../Source/WebCore/html/HTMLMediaElement.cpp:5116
#5  0x00007ff7f10b244e in WebCore::RenderThemeGtk::paintMediaSliderTrack (this=0x7ff7f67cf440 <WebCore::RenderTheme::singleton()::theme>, o=..., paintInfo=..., r=...)
    at ../../Source/WebCore/rendering/RenderThemeGtk.cpp:1893
#6  0x00007ff7f07e9f93 in WebCore::RenderTheme::paint (this=0x7ff7f67cf440 <WebCore::RenderTheme::singleton()::theme>, box=..., controlStates=..., paintInfo=..., rect=...)
    at ../../Source/WebCore/rendering/RenderTheme.cpp:385
[...]

GStreamer has utility functions to perform these scale operations safely, the attached patch uses them.
Comment 1 Alicia Boya García 2018-09-07 10:59:09 PDT
Created attachment 349161 [details]
Patch
Comment 2 Philippe Normand 2018-09-07 13:59:45 PDT
Comment on attachment 349161 [details]
Patch

Nice catch :)
Comment 3 WebKit Commit Bot 2018-09-10 07:04:39 PDT
Comment on attachment 349161 [details]
Patch

Clearing flags on attachment: 349161

Committed r235848: <https://trac.webkit.org/changeset/235848>
Comment 4 WebKit Commit Bot 2018-09-10 07:04:40 PDT
All reviewed patches have been landed.  Closing bug.