New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 847096 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocking:
issue 846800



Sign in to add a comment

Linux Msan failing browser_tests (NetworkContextConfigurationBrowserTest.FileURL) since r562045

Project Member Reported by tapted@chromium.org, May 28 2018

Issue description

https://ci.chromium.org/buildbot/chromium.memory/Linux%20MSan%20Tests/10019 (r562039 - r562064)

https://ci.chromium.org/buildbot/chromium.memory/Linux%20ChromiumOS%20MSan%20Tests/7306 (562009 - r562064)

browser_tests browser_tests
Run on OS: 'Ubuntu-14.04'
Max shard duration: 0:28:33.658760 (shard #2)
failures:
SafeBrowsingNetworkContext/NetworkContextConfigurationBrowserTest.FileURL/2
SafeBrowsingNetworkContext/NetworkContextConfigurationBrowserTest.FileURL/1
IncognitoProfileMainNetworkContext/NetworkContextConfigurationBrowserTest.FileURL/1
IncognitoProfileMainNetworkContext/NetworkContextConfigurationBrowserTest.FileURL/2
SystemNetworkContext/NetworkContextConfigurationBrowserTest.FileURL/1
SystemNetworkContext/NetworkContextConfigurationBrowserTest.FileURL/2
ProfileMainNetworkContext/NetworkContextConfigurationBrowserTest.FileURL/2
ProfileMainNetworkContext/NetworkContextConfigurationBrowserTest.FileURL/1

Suspect

Proxy lstat() system calls to broker process for network service.

This particular stat() variant was not being handled in the same
way as the others.

Bug:  846800 
Change-Id: I2ec9b94f704f9f00aa3dd1dbd0a3be71a0583b75
Reviewed-on: https://chromium-review.googlesource.com/1073869
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562045}



