New issue
Advanced search Search tips

Issue 748657 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Undefined-shift in blink::LayoutUnit::LayoutUnit

Project Member Reported by ClusterFuzz, Jul 25 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=5411572804747264

Fuzzer: libFuzzer_v8_serialized_script_value_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Undefined-shift
Crash Address: 
Crash State:
  blink::LayoutUnit::LayoutUnit
  blink::LayoutBox::LayoutBox
  blink::LayoutBlock::LayoutBlock
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=489276:489324

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5411572804747264


Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Components: Blink>Layout

Comment 2 by e...@chromium.org, Aug 21 2017

Owner: jsc...@chromium.org
Status: Assigned (was: Untriaged)
Justin, your change to move LayoutUnit back to WTF saturation is the only WTF change in the regression range. Any idea why this is triggering now but not before?

Comment 3 by jsc...@chromium.org, Aug 21 2017

Uh... this makes no sense to me. The blamed line is a template parameter—meaning it's a compile-time constant that can never be negative. I cannot imagine how ubsan could even trigger on this.

Comment 4 by jsc...@chromium.org, Aug 21 2017

Nevermind, I misread the report. This shift is definitely undefined behavior, and always has been. Either the code change allowed ubsan to find it, or it just randomly hit it now.

Regardless, it's really easy to fix and I'm totally looking for something to procrastinate on. so, I'll have a CL up shortly.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4a91cc41c00e7e3bee6a5fcb4c250176c8f55129

commit 4a91cc41c00e7e3bee6a5fcb4c250176c8f55129
Author: Justin Schuh <jschuh@chromium.org>
Date: Wed Aug 23 20:26:45 2017

Fix undefined behavior on shift in SaturatedSet()

Doesn't change emitted code, but eliminates undefined behavior.

Bug:  748657 
Change-Id: I00707257f9fe48fcc5bb3747e1c54717c128e418
Reviewed-on: https://chromium-review.googlesource.com/624947
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Justin Schuh <jschuh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496790}
[modify] https://crrev.com/4a91cc41c00e7e3bee6a5fcb4c250176c8f55129/third_party/WebKit/Source/platform/wtf/SaturatedArithmetic.h

Project Member

Comment 6 by ClusterFuzz, Aug 24 2017

ClusterFuzz has detected this issue as fixed in range 496738:496798.

Detailed report: https://clusterfuzz.com/testcase?key=5411572804747264

Fuzzer: libFuzzer_v8_serialized_script_value_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Undefined-shift
Crash Address: 
Crash State:
  blink::LayoutUnit::LayoutUnit
  blink::LayoutBox::LayoutBox
  blink::LayoutBlock::LayoutBlock
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=489276:489324
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=496738:496798

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5411572804747264

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 7 by ClusterFuzz, Aug 24 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 5411572804747264 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Sign in to add a comment