New issue
Advanced search Search tips

Issue 741984 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Feature

Blocking:
issue 713768



Sign in to add a comment

Allow syscall mincore in child processes on Linux and Android

Project Member Reported by hajimehoshi@chromium.org, Jul 13 2017

Issue description

We'd like to use base::ProcessMemoryDump::CountResidentBytes to know resident size of base::SharedMemory not only in browser process but also renderer or gpu processes (crbug/713768). This function calls mincore sycall on POSIX, and this is now forbidden by the sandbox. Let's allow this syscall on child processes.

Note: mincore is already permitted on x86 Android (https://cs.chromium.org/chromium/src/sandbox/linux/seccomp-bpf-helpers/baseline_policy_android.cc?sq=package:chromium&dr&l=91) and GPU processes (sometimes) (https://cs.chromium.org/chromium/src/content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc?q=mincore&sq=package:chromium&l=174&dr=C).

 
Blocking: 713768
Labels: -Restrict-View-Google
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 18 2017

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

commit 94cba8e27ccc8f6bdab1ec4855a2e30f149b078c
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Tue Jul 18 05:13:46 2017

Linux sandbox: Allow mincore for SharedMemoryTracker

We'd like to use CountResidentBytes in child processes to know resident
size of base::SharedMemory, but this function is currently unavailable
in sandboxed processes since this function uses 'mincore' syscall.

This CL changes the sandbox policy to allow mincore everywhere on Linux
and Android.

Bug:  741984 
Change-Id: Ie3e595c872e2ec52d2b36660cee0a6710012859a
Reviewed-on: https://chromium-review.googlesource.com/566757
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487390}
[modify] https://crrev.com/94cba8e27ccc8f6bdab1ec4855a2e30f149b078c/content/common/sandbox_linux/bpf_gpu_policy_linux.cc
[modify] https://crrev.com/94cba8e27ccc8f6bdab1ec4855a2e30f149b078c/content/common/sandbox_linux/bpf_gpu_policy_linux.h
[modify] https://crrev.com/94cba8e27ccc8f6bdab1ec4855a2e30f149b078c/content/common/sandbox_linux/bpf_renderer_policy_linux.cc
[modify] https://crrev.com/94cba8e27ccc8f6bdab1ec4855a2e30f149b078c/content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc
[modify] https://crrev.com/94cba8e27ccc8f6bdab1ec4855a2e30f149b078c/sandbox/linux/seccomp-bpf-helpers/baseline_policy_android.cc

Status: Fixed (was: Untriaged)
Status: Started (was: Fixed)
Hmm, I tried to call CountResidentBytes in SharedMemoryTracker::OnMemoryDump as experimental, but crashed:

../../sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc:**CRASHING**:seccomp-bpf failure in syscall 0027
Received signal 11 SEGV_MAPERR 00000000001b
#0 0x7f178e360bb7 base::debug::StackTrace::StackTrace()
#1 0x7f178e36068f base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7f178e4e3330 <unknown>
#3 0x7f1781e55246 sandbox::CrashSIGSYS_Handler()
#4 0x7f1781e59988 sandbox::Trap::SigSys()
#5 0x7f178e4e3330 <unknown>
#6 0x7f178495b617 mincore
#7 0x7f178e4189de base::trace_event::ProcessMemoryDump::CountResidentBytes()
#8 0x7f178e3903ae base::SharedMemoryTracker::OnMemoryDump()
#9 0x7f178e413ea1 base::trace_event::MemoryDumpManager::InvokeOnMemoryDump()
#10 0x7f178e4135cf base::trace_event::MemoryDumpManager::SetupNextMemoryDump()
#11 0x7f178e413dd1 base::trace_event::MemoryDumpManager::InvokeOnMemoryDump()
#12 0x7f178e3613fb base::debug::TaskAnnotator::RunTask()
#13 0x7f178e3940aa base::MessageLoop::RunTask()
#14 0x7f178e3943e2 base::MessageLoop::DeferOrRunPendingTask()
#15 0x7f178e394744 base::MessageLoop::DoWork()
#16 0x7f178e395ff9 base::MessagePumpDefault::Run()
#17 0x7f178e393c6f base::MessageLoop::Run()
#18 0x7f178e3c7c37 base::RunLoop::Run()
#19 0x7f178e402f0c base::Thread::Run()
#20 0x7f178e4034af base::Thread::ThreadMain()
#21 0x7f178e3fa6bc base::(anonymous namespace)::ThreadFunc()
#22 0x7f178e4db184 start_thread
#23 0x7f1784960ffd clone
r8: 0000000000000000  r9: 0000000000000001 r10: 000000000000000b r11: 0000000000000000
r12: 000000000000001b r13: 0000000000000001 r14: 00007f17743861c8 r15: 00007f1781e5ace9
di: 00007f1774386040  si: 00007f1774386030  bp: 0000000000000000  bx: 0000000000000000
dx: 000000000000001b  ax: 0000000000000000  cx: 000000000000001b  sp: 00007f1774386170
ip: 00007f1781e55246 efl: 0000000000010206 cgf: 0000000000000033 erf: 0000000000000006
trp: 000000000000000e msk: 0000000000000000 cr2: 000000000000001b
[end of stack trace]


