mtklein@, could you please take a look and may be suggest another owner?

This happens when someone tries to draw an uninitialized image onto some other image.
The bug is not in blit_row_s32a_opaque.  The failure we're seeing here is its defensive assertion that its input is initialized:
  Uninitialized bytes in __msan_check_mem_is_initialized at offset 0 inside [0x7f43f8bbb000, 924)

The problem is likely way up-stack from blit_row_s32a_opaque.
Not sure who to pass this on to.

reed@, since most of stack frames refer to your CLs, could you please take a look?

reed@ -- setting current milestone (M50) as the milestone since it is a Medium Pri bug. Please feel free to change.

reed: Uh oh! This issue still open and hasn't been updated in the last 25 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

reed: Uh oh! This issue still open and hasn't been updated in the last 40 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

repeating #2 -- the caller has passed in an uninitialized buffer. skia can safely handle any byte values, so there is no chance for skia to behave oddly.

unassigning skia ...

Are you saying that something in the picture_ object passed to drawPicture (https://code.google.com/p/chromium/codesearch#chromium/src/cc/playback/display_item_list.cc&sq=package:chromium&type=cs&l=142) is not initialized correctly? If I understand correctly, the rest of the stack is all skia  code after the call to drawPicture (https://cluster-fuzz.appspot.com/testcase?key=5204216926175232).

Yes, but all image buffers are just ref'd from the caller, as skia just records the buffers during the "record" phase, and then later is told to "playback" into a drawing canvas.

Remember that if contents of uninitialized memory end up on the screen 
this is an information leak that may allow further ASLR bypass 

Looking at the "most recent" crash at the bottom, I see a slightly different (looking) code-path. Looping in Robert and Stephen.

Could this be related to an uninitialized special-surface/image (as part of saveLayer or not) ?

It's possible. Canvas uses an SkPictureImageFilter to draw the image at a fixed DPI.

There was a fix that went into SkPictureImageFilter from ericrk@ recently that may be related, and may have fixed it.

Fix was here: https://codereview.chromium.org/1969193002. Went in on May 12. However, I would've expected ClusterFuzz to have picked up the fix by now, if it did fix it.

Did the fix only go into trunk? According to the ClusterFuzz report, this is affecting:

Stable (49.0.2623.110)
Beta (50.0.2661.57)

It only went into trunk AFAIK. I'm pretty sure that problem was introduced in M52, or M51 at the earliest, so shouldn't be showing up in M49. Robert, could you confirm?

For what it's worth, ClusterFuzz thinks the regression occurred at some point between 361453 and 361496. Not an absolute of course, but that puts this back in November. Are you able to view the detailed report? The suspected CLs list might be interesting.

Hmm. There are only two Skia changes in that range, but https://codereview.chromium.org/1476563002 might be plausible. +herb

There's also https://codereview.chromium.org/1457783005, which is interesting because the canvas test case has a gradient in it. +fmalita

Unfortunately I'm WFH and don't have access to a system to repro MSAN failures ATM.

It's quite possible this is related to the gradient change - the allocation trace points to SkARGB32_Shader_Blitter and the repro uses a gradient stroke:

Uninitialized value was created by a heap allocation
    #0 0x7f45ba6d50d2 in __interceptor_malloc
    #1 0x7f45c07d3451 in sk_malloc_throw(unsigned long) skia/ext/SkMemory_new_handler.cpp:58:66
    #2 0x7f45bfbbb47d in SkARGB32_Shader_Blitter::SkARGB32_Shader_Blitter(SkPixmap const&, SkPaint const&, SkShader::Context*) third_party/skia/src/core/SkBlitter_ARGB32.cpp:326:27
    #3 0x7f45bfb85def in createT<SkARGB32_Shader_Blitter, SkPixmap, SkPaint, SkShader::Context *> third_party/skia/src/core/SkSmallAllocator.h:64:26
    #4 0x7f45bfb85def in SkBlitter::Choose(SkPixmap const&, SkMatrix const&, SkPaint const&, SkSmallAllocator<3u, 2100ul>*, bool) third_party/skia/src/core/SkBlitter.cpp:928
    #5 0x7f45bf23a919 in choose third_party/skia/src/core/SkDraw.cpp:64:20
    #6 0x7f45bf23a919 in SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool, bool, SkBlitter*) const third_party/skia/src/core/SkDraw.cpp:1112
    #7 0x7f45bfb03902 in drawPath third_party/skia/include/core/SkDraw.h:54:9
    #8 0x7f45bfb03902 in SkBitmapDevice::drawPath(SkDraw const&, SkPath const&, SkPaint const&, SkMatrix const*, bool) third_party/skia/src/core/SkBitmapDevice.cpp:235
    #9 0x7f45bf1d6552 in SkCanvas::onDrawPath(SkPath const&, SkPaint const&) third_party/skia/src/core/SkCanvas.cpp:2198:9
    #10 0x7f45bf3f4893 in draw<SkRecords::DrawPath> third_party/skia/src/core/SkRecordDraw.cpp:106:1

I can repro in a local MSAN build, and can confirm that it's related to https://codereview.chromium.org/1457783005.

 Issue 599629  has been merged into this issue.

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/7b38e3cf75296c749c843fa89af14f70f4e4b2db

commit 7b38e3cf75296c749c843fa89af14f70f4e4b2db
Author: fmalita <fmalita@chromium.org>
Date: Thu May 26 18:13:52 2016

Fix int32 overflow in LinearGradientContext::shade4_dx_clamp

The unconditional increment in shade4_dx_clamp can overflow int32

=> n == SK_MinS32
=> count ~= SK_MinS32
=> we skip the main shader loop 'cause count < 0

R=reed@google.com,mtklein@google.com
BUG= chromium:599458 
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2010843002

Review-Url: https://codereview.chromium.org/2010843002

[modify] https://crrev.com/7b38e3cf75296c749c843fa89af14f70f4e4b2db/src/effects/gradients/SkLinearGradient.cpp
[modify] https://crrev.com/7b38e3cf75296c749c843fa89af14f70f4e4b2db/tests/GradientTest.cpp

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

commit 1511a4c4e3cf9238f6cc074f8f82aafec56a23fd
Author: skia-deps-roller <skia-deps-roller@chromium.org>
Date: Thu May 26 22:11:10 2016

Roll src/third_party/skia/ 0e5b249e5..4e4e30823 (4 commits).

https://chromium.googlesource.com/skia.git/+log/0e5b249e549a..4e4e30823fba

$ git log 0e5b249e5..4e4e30823 --date=short --no-merges --format='%ad %ae %s'
2016-05-26 liyuqian Now we can use drawer to view the state information of the native app, and set its state using the spinner.
2016-05-26 reed add more SK_WARN_UNUSED_RESULT attributes to functions that ignore out-params if they fail
2016-05-26 borenet Temporarily change GalaxyS3 product.board
2016-05-26 fmalita Fix int32 overflow in LinearGradientContext::shade4_dx_clamp

BUG= 599458 

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=kjlubick@google.com

Review-Url: https://codereview.chromium.org/2016843002
Cr-Commit-Position: refs/heads/master@{#396298}

[modify] https://crrev.com/1511a4c4e3cf9238f6cc074f8f82aafec56a23fd/DEPS

Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Request-XX label, where XX is the Chrome milestone.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz

ClusterFuzz has detected this issue as fixed in range 396253:396347.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5204216926175232

Fuzzer: inferno_canvas_wrecker
Job Type: linux_msan_chrome
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  sk_sse41::blit_row_s32a_opaque
  Sprite_D32_S32::blitRect
  SkScan::FillIRect
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=361453:361496
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=396253:396347

Minimized Testcase (0.97 Kb): https://cluster-fuzz.appspot.com/download/AMIfv94j7EZj51YyTil7YUo0Z60Q3I_li5XCsil7xQLoetPthC4woFLu3pIPxnh1ZWB3HOpeR3XiDtRrtKvccZPzsOyTRcZhsdpTKkXU8TKEm4KrQv7bGg_zLQirCHNzXqFDI4TK0ho5_V2WXjLZC1xKJ234uE0zJA

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

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

ClusterFuzz has detected this issue as fixed in range 396253:396347.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5204216926175232

Fuzzer: inferno_canvas_wrecker
Job Type: linux_msan_chrome
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  sk_sse41::blit_row_s32a_opaque
  Sprite_D32_S32::blitRect
  SkScan::FillIRect
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=361453:361496
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=396253:396347

Minimized Testcase (0.97 Kb): https://cluster-fuzz.appspot.com/download/AMIfv94j7EZj51YyTil7YUo0Z60Q3I_li5XCsil7xQLoetPthC4woFLu3pIPxnh1ZWB3HOpeR3XiDtRrtKvccZPzsOyTRcZhsdpTKkXU8TKEm4KrQv7bGg_zLQirCHNzXqFDI4TK0ho5_V2WXjLZC1xKJ234uE0zJA

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

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

The fix is low-risk, and should be safe to merge in M51/M52.

[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.

[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.

[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.

Internally reported MSan bugs and internally reported Medium severity bugs only go to beta, so let's land it on M52.

Tina/Krishna - please approve for M52.

OK, Approved for M52 branch 2743 based on comment #38. Please merge ASAP.

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/1fd666cbb9655e1ce5f37f435d1f9ea2d7b47827

commit 1fd666cbb9655e1ce5f37f435d1f9ea2d7b47827
Author: fmalita <fmalita@chromium.org>
Date: Sat May 28 13:57:33 2016

[M52] Cherry-pick e6c515f3d34096426b6822ad90a757131e8baf31

Fix int32 overflow in LinearGradientContext::shade4_dx_clamp

The unconditional increment in shade4_dx_clamp can overflow int32

=> n == SK_MinS32
=> count ~= SK_MinS32
=> we skip the main shader loop 'cause count < 0

Also include trivial 0e59bb7aaad272ac42d6fba53e8439bd9fa1ff3d followup
to ensure we're not tripping ASAN.

TBR=reed@google.com,mtklein@google.com
BUG= chromium:599458 

Review-Url: https://codereview.chromium.org/2010843002
NOTREECHECKS=true
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2021673002

[modify] https://crrev.com/1fd666cbb9655e1ce5f37f435d1f9ea2d7b47827/src/effects/gradients/SkLinearGradient.cpp
[modify] https://crrev.com/1fd666cbb9655e1ce5f37f435d1f9ea2d7b47827/tests/GradientTest.cpp

As per c#38, assuming this doesn't need to go in M51. Please remove merge request labels if that is still the case. If not, please update the bug appropriately. 

Per comment #40, this is already merged to M52. So removing "Merge-Approved-52" label.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot