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

Issue 152530 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2013
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Seccomp sandbox fails at initialization while setting-up probe processes (from the GPU process)

Project Member Reported by thestig@chromium.org, Sep 26 2012

Issue description

https://go/crash/reportdetail?reportid=cb8ac36d731fd6dc

Product, Version: Chrome_Linux ,  22.0.1229.79
Kernel Version: Linux 3.2.0-29-generic #46
CPU Architecture: amd64
ptype: gpu-process
lsb-release: Ubuntu 12.04.1 LTS

Thread 0 *CRASHED* ( SIGABRT @ 0x3e8000048de )

0x7ffd4f26f445	 [libc-2.15.so]	 - ../nptl/sysdeps/unix/sysv/linux/raise.c:64]	raise
0x7ffd4f272baa	 [libc-2.15.so]	 - abort.c:91]	abort
0x7ffd55e66288	 [chrome]	 - base/debug/debugger_posix.cc:230]	base::debug::BreakDebugger
0x7ffd55e7bc24	 [chrome]	 - base/logging.cc:663]	logging::LogMessage::~LogMessage
0x7ffd56522137	 [chrome]	 - ./sandbox/linux/seccomp-bpf/sandbox_bpf.h:294]	playground2::Sandbox::die
0x7ffd57a83263	 [chrome]	 - sandbox/linux/seccomp-bpf/sandbox_bpf.cc:129]	playground2::Sandbox::RunFunctionInPolicy
0x7ffd57a83457	 [chrome]	 - sandbox/linux/seccomp-bpf/sandbox_bpf.cc:143]	playground2::Sandbox::kernelSupportSeccompBPF
0x7ffd57a834b4	 [chrome]	 - sandbox/linux/seccomp-bpf/sandbox_bpf.cc:179]	playground2::Sandbox::supportsSeccompSandbox
0x7ffd5652233d	 [chrome]	 - content/common/sandbox_seccomp_bpf_linux.cc:526]	content::SandboxSeccompBpf::SupportsSandbox
0x7ffd56521a44	 [chrome]	 - content/common/sandbox_linux.cc:117]	content::LinuxSandbox::PreinitializeSandboxBegin
0x7ffd56521be8	 [chrome]	 - content/common/sandbox_linux.cc:145]	content::LinuxSandbox::PreinitializeSandbox
0x7ffd56521cac	 [chrome]	 - content/common/sandbox_linux.cc:194]	content::LinuxSandbox::StartSeccompLegacy
0x7ffd5652124a	 [chrome]	 - content/common/sandbox_init_linux.cc:37]	content::InitializeSandbox
0x7ffd55e0e2b7	 [chrome]	 - content/gpu/gpu_main.cc:161]	GpuMain
 
Labels: Stability-Crash
FWIW, every affected user seems to have the nvidia closed-source driver installed. Most commonly 295.49 and 304.51.

Comment 2 by jln@chromium.org, Oct 11 2012

Cc: jorgelo@chromium.org
 Issue 155182  has been merged into this issue.
 Issue 155182  has been merged into this issue.

Comment 4 Deleted

Comment 5 by jln@chromium.org, Oct 11 2012

We have code that tests seccomp BPF support by creating a sandboxed process.

This code can fail in two ways:

1. The process crashes or otherwise exits abnormally but nothing is written to stderr
2. The process crashes or otherwise exits abnormally but something is written to stderr

(2) will lead to a SANDBOX_DIE that will kill the process that called playground2::Sandbox::supportsSeccompSandbox().

I've argued before that this is quite fragile as glibc or something else could write to stderr because of some environment variables or something else.

One possible failure (that writes to stderr, case (2) above) would be that the process is multi threaded. However, this should be cought before, in InitializeSandbox().

Since the error message printed to stderr should now be on the stack (read to "buf" from the pipe), it might be in the minidump, I'll take a look.

But I would suggest in any case that we remove "dup2(fds[1], 2)" in the child in Sandbox::RunFunctionInPolicy on non DEBUG build. This brings little value and is way too fragile in my opinion.

Comment 6 by jln@chromium.org, Oct 11 2012

Owner: jln@chromium.org
Status: Assigned
There is "[18788:18788:1009/200638:FATAL:sandbox_bpf.cc(134)] Failed to set up stderr" on the stack.

That's puzzling. It could be a signal, even though that seems unlikely. Depending on what the current stderr was, it could be possible that dup2(fds[1], 2) would fail while closing stderr. 
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 11 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=161443

------------------------------------------------------------------------
r161443 | jln@chromium.org | 2012-10-11T23:27:29.467205Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/seccomp-bpf/sandbox_bpf.cc?r1=161443&r2=161442&pathrev=161443

Seccomp BPF: handle EINTR in error reporting setup.

Wrap dup2 with HANDLE_EINTR in the error reporting set-up for the BPF
support detection process.

We also print errno as an attempt to obtain more information on this puzzling
bug.

BUG= 152530 


Review URL: https://chromiumcodereview.appspot.com/11103021
------------------------------------------------------------------------

Comment 8 by jln@chromium.org, Oct 16 2012

Cc: kbr@chromium.org cevans@chromium.org
 Issue 155751  has been merged into this issue.

Comment 9 by jln@chromium.org, Oct 16 2012

Summary: Seccomp sandbox fails at initialization while setting-up probe processes (from the GPU process)

Comment 10 by jln@chromium.org, Oct 16 2012

I'm worried about the rate of errors here. I've spent a lot of time scratching my head and I can't imagine what could happen, other than a dup2 EINTR.

We'll see if the r161443 fixes it with the roll-out of the next dev channel.

Comment 11 Deleted

Comment 12 Deleted

Comment 13 by jln@chromium.org, Oct 18 2012

I finally have a theory to explain this bug.

While starting Chrome from a windows manager, stderr can be something such as $HOME/.xession-errors, and dup2 closes it.

This file could be on all kind of filesystems, including NFS or FUSE. Some modern distributions use ecryptfs for their users' home directories.

Looking at the later for instance, one can see that its flush member in the file_operations structure (called on close()) will call Linux' class path: filemap_write_and_wait()--filemap_fdatawrite()--do_writepages(), which will end-up calling ecryptfs_writepage() and then ecryptfs_encrypt_page.

Of course there are plenty of reasons for such an operation to fail.

Let's see what the crash reports say after a few days of dev channel, if EINTR doesn't solve it in practice, we can just rely on the exit code of the probe process and disable reading the pipe to generate a fatal error when dup2 fails.
Or we could even remain this altogether in non DEBUG builds, because realistically, there isn't anything the user would see or do about it.

Comment 14 Deleted

Comment 15 by jln@chromium.org, Oct 18 2012

Still no crash on the new dev channel. I would like to tentatively merge r161443, which is very simple, Linux only.

Comment 16 by kareng@google.com, Oct 18 2012

Labels: -Merge-Requested Merge-Approved
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 18 2012

Labels: -Merge-Approved merge-merged-1271
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=162734

------------------------------------------------------------------------
r162734 | jln@chromium.org | 2012-10-18T17:50:20.082526Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1271/src/sandbox/linux/seccomp-bpf/sandbox_bpf.cc?r1=162734&r2=162733&pathrev=162734

Merge 161443 - Seccomp BPF: handle EINTR in error reporting setup.

Wrap dup2 with HANDLE_EINTR in the error reporting set-up for the BPF
support detection process.

We also print errno as an attempt to obtain more information on this puzzling
bug.

BUG= 152530 


Review URL: https://chromiumcodereview.appspot.com/11103021

TBR=jln@chromium.org
Review URL: https://codereview.chromium.org/11203003
------------------------------------------------------------------------

Comment 18 by jln@chromium.org, Oct 26 2012

Labels: -Pri-2 Pri-1 ReleaseBlock-Stable
The new beta came out (23.0.1271.52) and we're still saying this error at a high rate.