[ RUN      ] SafeBrowsingNetworkContext/NetworkContextConfigurationBrowserTest.FileURL/2
Xlib:  extension "RANDR" missing on display ":99".
[7734:7961:0525/181643.232204:ERROR:bus.cc(394)] Failed to connect to the bus: Could not parse server address: Unknown address type (examples of valid types are "tcp" and on UNIX "unix")
[7734:7734:0525/181643.316481:WARNING:password_store_factory.cc(250)] Using basic (unencrypted) store for password storage. See https://chromium.googlesource.com/chromium/src/+/master/docs/linux_password_storage.md for more information about password storage options.
(browser_tests:7734): LIBDBUSMENU-GLIB-WARNING **: Unable to get session bus: Unknown or unsupported transport 'disabled' for address 'disabled:'
[7944:8062:0525/181644.519744:ERROR:network_service_test_helper.cc(74)] Intentionally terminating current process to simulate NetworkService crash for testing.
==8094==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1b90a45f in sandbox::syscall_broker::BrokerFilePermission::ValidatePath(char const*) ./../../sandbox/linux/syscall_broker/broker_file_permission.cc:30:7
    #1 0x1b90b2b5 in sandbox::syscall_broker::BrokerFilePermission::CheckStat(char const*, char const**) const ./../../sandbox/linux/syscall_broker/broker_file_permission.cc:200:8
    #2 0x1b915735 in sandbox::syscall_broker::BrokerPermissionList::GetFileNameIfAllowedToStat(char const*, char const**) const ./../../sandbox/linux/syscall_broker/broker_permission_list.cc:113:31
    #3 0x1b9148ca in sandbox::syscall_broker::CommandStatIsSafe(std::__1::bitset<10ul> const&, sandbox::syscall_broker::BrokerPermissionList const&, char const*, char const**) ./../../sandbox/linux/syscall_broker/broker_command.cc:85:17
    #4 0x1b918468 in sandbox::syscall_broker::BrokerClient::Stat(char const*, bool, stat*) const ./../../sandbox/linux/syscall_broker/broker_client.cc:169:8
    #5 0x1b90da74 in Stat ./../../sandbox/linux/syscall_broker/broker_process.cc:144:26
    #6 0x1b90da74 in sandbox::syscall_broker::BrokerProcess::SIGSYS_Handler(sandbox::arch_seccomp_data const&, void*) ./../../sandbox/linux/syscall_broker/broker_process.cc:261:0
    #7 0x1b94f29a in sandbox::Trap::SigSys(int, siginfo_t*, ucontext_t*) ./../../sandbox/linux/seccomp-bpf/trap.cc:244:10
    #8 0x2a0579b in SignalAction(int, void*, void*) /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/msan/msan_interceptors.cc:1008:3
    #9 0x7fce8aa4b32f in _L_unlock_13 ??:?
    #10 0x7fce8aa4b32f in ?? ??:0
    #11 0x7fce819c4d94 in __lxstat /build/eglibc-ripdx6/eglibc-2.19/io/../sysdeps/unix/sysv/linux/wordsize-64/lxstat.c:35:0
    #12 0x7fce8191c897 in realpath /build/eglibc-ripdx6/eglibc-2.19/stdlib/canonicalize.c:161:0
    #13 0x29db571 in __interceptor_realpath /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:3509:15
    #14 0x12ee8687 in base::MakeAbsoluteFilePath(base::FilePath const&) ./../../base/files/file_util_posix.cc:369:7
    #15 0x1590fdec in net::URLRequestFileJob::FetchMetaInfo(base::FilePath const&, net::URLRequestFileJob::FileMetaInfo*) ./../../net/url_request/url_request_file_job.cc:214:30
    #16 0x12de0ebc in Run ./../../base/callback.h:96:12
    #17 0x12de0ebc in base::(anonymous namespace)::PostTaskAndReplyRelay::RunTaskAndPostReply(base::(anonymous namespace)::PostTaskAndReplyRelay) ./../../base/threading/post_task_and_reply_impl.cc:79:0
    #18 0x12de1a48 in Invoke<void (*)(base::(anonymous namespace)::PostTaskAndReplyRelay), base::(anonymous namespace)::PostTaskAndReplyRelay> ./../../base/bind_internal.h:402:12
    #19 0x12de1a48 in MakeItSo<void (*)(base::(anonymous namespace)::PostTaskAndReplyRelay), base::(anonymous namespace)::PostTaskAndReplyRelay> ./../../base/bind_internal.h:547:0
    #20 0x12de1a48 in RunImpl<void (*)(base::(anonymous namespace)::PostTaskAndReplyRelay), std::__1::tuple<base::(anonymous namespace)::PostTaskAndReplyRelay>, 0> ./../../base/bind_internal.h:621:0
    #21 0x12de1a48 in base::internal::Invoker<base::internal::BindState<void (*)(base::(anonymous namespace)::PostTaskAndReplyRelay), base::(anonymous namespace)::PostTaskAndReplyRelay>, void ()>::RunOnce(base::internal::BindStateBase*) ./../../base/bind_internal.h:589:0
    #22 0x12b87b31 in Run ./../../base/callback.h:96:12
    #23 0x12b87b31 in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) ./../../base/debug/task_annotator.cc:101:0
    #24 0x12db1fd0 in base::internal::TaskTracker::RunOrSkipTask(base::internal::Task, base::internal::Sequence*, bool) ./../../base/task_scheduler/task_tracker.cc:531:23
    #25 0x12f1a9f0 in base::internal::TaskTrackerPosix::RunOrSkipTask(base::internal::Task, base::internal::Sequence*, bool) ./../../base/task_scheduler/task_tracker_posix.cc:23:16
    #26 0x12daf3c0 in base::internal::TaskTracker::RunAndPopNextTask(scoped_refptr<base::internal::Sequence>, base::internal::CanScheduleSequenceObserver*) ./../../base/task_scheduler/task_tracker.cc:404:3
    #27 0x12d9eaf4 in base::internal::SchedulerWorker::RunWorker() ./../../base/task_scheduler/scheduler_worker.cc:321:24
    #28 0x12d9e186 in base::internal::SchedulerWorker::RunPooledWorker() ./../../base/task_scheduler/scheduler_worker.cc:215:3
    #29 0x12f1c7e7 in base::(anonymous namespace)::ThreadFunc(void*) ./../../base/threading/platform_thread_posix.cc:76:13
    #30 0x7fce8aa43183 in start_thread /build/eglibc-ripdx6/eglibc-2.19/nptl/pthread_create.c:312:0
    #31 0x7fce819d403c in clone /build/eglibc-ripdx6/eglibc-2.19/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:111:0
  Uninitialized value was created by an allocation of 'full_path' in the stack frame of function '_ZN4base20MakeAbsoluteFilePathERKNS_8FilePathE'
    #0 0x12ee8550 in base::MakeAbsoluteFilePath(base::FilePath const&) ./../../base/files/file_util_posix.cc:366:0
