Add domain-specific language for BPF policies

Add domain-specific language for BPF policies

This CL adds basic support for equality testing of system call
arguments, and conjunctive and disjunctive combinations of tests.

BUG=375497
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=283350

[mdempsky, 2014-05-20 18:50:19]

As discussed on email, here's my first pass at implementing a DSL for BPF
policies.  Take a look at bpf_dsl_unittest.cc to see how policies look like with
the DSL (basically the same as sketched in email).

For ease of implementation I globbed everything into bpf_dsl.h and bpf_dsl.cc,
but I can split that apart if preferred.

[mdempsky, 2014-05-20 22:21:28]

Julien pointed out we don't actually have that many policies currently, so I
went ahead and prepared https://codereview.chromium.org/299683004/ to show what
it would look like to convert all existing policies to use this DSL.  It
required just two further extensions to the DSL provided by this CL (doing
equality tests on masked syscalls, and some extra support for ElseIf() syntax),
and all sandbox_linux_unittests and nacl_loader_unittests tests pass.

[mdempsky, 2014-05-20 23:26:29]

https://codereview.chromium.org/299743002/diff/60001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h (right):

https://codereview.chromium.org/299743002/diff/60001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:21: class CondImpl : public
base::RefCounted<CondImpl> {
I really want to change this to

    class CondImpl;
    typedef scoped_refptr<const CondImpl> Cond;
    extern template class scoped_refptr<const CondImpl>;

so I can keep CondImpl opaque to users, but Clang then complains that CondImpl
is an incomplete type in bpf_dsl_unittest.cc:

In file included from
../../sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc:5:
In file included from ../../sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:11:
../../base/memory/ref_counted.h:289:11: error: member access into incomplete
type 'const sandbox::bpfdsl::CondImpl'
      ptr_->Release();
          ^
../../sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc:26:15: note: in
instantiation of member function 'scoped_refptr<const
sandbox::bpfdsl::CondImpl>::~scoped_refptr' requested here
          .If(pid == 0).Then(
              ^
../../sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:23:7: note: forward
declaration of 'sandbox::bpfdsl::CondImpl'
class CondImpl;
      ^

I would think the "extern template" would let the compiler just trust there will
be an appropriate instantiation for scoped_refptr<const CondImpl> in another
translation unit, at least in this case because scoped_refptr doesn't have any
partial specializations that could cause ambiguity.

Is there a way to workaround this?  Is this any better in C++11?

[mdempsky, 2014-05-20 23:35:05]

On 2014/05/20 23:26:29, mdempsky wrote:
> Is there a way to workaround this?  Is this any better in C++11?

Oh, gar, "extern template" is a new C++11 feature anyway...

[mdempsky, 2014-06-02 21:55:37]

jln/jyasskin: FYI, I've largely overhauled the DSL and I think it's much cleaner
than before.  I included a lot more documentation, and I have test cases
covering all of the current functionality.

[mdempsky, 2014-06-05 17:41:49]

I'm toying with the idea of replacing

    If(condition).Then(
      result
    ).ElseIf(condition2).Then(
      result2
    ).Else(
      result3
    );

with

    IfThen(condition, result)
        .ElseIfThen(condition2, result2)
        .Else(result3);

or maybe just

    If(condition, result)
        .ElseIf(condition2, result2)
        .Else(result3);

The main benefit of the latter being that it plays more nicely with
clang-format's handling of builder expressions, even if its not quite as nice to
read as the former.  (It would also be a bit simpler to implement, but not
significantly.)

[jln (very slow on Chromium), 2014-06-24 00:03:52]

High level review: this looks great. I still need to look a bit deeper, but I
wanted to send some early feedback.

- Let's go for something like:
 If(condition, result)
        .ElseIf(condition2, result2)
        .Else(result3);

as you suggested to reduce clutter and so that clang-format works better (I'm
definitely open to more suggestions).

- I wonder if we should hide more stuff from the main header file. In theory
things such as BoolExprImpl are actually really part of the main interface that
will be used, but I'm practice I'm not sure users of this class would read it.

Maybe let's keep as-is but have the must-read interfaces at the top?

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/sa...
File sandbox/linux/sandbox_linux.gypi (right):

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/sa...
sandbox/linux/sandbox_linux.gypi:161: 'seccomp-bpf-helpers/bpf_dsl.cc',
Add to gn file as well. (Bots would complain automatically once it'll be used
from outside code)

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/sa...
File sandbox/linux/sandbox_linux_test_sources.gypi (right):

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/sa...
sandbox/linux/sandbox_linux_test_sources.gypi:35:
'seccomp-bpf-helpers/bpf_dsl_unittest.cc',
Don't forget to add this to the .gn file as well. (Bots wouldn't detect that)

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/se...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc (right):

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:37: ErrorResultExprImpl(int err) :
err_(err) {
Nit: explicit

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/se...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h (right):

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:50: //          }
nit: else {} so that the compiler checks that the if() branch always returns for
us?

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:178: // Helper class to make writing
policies easier.
Let's have this earlier in the file as it is one of the few things that users
will actually read.

[mdempsky, 2014-06-24 00:55:40]

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/sa...
File sandbox/linux/sandbox_linux.gypi (right):

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/sa...
sandbox/linux/sandbox_linux.gypi:161: 'seccomp-bpf-helpers/bpf_dsl.cc',
On 2014/06/24 00:03:52, jln wrote:
> Add to gn file as well. (Bots would complain automatically once it'll be used
> from outside code)

Done.

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/sa...
File sandbox/linux/sandbox_linux_test_sources.gypi (right):

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/sa...
sandbox/linux/sandbox_linux_test_sources.gypi:35:
'seccomp-bpf-helpers/bpf_dsl_unittest.cc',
On 2014/06/24 00:03:52, jln wrote:
> Don't forget to add this to the .gn file as well. (Bots wouldn't detect that)

Done.

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/se...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc (right):

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:37: ErrorResultExprImpl(int err) :
err_(err) {
On 2014/06/24 00:03:52, jln wrote:
> Nit: explicit

Done.

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/se...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h (right):

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:50: //          }
On 2014/06/24 00:03:52, jln wrote:
> nit: else {} so that the compiler checks that the if() branch always returns
for
> us?

Done.

https://chromiumcodereview.appspot.com/299743002/diff/160001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:178: // Helper class to make writing
policies easier.
On 2014/06/24 00:03:52, jln wrote:
> Let's have this earlier in the file as it is one of the few things that users
> will actually read.

Done.

[jln (very slow on Chromium), 2014-06-24 23:51:31]

Sending a few comments that are ready.

On top of that, as mentioned in person, I find that the grammar is generally
expressed beautifully, except for "struct IfThen".

"struct IfThen" conflates representing the (BoolExpr, ResultExpr) couple with
the fact that we want to be able to build lists of these.

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc (right):

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:158: struct IfThen : public
base::RefCounted<IfThen> {
This should really be a class.

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:165: static IfThenList
Cons(BoolExpr cond, ResultExpr then, IfThenList rest) {
It's a nit, but this confused me for a bit. Is this helper not better outside of
the class?

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:170: IfThen(BoolExpr cond_,
ResultExpr then_, IfThenList rest_)
Constructor arguments should not have a trailing _. Member variables should.

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:178: CHECK(num >= 0 && num < 6);
We'll need to extend the number of syscalls soon FYI.

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:184: (size <= 4) ?
ErrorCode::TP_32BIT : ErrorCode::TP_64BIT;
TP_32BIT vs TP_64BIT is one of the most hairy parts of how we currently express
policies.

When writing a whitelist logic, we want to use TP_64BIT (even for 32 bits value)
as it's more restrictive.

TP_32BIT means that having the 33MSb set would essentially be ignored. I'm a
little concerned that it could be exploited if the type T for Arg<T> is 32 bits
but the kernel is using a 64 bits type.

Do you think we should ignore this?

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:199: NOTREACHED() << "Unimplemented
ArgEq case";
I wonder if we shouldn't CHECK(false) here. This could be pretty catastrophic if
it went through.

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h (right):

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:71: 
It could make sense to fw declare the sandbox classes here.

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:92: SandboxBPFDSLPolicy() :
SandboxBPFPolicy() {}
You need a virtual destructor as per the style guide. It's too error-prone to
not have one, even though here you didn't add any new members and the class is
abstract.

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:109: static ResultExpr
Trap(::sandbox::Trap::TrapFnc trap_func, void* aux);
Should this be private?

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:134: // argument, which is of type
|T|.
It's worth mentioning explicitly that it starts counting at 0.

[jln (very slow on Chromium), 2014-06-25 00:14:54]

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc (right):

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc:38: BPF_TEST_C(BPFDSL,
Basic, BasicPolicy) {
Let's set errno to 0 first.

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc:73: BPF_TEST_C(BPFDSL,
BooleanLogic, BooleanLogicPolicy) {
errno = 0;

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc:112: 
Let's add a Size test for a 32 bits argument like an int now.

On 64 bits machine, we'll run the risk that 0xFFFFFFFF00000000 | value will be
accepted as well if value has its MSb set.

Maybe this is acceptable, but we should document this in a test and also test
that slight variations such as 0xF7FFFFFF00000000 | value are not accepted.

(This is tested in the main BPF compiler already, but probably worth re-doing
quickly here).

https://chromiumcodereview.appspot.com/299743002/diff/180001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc:198: 
Currently this doesn't test the order if the two "ElseIf()" in the policy.

Maybe add a test for 0x10000 and test that it does EINVAL rather than EEXIST?

[mdempsky, 2014-06-25 23:50:22]

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc (right):

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:158: struct IfThen : public
base::RefCounted<IfThen> {
On 2014/06/24 23:51:31, jln wrote:
> This should really be a class.

Done.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:165: static IfThenList
Cons(BoolExpr cond, ResultExpr then, IfThenList rest) {
On 2014/06/24 23:51:31, jln wrote:
> It's a nit, but this confused me for a bit. Is this helper not better outside
of
> the class?

Done.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:170: IfThen(BoolExpr cond_,
ResultExpr then_, IfThenList rest_)
On 2014/06/24 23:51:31, jln wrote:
> Constructor arguments should not have a trailing _. Member variables should.

Done.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:178: CHECK(num >= 0 && num < 6);
On 2014/06/24 23:51:31, jln wrote:
> We'll need to extend the number of syscalls soon FYI.

Acknowledged.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:184: (size <= 4) ?
ErrorCode::TP_32BIT : ErrorCode::TP_64BIT;
On 2014/06/24 23:51:31, jln wrote:
> TP_32BIT vs TP_64BIT is one of the most hairy parts of how we currently
express
> policies.
> 
> When writing a whitelist logic, we want to use TP_64BIT (even for 32 bits
value)
> as it's more restrictive.
> 
> TP_32BIT means that having the 33MSb set would essentially be ignored. I'm a
> little concerned that it could be exploited if the type T for Arg<T> is 32
bits
> but the kernel is using a 64 bits type.
> 
> Do you think we should ignore this?

Hm, I'll think about it some more.  I've put a TODO for now.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.cc:199: NOTREACHED() << "Unimplemented
ArgEq case";
On 2014/06/24 23:51:31, jln wrote:
> I wonder if we shouldn't CHECK(false) here. This could be pretty catastrophic
if
> it went through.

Done, though if we reach here, we'd later null pointer dereference anyway.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h (right):

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:71: 
On 2014/06/24 23:51:31, jln wrote:
> It could make sense to fw declare the sandbox classes here.

I played around with that, but then it felt too much like they were just forward
declarations for classes defined below, whereas they're actually classes from
other headers.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:92: SandboxBPFDSLPolicy() :
SandboxBPFPolicy() {}
On 2014/06/24 23:51:31, jln wrote:
> You need a virtual destructor as per the style guide. It's too error-prone to
> not have one, even though here you didn't add any new members and the class is
> abstract.

Done.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:109: static ResultExpr
Trap(::sandbox::Trap::TrapFnc trap_func, void* aux);
On 2014/06/24 23:51:31, jln wrote:
> Should this be private?

No, it's a helper method for child classes to use.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl.h:134: // argument, which is of type
|T|.
On 2014/06/24 23:51:31, jln wrote:
> It's worth mentioning explicitly that it starts counting at 0.

Done.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc (right):

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc:38: BPF_TEST_C(BPFDSL,
Basic, BasicPolicy) {
On 2014/06/25 00:14:53, jln wrote:
> Let's set errno to 0 first.

Done.  I added a helper macro that clears errno and updated the unit tests to
consistently use it.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc:73: BPF_TEST_C(BPFDSL,
BooleanLogic, BooleanLogicPolicy) {
On 2014/06/25 00:14:54, jln wrote:
> errno = 0;

Done.

https://codereview.chromium.org/299743002/diff/180001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc:198: 
On 2014/06/25 00:14:53, jln wrote:
> Currently this doesn't test the order if the two "ElseIf()" in the policy.
> 
> Maybe add a test for 0x10000 and test that it does EINVAL rather than EEXIST?

I believe the 0x100 test case covered this, but this test case was probably more
confusing than necessary.  I've tried to simplify it a bit so the semantics are
clearer.

[jln (very slow on Chromium), 2014-06-27 01:45:50]

lgtm, but be careful about IA32 compilation.

- I would love if Jeffrey could glance at the operator overloading and check
that there are no dangerous side effects that I missed. But feel free to land
without waiting for that.

- Similarly it could be a good idea to add even more tests to
bpf_dsl_unittest.cc. But this can be done later in another CL, this coverage is
a good start.

https://chromiumcodereview.appspot.com/299743002/diff/240001/sandbox/linux/se...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc (right):

https://chromiumcodereview.appspot.com/299743002/diff/240001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc:58: if (sysno ==
__NR_socketpair) {
x86 32 bits is unhappy with this since it's multiplexed :(

You could still keep this test as-is but exclude 32 bits Intel with #ifdef and
add a small additional test that covers 32 bits Intel.

https://chromiumcodereview.appspot.com/299743002/diff/240001/sandbox/linux/se...
File sandbox/linux/seccomp-bpf-helpers/cons_unittest.cc (right):

https://chromiumcodereview.appspot.com/299743002/diff/240001/sandbox/linux/se...
sandbox/linux/seccomp-bpf-helpers/cons_unittest.cc:23: }  // namespace
It's perfectly ok to have TEST() in the anonymous namespace (and it's often the
case in sandbox/)

[mdempsky, 2014-06-28 00:36:42]

https://codereview.chromium.org/299743002/diff/240001/sandbox/linux/seccomp-b...
File sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc (right):

https://codereview.chromium.org/299743002/diff/240001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/bpf_dsl_unittest.cc:58: if (sysno ==
__NR_socketpair) {
On 2014/06/27 01:45:50, jln (OOO til July 15th) wrote:
> x86 32 bits is unhappy with this since it's multiplexed :(

Ah, right.

> You could still keep this test as-is but exclude 32 bits Intel with #ifdef and
> add a small additional test that covers 32 bits Intel.

Done.

https://codereview.chromium.org/299743002/diff/240001/sandbox/linux/seccomp-b...
File sandbox/linux/seccomp-bpf-helpers/cons_unittest.cc (right):

https://codereview.chromium.org/299743002/diff/240001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf-helpers/cons_unittest.cc:23: }  // namespace
On 2014/06/27 01:45:50, jln (OOO til July 15th) wrote:
> It's perfectly ok to have TEST() in the anonymous namespace (and it's often
the
> case in sandbox/)

Oops, done.

[mdempsky, 2014-06-28 00:37:15]

Jeffrey: Do you think you'll want/have time to review this before I submit it?

[Jeffrey Yasskin, 2014-06-28 00:56:04]

I don't expect to have a chance to do a general review. I can answer
specific questions if you want, but go ahead and submit if you're
happy with it.

On Fri, Jun 27, 2014 at 5:37 PM,  <mdempsky@chromium.org> wrote:
> Jeffrey: Do you think you'll want/have time to review this before I submit
> it?
>
> https://codereview.chromium.org/299743002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jln (very slow on Chromium), 2014-06-28 01:02:11]

PS#15 lgtm

[mdempsky, 2014-07-15 19:20:28]

Moved to sandbox/linux/bpf_dsl.

[mdempsky, 2014-07-15 19:20:32]

The CQ bit was checked by mdempsky@chromium.org

[commit-bot: I haz the power, 2014-07-15 19:24:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mdempsky@chromium.org/299743002/330001

[commit-bot: I haz the power, 2014-07-16 05:11:13]

Change committed as 283350

[tapted, 2014-07-16 06:57:40]

A revert of this CL has been created in
https://codereview.chromium.org/391043003/ by tapted@chromium.org.

The reason for reverting is: Suspected for sandbox_linux_unittests failures on
Linux Tests (dbg)(2)(32) at
http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29...

errors in
BPFDSL.MoreBooleanLogic
Actual test failure: ../../sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc:117:(11) ==
((*__errno_location ()))
BPFDSL.MaskTest
Actual test failure:
../../sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc:206:(expect_errno) ==
((*__errno_location ()))
BPFDSL.ElseIfTest
Actual test failure: ../../sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc:235:(0) ==
(setuid(0))

output like
Value of: subprocess_exit_status
Actual: 1
Expected: kExpectedValue
Which is: 42.

[jln (very slow on Chromium), 2014-07-16 17:06:15]

linux_rel_precise32 can reproduce the failure.

[mdempsky, 2014-07-16 17:23:32]

On 2014/07/16 17:06:15, jln wrote:
> linux_rel_precise32 can reproduce the failure.

Thanks.  I'm wondering if glibc does something odd on x86 and wraps those system
calls somehow?  Maybe I should directly call the underlying system calls (e.g.,
via syscall() or LSS) to remove an unknown?