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

Issue 772503 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Linux sandbox: Allow localtime*_override to use UTC timezone for profiling service.

Project Member Reported by erikc...@chromium.org, Oct 6 2017

Issue description

The Linux sandbox overrides localtime to use a synchronous IPC to the browser process.
https://cs.chromium.org/chromium/src/content/zygote/zygote_main_linux.cc?q=localtime_override&sq=package:chromium&l=263

This was added to allow renderers to correctly do time conversions.
"""
    73fa6399bd9198863d7d9e01db6fcc8cdfcc8837
...
    Linux sandbox: plumb timezone calls through the sandbox
    
    The localtime (and localtime_r) functions try to access /etc/localtime
    in the filesystem. For sandboxed renderers, this fails, the the
    functions default to a UTC timezone.

    These functions are called from within WebCore and V8 and there's no
    clean way to patch the source code in place to do a hairpin turn and
    manage an IPC. Additionally, we cannot overwrite the calls with the
    usual symbol resolution procedures since the same chrome binary must
    serve as both the unsandboxed browser and sandboxed renderer.

"""

For Out of Process Heap Profiling, we spawn a profiling service in a sandboxed utility process. The browser process synchronously writes to an OS pipe [potentially blocking], which the profiling service needs to regularly read from. This currently can cause a deadlock, since the reader thread of the profiling service has LOG statements, which call localtime, which in turn attempts synchronous IPC to the browser process.

I'd like to be able to turn off this localtime*_override functionality and just use UTC for this sandboxed, profiling service. 

If you want more information about the profiling service, please see this preliminary design doc [not ready for review]: https://docs.google.com/document/d/1qBANWBo9_LXtwLa8xm3GBZ0oBwWbGPu0_keVKGunXaQ/edit#

I'm not too familiar with the Linux sandbox. The easiest solution for me to implement would be a global function exposed by the sandbox which turns off the IPC functionality, which can be called post sandbox initialization. I assume it's also possible to do some type of pre-sandbox initialization, but I'd have to do some digging.

Any concerns/objections? 
 
I'm not really an OWNER of this code but it seems reasonable to have a way to disable the sandbox IPC functionality. How would localtime work in the face of the sandbox though? Would it just report UTC or would it repeatedly try and fail to acquire resources?
That will depend on the implementation of libc. In glibc, the result is cached. See tzset_internal. So there should not be repeated attempts to acquire resources.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 10 2017

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

commit 0ba08cb3faee37bbeed8249f6f29d75145cc3c34
Author: Erik Chen <erikchen@chromium.org>
Date: Tue Oct 10 17:59:34 2017

Create a new sandbox type for the profiling service.

The new "profiling" sandbox type is a variant of the utility sandbox type. The
only difference is that on Linux, localtime will use UTC instead of brokering a
synchronous IPC to the browser process. This is needed to prevent deadlocks
between the browser process and the profiling service, when profiling is
enabled.

For more details, see  Issue 772503 .

Bug:  772503 ,  765414 
Change-Id: Id16171ea26155aed254567c61dbec05f635305a8
Reviewed-on: https://chromium-review.googlesource.com/707918
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507713}
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/chrome/profiling/profiling_manifest.json
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/content/app/DEPS
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/content/app/content_main_runner.cc
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/content/browser/utility_process_host_impl.cc
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/content/common/sandbox_mac.mm
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/content/zygote/zygote_main.h
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/content/zygote/zygote_main_linux.cc
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/services/service_manager/sandbox/sandbox_type.cc
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/services/service_manager/sandbox/sandbox_type.h
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/services/service_manager/sandbox/switches.cc
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/services/service_manager/sandbox/switches.h
[modify] https://crrev.com/0ba08cb3faee37bbeed8249f6f29d75145cc3c34/testing/buildbot/filters/mus.browser_tests.filter

Status: Fixed (was: Assigned)

Sign in to add a comment