I've looked at a 7 crash dumps and all but one showed that close() or dup2() was failing with EBADF.

Comment 19 by kareng@google.com, Oct 26 2012

so we can revert this and go back to where we were until we have a real fix, do you want me to revert it? i won't blok stable on this but i am ok taking a fix once we're sure it's fixed.

Comment 20 by kareng@google.com, Oct 26 2012

Labels: -ReleaseBlock-Stable

Comment 21 by jln@chromium.org, Oct 26 2012

This patch still makes things better, and we at least get an error message to help debug: let's keep it.

I'm going to disable FATAL failures on dup2 as the better fix.

Comment 22 by kareng@google.com, Oct 29 2012

Labels: -Mstone-23 MovedFrom-23 Mstone-24
moving these to m24. m23 has passed. 
Project Member

Comment 23 by bugdroid1@chromium.org, Oct 30 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=164850

------------------------------------------------------------------------
r164850 | jln@chromium.org | 2012-10-30T05:01:24.870889Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/seccomp-bpf/sandbox_bpf.cc?r1=164850&r2=164849&pathrev=164850

Seccomp-BPF: relax failure in probe process setup

When we set-up the probe process to test seccomp-bpf availability, setting
a pipe on stderr can sometimes fail. Presumably if this descriptor is backed
by a file on a file system that will return an error on close().

We don't consider not being able to set-up the pipe on stderr as a fatal error
anymore.

BUG= 152530 
NOTRY=true

Review URL: https://chromiumcodereview.appspot.com/11300014
------------------------------------------------------------------------
Status: Fixed

Comment 25 by jln@chromium.org, Nov 1 2012

I really do hope it's fixed (and I'm checking every day for reports), but we have so few users on Linux that I don't think the current dev channel is of any indication.

After we landed r161443, I also thought it would be fixed, we didn't get dev channel crashes for a long time.

But this new fix is pretty conservative, so I'm definitely hopeful that we are in a good shape now. We'll want to merge this back to 23, should I way another week before requesting the merge?
Status: Assigned
sounds good to me! I'll let you close the bug when you feel comfortable.
Labels: -Mstone-24
Please target the right milestone to which this can be fixed. For now, removing the milestone label.

Comment 28 by jln@chromium.org, Nov 5 2012

Labels: Merge-Requested Mstone-23 m
Looks fixed, I would like to merge to 24 and 23. This only affects Linux-only code.
jln@: there isn't anything to merge in M24. All your changes are in for M24.

Comment 30 by kareng@google.com, Nov 5 2012

let's let it go to m24 first. m23 is going to stable. 

Comment 31 by jln@chromium.org, Nov 5 2012

dharani: ohh yes of course, sorry, I got confused. Checking 24 crashes is actually why I say it's fixed in the first place.

kareng: r164850 has been on dev channel for a week now. Do you want some beta time before merging to 23?

Comment 32 by kareng@google.com, Nov 5 2012

we have a bunch of time before stable 2 so we can give it a bit longer.

Comment 33 by kareng@google.com, Nov 9 2012

Labels: -Merge-Requested Merge-Approved
Project Member

Comment 34 by bugdroid1@chromium.org, Nov 9 2012

Labels: -Merge-Approved
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=166990

------------------------------------------------------------------------
r166990 | jln@chromium.org | 2012-11-09T22:08:06.244665Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1271/src/sandbox/linux/seccomp-bpf/sandbox_bpf.cc?r1=166990&r2=166989&pathrev=166990

Merge 164850 - Seccomp-BPF: relax failure in probe process setup

When we set-up the probe process to test seccomp-bpf availability, setting
a pipe on stderr can sometimes fail. Presumably if this descriptor is backed
by a file on a file system that will return an error on close().

We don't consider not being able to set-up the pipe on stderr as a fatal error
anymore.

BUG= 152530 
NOTRY=true

Review URL: https://chromiumcodereview.appspot.com/11300014

TBR=jln@chromium.org
Review URL: https://codereview.chromium.org/11369167
------------------------------------------------------------------------