SUMMARY: MemorySanitizer: use-of-uninitialized-value (/b/s/w/ir/out/Release/browser_tests+0x1b90a45f)
Exiting
../../chrome/browser/net/network_context_configuration_browsertest.cc:474: Failure
Value of: simple_loader->ResponseInfo()
  Actual: false
Expected: true
Stack trace:
    #0 0x000008ef0807 in StackTraceGetter::CurrentStackTrace(int, int) ./../../third_party/googletest/custom/gtest/internal/custom/stack_trace_getter.cc:22:27
    #1 0x000008f25329 in testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop(int) ./../../third_party/googletest/src/googletest/src/gtest.cc:810:35
    #2 0x000008f23482 in testing::internal::AssertHelper::operator=(testing::Message const&) const ./../../third_party/googletest/src/googletest/src/gtest.cc:386:25
    #3 0x000003c0ebd5 in (anonymous namespace)::NetworkContextConfigurationBrowserTest_FileURL_Test::RunTestOnMainThread() ./../../chrome/browser/net/network_context_configuration_browsertest.cc:474:3
    #4 0x000014875af2 in content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() ./../../content/public/test/browser_test_base.cc:385:5
    #5 0x00001315b7ed in Run ./../../base/callback.h:125:12
    #6 0x00001315b7ed in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() ./../../chrome/browser/chrome_browser_main.cc:2098:0
    #7 0x000013156910 in ChromeBrowserMainParts::PreMainMessageLoopRun() ./../../chrome/browser/chrome_browser_main.cc:1492:18
    #8 0x00000c01ca4b in content::BrowserMainLoop::PreMainMessageLoopRun() ./../../content/browser/browser_main_loop.cc:960:13
    #9 0x00000d61d0e1 in Run ./../../base/callback.h:125:12
    #10 0x00000d61d0e1 in content::StartupTaskRunner::RunAllTasksNow() ./../../content/browser/startup_task_runner.cc:44:0
    #11 0x00000c014d98 in content::BrowserMainLoop::CreateStartupTasks() ./../../content/browser/browser_main_loop.cc:871:25
    #12 0x00000c029f66 in content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) ./../../content/browser/browser_main_runner_impl.cc:148:15
    #13 0x00000c00962c in content::BrowserMain(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) ./../../content/browser/browser_main.cc:47:20
    #14 0x000012956da5 in content::RunBrowserProcessMain(content::MainFunctionParams const&, content::ContentMainDelegate*, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) ./../../content/app/content_main_runner_impl.cc:620:10
    #15 0x00001295b16a in content::ContentMainRunnerImpl::Run() ./../../content/app/content_main_runner_impl.cc:964:12
    #16 0x00001bad3ad9 in service_manager::Main(service_manager::MainParams const&) ./../../services/service_manager/embedder/main.cc:459:29
    #17 0x0000129510c8 in content::ContentMain(content::ContentMainParams const&) ./../../content/app/content_main.cc:19:10
    #18 0x000014873f20 in content::BrowserTestBase::SetUp() ./../../content/public/test/browser_test_base.cc:323:3
[7734:7734:0525/181646.613623:WARNING:pref_notifier_impl.cc(23)] Pref observer found at shutdown.
[7734:7734:0525/181646.613764:WARNING:pref_notifier_impl.cc(23)] Pref observer found at shutdown.
[  FAILED  ] SafeBrowsingNetworkContext/NetworkContextConfigurationBrowserTest.FileURL/2, where GetParam() = 8-byte object <02-00 00-00 01-00 00-00> (5420 ms)

 
Project Member

Comment 1 by bugdroid1@chromium.org, May 28 2018

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

commit 58a387f3e083504564fbe31b583f1fd9cd4f8964
Author: Trent Apted <tapted@chromium.org>
Date: Mon May 28 01:53:26 2018

Revert "Proxy lstat() system calls to broker process for network service."

This reverts commit 4b0e0023a1190ff6534b514c00b7e9c1d611bb5c.

Reason for revert: Suspect for uninitialized memory access

==8094==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1b90a45f in sandbox::syscall_broker::BrokerFilePermission::ValidatePath(char const*)

and failing NetworkContextConfigurationBrowserTest.FileURL tests since

https://ci.chromium.org/buildbot/chromium.memory/Linux%20MSan%20Tests/10019

