New issue
Advanced search Search tips

Issue 739669 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

MSan: uninitialized read in cc::PaintOpReader::ReadData

Project Member Reported by glider@chromium.org, Jul 6 2017

Issue description

Two bots are complaining: https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20ChromiumOS%20MSan%20Tests and https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20MSan%20Tests

Sample report (from https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.memory%2FLinux_ChromiumOS_MSan_Tests%2F1566%2F%2B%2Frecipes%2Fsteps%2Fcc_unittests%2F0%2Flogs%2FP__x2f_PaintOpSerializationTest.DeserializationFailures__x2f_19%2F0):

[ RUN      ] P/PaintOpSerializationTest.DeserializationFailures/19
==22519==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x42b57d4 in cc::PaintOpReader::ReadData(unsigned long, void*) cc/paint/paint_op_reader.cc:30:7
    #1 0x42ab134 in cc::DrawTextOp::Deserialize(void const*, unsigned long, void*, unsigned long) cc/paint/paint_op_buffer.cc:947:10
    #2 0x42afc2b in cc::PaintOp::Deserialize(void const*, unsigned long, void*, unsigned long) cc/paint/paint_op_buffer.cc:1359:10
    #3 0x185cd77 in cc::PaintOpSerializationTest_DeserializationFailures_Test::TestBody() cc/paint/paint_op_buffer_unittest.cc:2000:26
    #4 0x3aa2d9b in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc:2455:12
    #5 0x3aa2d9b in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2471:0
    #6 0x3aa66b0 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2653:11
    #7 0x3aa80a9 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2771:28
    #8 0x3ac86c3 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4648:43
    #9 0x3ac7541 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc:2455:12
    #10 0x3ac7541 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4256:0
    #11 0x421d4a4 in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2237:46
    #12 0x421d4a4 in base::TestSuite::Run() base/test/test_suite.cc:271:0
    #13 0x4223913 in Run base/callback.h:80:12
    #14 0x4223913 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, unsigned long, int, bool, base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:216:0
    #15 0x4223121 in base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:462:10
    #16 0x3872d6e in main cc/test/run_all_unittests.cc:15:10
    #17 0x7fae8fee6f44 in __libc_start_main /build/eglibc-MjiXCM/eglibc-2.19/csu/libc-start.c:287:0
    #18 0x6c5c07 in _start ??:0:0
  Uninitialized value was created by a heap allocation
    #0 0x6e2e83 in __interceptor_posix_memalign ??:0:0
    #1 0x462712f in base::AlignedAlloc(unsigned long, unsigned long) base/memory/aligned_memory.cc:31:7
    #2 0x185c571 in cc::PaintOpSerializationTest_DeserializationFailures_Test::TestBody() cc/paint/paint_op_buffer_unittest.cc:1980:26
    #3 0x3aa2d9b in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc:2455:12
    #4 0x3aa2d9b in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2471:0
    #5 0x3aa66b0 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2653:11
    #6 0x3aa80a9 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2771:28
    #7 0x3ac86c3 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4648:43
    #8 0x3ac7541 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc:2455:12
    #9 0x3ac7541 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4256:0
    #10 0x421d4a4 in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2237:46
    #11 0x421d4a4 in base::TestSuite::Run() base/test/test_suite.cc:271:0
    #12 0x4223913 in Run base/callback.h:80:12
    #13 0x4223913 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, unsigned long, int, bool, base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:216:0
    #14 0x4223121 in base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:462:10
    #15 0x3872d6e in main cc/test/run_all_unittests.cc:15:10
    #16 0x7fae8fee6f44 in __libc_start_main /build/eglibc-MjiXCM/eglibc-2.19/csu/libc-start.c:287:0
SUMMARY: MemorySanitizer: use-of-uninitialized-value (/b/s/w/ir/out/Release/cc_unittests+0x42b57d4)
Exiting

and

