189419 2018-09-07 10:54:26 -0700 [GStreamer] Fix overflow in buffered ranges 2018-09-10 07:04:40 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 aboya aboya bugs-noreply calvaris commit-queue eocanha pnormand oldest_to_newest 1457555 0 aboya 2018-09-07 10:54:26 -0700 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. 1457559 1 349161 aboya 2018-09-07 10:59:09 -0700 Created attachment 349161 Patch 1457659 2 349161 pnormand 2018-09-07 13:59:45 -0700 Comment on attachment 349161 Patch Nice catch :) 1458122 3 349161 commit-queue 2018-09-10 07:04:39 -0700 Comment on attachment 349161 Patch Clearing flags on attachment: 349161 Committed r235848: <https://trac.webkit.org/changeset/235848> 1458123 4 commit-queue 2018-09-10 07:04:40 -0700 All reviewed patches have been landed. Closing bug. 349161 2018-09-07 10:59:09 -0700 2018-09-10 07:04:39 -0700 Patch bug-189419-20180907195907.patch text/plain 2337 aboya U3VidmVyc2lvbiBSZXZpc2lvbjogMjM1NjI4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggOGE4YjZjMTRhZDlhNDFl ZmIyY2QzM2Q0Yjk0MjVlZWYyNGMzYzVhYi4uYzExYzNlM2M5YjE1Y2Y5Njk2N2E5ZmEzZjFmNGY4 YWNhMmQxMzE3YyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDE4LTA5LTA3ICBBbGlj aWEgQm95YSBHYXJjw61hICA8YWJveWFAaWdhbGlhLmNvbT4KKworICAgICAgICBbR1N0cmVhbWVy XSBGaXggb3ZlcmZsb3cgaW4gYnVmZmVyZWQgcmFuZ2VzCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODk0MTkKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBTY2FsZSBvcGVyYXRpb25zIG9uIGJpZyBudW1iZXJz IChsaWtlIG1lZGlhIHRpbWVzdGFtcHMgb3IgZHVyYXRpb25zKQorICAgICAgICBzaG91bGQgYmUg bWFkZSB3aXRoIEdTdHJlYW1lciB1dGlsaXR5IGZ1bmN0aW9ucyB0byBhdm9pZCBvdmVyZmxvd3Mu CisKKyAgICAgICAgVGhpcyBmaXhlcyBhbiBhc3NlcnRpb24gd2hlbiBhIDI0IGhvdXIgbG9uZyBm cmFnbWVudGVkIE1QNCBmaWxlIGlzCisgICAgICAgIHBsYXllZC4KKworICAgICAgICAqIHBsYXRm b3JtL2dyYXBoaWNzL2dzdHJlYW1lci9NZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIuY3BwOgor ICAgICAgICAoV2ViQ29yZTo6TWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyOjpidWZmZXJlZCBj b25zdCk6CisKIDIwMTgtMDktMDMgIERlYW4gSmFja3NvbiAgPGRpbm9AYXBwbGUuY29tPgogCiAg ICAgICAgIE1vdmUgU3lzdGVtUHJldmlldyBjb2RlIGZyb20gV2ViS2l0QWRkaXRpb25zIHRvIFdl YktpdApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVh bWVyL01lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyLmNwcApp bmRleCBhMzZkMWE0OTU2NzQzODkyZmU2NjUxZTRmMDgwY2RlYTJjOGRjYzczLi4wMzg5Y2RhMzE4 NDhlOWQ4OTBkZGFkMGYxNTNiOWRlYzA0MWI3YTM0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVy LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVk aWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyLmNwcApAQCAtMTEyNCw4ICsxMTI0LDkgQEAgc3RkOjp1 bmlxdWVfcHRyPFBsYXRmb3JtVGltZVJhbmdlcz4gTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVy OjpidWZmZXJlZCgpIGNvbnMKICAgICBmb3IgKGd1aW50IGluZGV4ID0gMDsgaW5kZXggPCBudW1C dWZmZXJpbmdSYW5nZXM7IGluZGV4KyspIHsKICAgICAgICAgZ2ludDY0IHJhbmdlU3RhcnQgPSAw LCByYW5nZVN0b3AgPSAwOwogICAgICAgICBpZiAoZ3N0X3F1ZXJ5X3BhcnNlX250aF9idWZmZXJp bmdfcmFuZ2UocXVlcnksIGluZGV4LCAmcmFuZ2VTdGFydCwgJnJhbmdlU3RvcCkpIHsKLSAgICAg ICAgICAgIHRpbWVSYW5nZXMtPmFkZChNZWRpYVRpbWUocmFuZ2VTdGFydCAqIHRvR3N0VW5zaWdu ZWQ2NFRpbWUobWVkaWFEdXJhdGlvbikgLyBHU1RfRk9STUFUX1BFUkNFTlRfTUFYLCBHU1RfU0VD T05EKSwKLSAgICAgICAgICAgICAgICBNZWRpYVRpbWUocmFuZ2VTdG9wICogdG9Hc3RVbnNpZ25l ZDY0VGltZShtZWRpYUR1cmF0aW9uKSAvIEdTVF9GT1JNQVRfUEVSQ0VOVF9NQVgsIEdTVF9TRUNP TkQpKTsKKyAgICAgICAgICAgIHVpbnQ2NF90IHN0YXJ0VGltZSA9IGdzdF91dGlsX3VpbnQ2NF9z Y2FsZV9pbnRfcm91bmQodG9Hc3RVbnNpZ25lZDY0VGltZShtZWRpYUR1cmF0aW9uKSwgcmFuZ2VT dGFydCwgR1NUX0ZPUk1BVF9QRVJDRU5UX01BWCk7CisgICAgICAgICAgICB1aW50NjRfdCBzdG9w VGltZSA9IGdzdF91dGlsX3VpbnQ2NF9zY2FsZV9pbnRfcm91bmQodG9Hc3RVbnNpZ25lZDY0VGlt ZShtZWRpYUR1cmF0aW9uKSwgcmFuZ2VTdG9wLCBHU1RfRk9STUFUX1BFUkNFTlRfTUFYKTsKKyAg ICAgICAgICAgIHRpbWVSYW5nZXMtPmFkZChNZWRpYVRpbWUoc3RhcnRUaW1lLCBHU1RfU0VDT05E KSwgTWVkaWFUaW1lKHN0b3BUaW1lLCBHU1RfU0VDT05EKSk7CiAgICAgICAgIH0KICAgICB9CiAK