Comment 35 by jln@chromium.org, Nov 27 2012

Labels: -m
Somewhat still happening in 23.0.1271.91

I see this in crashdumps: "FATAL:sandbox_bpf.cc(167)] Failed to set up stderr: Bad file descriptor"

We do:

pipe2(fds, O_NONBLOCK|O_CLOEXEC);

Then we fork().

In the child we do HANDLE_EINTR(close(fds[0]), HANDLE_EINTR(dup2(fds[1], 2), HANDLE_EINTR(close(fds[1]).

One of the two close() fails.
Project Member

Comment 36 by bugdroid1@chromium.org, Dec 5 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=171351

------------------------------------------------------------------------
r171351 | jln@chromium.org | 2012-12-05T23:27:21.967351Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/seccomp-bpf/sandbox_bpf.cc?r1=171351&r2=171350&pathrev=171351

sandbox-bpf: more robust probe process in release mode

In release mode, we don't fail on probe process failing on close() as an
attempt to circumvent a very puzzling bug.

BUG= 152530 


Review URL: https://chromiumcodereview.appspot.com/11446011
------------------------------------------------------------------------

Comment 37 by jln@chromium.org, Dec 17 2012

Labels: Merge-Requested
I would like to merge this to 24 and 23. No Crash reports since this has landed on Dev (25.0.1354.0).
Labels: -Mstone-23 -Merge-Requested Mstone-24 Merge-Approved
Merge approve for M24 branch 1312.
Project Member

Comment 39 by bugdroid1@chromium.org, Dec 17 2012

Labels: -Merge-Approved merge-merged-1312
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=173536

------------------------------------------------------------------------
r173536 | jln@chromium.org | 2012-12-17T21:41:41.516117Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/sandbox/linux/seccomp-bpf/sandbox_bpf.cc?r1=173536&r2=173535&pathrev=173536

Merge 171351
> sandbox-bpf: more robust probe process in release mode
> 
> In release mode, we don't fail on probe process failing on close() as an
> attempt to circumvent a very puzzling bug.
> 
> BUG= 152530 
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/11446011

TBR=jln@chromium.org
Review URL: https://codereview.chromium.org/11573050
------------------------------------------------------------------------
Project Member

Comment 40 by bugdroid1@chromium.org, Dec 18 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=173606

------------------------------------------------------------------------
r173606 | dharani@google.com | 2012-12-18T00:52:31.319967Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/sandbox/linux/seccomp-bpf/sandbox_bpf.cc?r1=173606&r2=173605&pathrev=173606

Revert 173536
> Merge 171351
> > sandbox-bpf: more robust probe process in release mode
> > 
> > In release mode, we don't fail on probe process failing on close() as an
> > attempt to circumvent a very puzzling bug.
> > 
> > BUG= 152530 
> > 
> > 
> > Review URL: https://chromiumcodereview.appspot.com/11446011
> 
> TBR=jln@chromium.org
> Review URL: https://codereview.chromium.org/11573050

TBR=jln@chromium.org
Review URL: https://codereview.chromium.org/11612010
------------------------------------------------------------------------
Project Member

Comment 41 by bugdroid1@chromium.org, Dec 18 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=173609

------------------------------------------------------------------------
r173609 | jln@chromium.org | 2012-12-18T01:05:26.878141Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/sandbox/linux/seccomp-bpf/sandbox_bpf.cc?r1=173609&r2=173608&pathrev=173609

Merge 171351
> sandbox-bpf: more robust probe process in release mode
> 
> In release mode, we don't fail on probe process failing on close() as an
> attempt to circumvent a very puzzling bug.
> 
> BUG= 152530 
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/11446011

TBR=jln@chromium.org
Review URL: https://codereview.chromium.org/11620010
------------------------------------------------------------------------

Comment 42 by jln@chromium.org, Feb 28 2013

Status: Fixed
Project Member

Comment 43 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-Internals -Internals-Core -Mstone-24 Cr-Internals M-24 Cr-Internals-Core

Sign in to add a comment