[ RUN      ] P/PaintOpSerializationTest.DeserializationFailures/15
==22518==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x42b59d8 in cc::PaintOpReader::ReadArray(unsigned long, SkPoint*) cc/paint/paint_op_reader.cc:42:7
    #1 0x42aa010 in cc::DrawPosTextOp::Deserialize(void const*, unsigned long, void*, unsigned long) cc/paint/paint_op_buffer.cc:874:10
    #2 0x42afc2b in cc::PaintOp::Deserialize(void const*, unsigned long, void*, unsigned long) cc/paint/paint_op_buffer.cc:1359:10
    #3 0x185cd77 in cc::PaintOpSerializationTest_DeserializationFailures_Test::TestBody() cc/paint/paint_op_buffer_unittest.cc:2000:26
    #4 0x3aa2d9b in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc:2455:12
    #5 0x3aa2d9b in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2471:0
    #6 0x3aa66b0 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2653:11
    #7 0x3aa80a9 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2771:28
    #8 0x3ac86c3 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4648:43
    #9 0x3ac7541 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc:2455:12
    #10 0x3ac7541 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4256:0
    #11 0x421d4a4 in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2237:46
    #12 0x421d4a4 in base::TestSuite::Run() base/test/test_suite.cc:271:0
    #13 0x4223913 in Run base/callback.h:80:12
    #14 0x4223913 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, unsigned long, int, bool, base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:216:0
    #15 0x4223121 in base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:462:10
    #16 0x3872d6e in main cc/test/run_all_unittests.cc:15:10
    #17 0x7efc2e286f44 in __libc_start_main /build/eglibc-MjiXCM/eglibc-2.19/csu/libc-start.c:287:0
    #18 0x6c5c07 in _start ??:0:0
  Uninitialized value was created by a heap allocation
    #0 0x6e2e83 in __interceptor_posix_memalign ??:0:0
    #1 0x462712f in base::AlignedAlloc(unsigned long, unsigned long) base/memory/aligned_memory.cc:31:7
    #2 0x185c571 in cc::PaintOpSerializationTest_DeserializationFailures_Test::TestBody() cc/paint/paint_op_buffer_unittest.cc:1980:26
    #3 0x3aa2d9b in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc:2455:12
    #4 0x3aa2d9b in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2471:0
    #5 0x3aa66b0 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2653:11
    #6 0x3aa80a9 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2771:28
    #7 0x3ac86c3 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4648:43
    #8 0x3ac7541 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc:2455:12
    #9 0x3ac7541 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4256:0
    #10 0x421d4a4 in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2237:46
    #11 0x421d4a4 in base::TestSuite::Run() base/test/test_suite.cc:271:0
    #12 0x4223913 in Run base/callback.h:80:12
    #13 0x4223913 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, unsigned long, int, bool, base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:216:0
    #14 0x4223121 in base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) base/test/launcher/unit_test_launcher.cc:462:10
    #15 0x3872d6e in main cc/test/run_all_unittests.cc:15:10
    #16 0x7efc2e286f44 in __libc_start_main /build/eglibc-MjiXCM/eglibc-2.19/csu/libc-start.c:287:0
SUMMARY: MemorySanitizer: use-of-uninitialized-value (/b/s/w/ir/out/Release/cc_unittests+0x42b59d8)
Exiting

https://chromium-review.googlesource.com/c/528359/ is the culprit
 
I'm gonna revert the offending CL
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 7 2017

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

commit 5b040f683487f9eb386d8b5a8b71ee4ad1266c5e
Author: Adrienne Walker <enne@chromium.org>
Date: Fri Jul 07 17:07:09 2017

cc: PaintOp serialization

Adds basic support for serializing and deserializing paint ops.

This punts on a number of types related to Skia, including
SkTypeface, SkTextBlob, PaintFlags/SkPaint internals, and
SkImage/PaintImage.  These will be supported in follow up patches
either by adding Skia support directly (like SkPath has) or by
replacing recording data structures (like PaintShader).

An original version of this patch optionally would sometimes cast
the input buffer and return that, but for simplicity, this always
writes a copy of the op into the output buffer.  This eliminates
a class of TOCTOU issues and we can rethink it later as needed
for performance.

Reland of: https://chromium-review.googlesource.com/c/528359/

Bug:  737629 , 739669 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: If45b40928b4eb7b6c411a69fc7178a570e5e87e1
Reviewed-on: https://chromium-review.googlesource.com/561921
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484962}
[modify] https://crrev.com/5b040f683487f9eb386d8b5a8b71ee4ad1266c5e/cc/paint/BUILD.gn
[modify] https://crrev.com/5b040f683487f9eb386d8b5a8b71ee4ad1266c5e/cc/paint/paint_flags.h
[modify] https://crrev.com/5b040f683487f9eb386d8b5a8b71ee4ad1266c5e/cc/paint/paint_op_buffer.cc
[modify] https://crrev.com/5b040f683487f9eb386d8b5a8b71ee4ad1266c5e/cc/paint/paint_op_buffer.h
[modify] https://crrev.com/5b040f683487f9eb386d8b5a8b71ee4ad1266c5e/cc/paint/paint_op_buffer_unittest.cc
[add] https://crrev.com/5b040f683487f9eb386d8b5a8b71ee4ad1266c5e/cc/paint/paint_op_reader.cc
[add] https://crrev.com/5b040f683487f9eb386d8b5a8b71ee4ad1266c5e/cc/paint/paint_op_reader.h
[add] https://crrev.com/5b040f683487f9eb386d8b5a8b71ee4ad1266c5e/cc/paint/paint_op_writer.cc
[add] https://crrev.com/5b040f683487f9eb386d8b5a8b71ee4ad1266c5e/cc/paint/paint_op_writer.h

Comment 3 by enne@chromium.org, Aug 28 2017

Status: Fixed (was: Assigned)

Sign in to add a comment