Original change's description:
> Proxy lstat() system calls to broker process for network service.
>
> This particular stat() variant was not being handled in the same
> way as the others.
>
> Bug:  846800 
> Change-Id: I2ec9b94f704f9f00aa3dd1dbd0a3be71a0583b75
> Reviewed-on: https://chromium-review.googlesource.com/1073869
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Commit-Queue: Tom Sepez <tsepez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#562045}

TBR=tsepez@chromium.org,rsesek@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  846800 ,  847096 
Change-Id: I49b936a9afc0491fb9217c165c2060b760d7b0ba
Reviewed-on: https://chromium-review.googlesource.com/1074847
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562159}
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_client.cc
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_client.h
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_host.cc
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_process.cc
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_process.h
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/sandbox/linux/syscall_broker/broker_process_unittest.cc
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/services/service_manager/sandbox/linux/bpf_broker_policy_linux.cc
[modify] https://crrev.com/58a387f3e083504564fbe31b583f1fd9cd4f8964/services/service_manager/sandbox/linux/bpf_network_policy_linux.cc

Comment 2 by tsepez@chromium.org, May 29 2018

Blocking: 846800

Comment 3 by tsepez@chromium.org, May 29 2018

Cc: tsepez@chromium.org
Owner: kcc@chromium.org
This appears to be an interaction between the way the path is assembled in /build/eglibc-ripdx6/eglibc-2.19/stdlib/canonicalize.c and msan's taint tracking. Normally, this data would go right to the syscall and be beyond msan's analysis, but our proxying code, built under msan, is now touching this memory.

I'm suprised that the strlen() at
  https://cs.chromium.org/chromium/src/sandbox/linux/syscall_broker/broker_file_permission.cc?rcl=9e3846dbd0cf329d087fa05841f1212089304182&l=25

didn't first fire before the indicated line, but no matter.

stdlib/canonicalize.c isn't in cs.chromium.org so I'm wondering if it is built for msan with its intermediate calculations clearing the msan uninitialized bits.

Comment 4 by tsepez@chromium.org, May 29 2018

Cc: dcheng@chromium.org

Comment 5 by tsepez@chromium.org, May 29 2018

dcheng@ - as a base owner, is it OK to stick a memset(full_path, 0, PATH_MAX) in
https://cs.chromium.org/chromium/src/base/files/file_util_posix.cc?rcl=d77547dbb022b5288ee579cad362ec0dd13d2945&l=369 under an #ifdef MSAN until
this issue is resolved?

Comment 6 by tsepez@chromium.org, May 29 2018

(basically, canonicalize.c is using our caller-provided realpath() result buffer as scratch space for its intermediate calculations which may not be tracked by msan).

Comment 7 by tsepez@chromium.org, May 29 2018

Owner: euge...@chromium.org
Setting better owner, maybe.
Cc: euge...@chromium.org
Owner: tsepez@chromium.org
How interesting!
MSan handles realpath() by wrapping it via symbol interposition and updating shadow memory after the libc function returns. Your syscall filter interfered after the new path was constructed but before MSan had a chance to update shadow.

You'll need to do MSAN_UNPOISON before touching this buffer.

Project Member

Comment 9 by bugdroid1@chromium.org, May 30 2018

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

commit c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90
Author: Tom Sepez <tsepez@chromium.org>
Date: Wed May 30 17:29:14 2018

Reland "Proxy lstat() system calls to broker process for network service."

This reverts commit 58a387f3e083504564fbe31b583f1fd9cd4f8964.

The original patch was reverted due to a spurious msan warning. This
version unpoisons the buffer per discussion on  bug 847096 .

Bug:  847096 , 846800 
Change-Id: Ibd82c5c6d46521325ff81146a8f000deff05148c
Reviewed-on: https://chromium-review.googlesource.com/1077432
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562889}
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_client.cc
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_client.h
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_host.cc
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_process.cc
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_process.h
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/sandbox/linux/syscall_broker/broker_process_unittest.cc
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/services/service_manager/sandbox/linux/bpf_broker_policy_linux.cc
[modify] https://crrev.com/c3a7aeaf7d4ea5f0cef6eeb71c22a2c835a4cb90/services/service_manager/sandbox/linux/bpf_network_policy_linux.cc

Status: Fixed (was: Assigned)

Sign in to add a comment