It looks like renderer or gpu processes didn't die. I'm now investigating what process is involved in this stacktrace.
Dumped the command line just before crashing:

/proc/self/exe --type=utility --field-trial-handle=10930977584389936282,7899221915136510228,131072 --lang=en-US --service-request-channel-token=721E444B8A9B483C1F63A3B1D35B0083 --shared-files=v8_natives_data:100,v8_snapshot_data:101


It look like that was a utility process. I was wondering if it is possible to allow mincore in all child processes other than renderer or gpu.
I'm surprised we're going out to an utility process for this. Can we just add to the utility process policy?
> I'm surprised we're going out to an utility process for this. Can we just add to the utility process policy?

Our SharedMemoryTracker is used in all processes using SharedMemory. I'm not sure there are any other child processes I don't know. I didn't know even the existence of 'utility' process.
Aaah I see what you mean. If we want to use the SharedMemoryTracker everywhere, then we might consider it adding it as an exception in the baseline policy, without adding it to any existing allowed syscall groups.
Right! So I was wondering whether you think it is OK to allow 'mincore' in baseline policy, or fixing 'utility' process policy is enough. I feel like the latter would fix the current situation but might cause the same problem in the future.
I have created a CL https://chromium-review.googlesource.com/c/578769/, but I'd like to reach agreements before getting reviewed.
I'd rather not add mincore to any of the sets of allowed syscalls. I'd be OK to add it individually to the baseline policies though.
Aha, thanks. I've fixed the CL https://chromium-review.googlesource.com/c/578769/ to fix only baseline policies. baseline_policy_android.cc has been fixed in the previous CL, so I think fixing baseline_policy.cc is enough.
Project Member

Comment 15 by bugdroid1@chromium.org, Jul 25 2017

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

commit ce020ebf0ea534d9ea726ab50d370d81a7dd7b1d
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Tue Jul 25 03:45:33 2017

Linux sandbox: Allow mincore in baseline policy

The syscall 'mincore' will be used SharedMemoryTracker::OnMemoryDump
( crbug.com/713768 ) in all processes using SharedMemory. In the current
situation, renderer and gpu processes are allowed to use mincore but not
other child processes like utility. Actually utility processe uses
SharedMemory so the process crashes when 'mincore' is called. Other than
utility, such other process might appear in the future.

This CL fixes the baseline policy to allow 'mincore' in all sandbox-ed
processes.

Bug:  741984 
Change-Id: I639c729b50d7326857d04e4dbcfaaa3dd2f20120
Reviewed-on: https://chromium-review.googlesource.com/578769
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489218}
[modify] https://crrev.com/ce020ebf0ea534d9ea726ab50d370d81a7dd7b1d/content/common/sandbox_linux/bpf_gpu_policy_linux.cc
[modify] https://crrev.com/ce020ebf0ea534d9ea726ab50d370d81a7dd7b1d/content/common/sandbox_linux/bpf_renderer_policy_linux.cc
[modify] https://crrev.com/ce020ebf0ea534d9ea726ab50d370d81a7dd7b1d/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc
[modify] https://crrev.com/ce020ebf0ea534d9ea726ab50d370d81a7dd7b1d/sandbox/linux/seccomp-bpf-helpers/baseline_policy_android.cc

Status: Fixed (was: Started)

Sign in to add a comment