Linux sandbox: implement new BPF testing macros

Linux sandbox: implement new BPF testing macros

Implement new BPF_TEST_C and BPF_TEST_D macros to allow writing new BPF
tests without using the deprecated "BPF policies as function pointers".

BUG=368970
R=mdempsky@chromium.org
TBR=jorgelo

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

[jln (very slow on Chromium), 2014-05-07 00:25:10]

Matthew, could you please take a look?

There is very little new code :)

https://codereview.chromium.org/269303002/diff/1/sandbox/linux/seccomp-bpf/bp...
File sandbox/linux/seccomp-bpf/bpf_tester_compatibility_delegate.h (right):

https://codereview.chromium.org/269303002/diff/1/sandbox/linux/seccomp-bpf/bp...
sandbox/linux/seccomp-bpf/bpf_tester_compatibility_delegate.h:20: // This
templated class allows to build a BPFTesterDelegate from a
This comment is new.

https://codereview.chromium.org/269303002/diff/1/sandbox/linux/seccomp-bpf/bp...
sandbox/linux/seccomp-bpf/bpf_tester_compatibility_delegate.h:27: template
<class Aux = void>
This is code moving without changes.

[mdempsky, 2014-05-07 00:57:39]

lgtm

https://codereview.chromium.org/269303002/diff/1/sandbox/linux/seccomp-bpf/bp...
File sandbox/linux/seccomp-bpf/bpf_tester_compatibility_delegate.h (right):

https://codereview.chromium.org/269303002/diff/1/sandbox/linux/seccomp-bpf/bp...
sandbox/linux/seccomp-bpf/bpf_tester_compatibility_delegate.h:20: // This
templated class allows to build a BPFTesterDelegate from a
I'd say "allows building" instead of "allows to build".

https://codereview.chromium.org/269303002/diff/1/sandbox/linux/seccomp-bpf/bp...
File sandbox/linux/seccomp-bpf/bpf_tests.h (right):

https://codereview.chromium.org/269303002/diff/1/sandbox/linux/seccomp-bpf/bp...
sandbox/linux/seccomp-bpf/bpf_tests.h:32: // This form of BPF_TEST is now
discouraged (but still allowed) in favor of
Doesn't need to be addressed in this CL, but the order that things are defined
and documented in is getting a bit confusing, I think.  E.g., if you start
reading from the top, you first learn that BPF_DEATH_TEST is like BPF_TEST, but
you don't know what that is yet... then scrolling down to find out what BPF_TEST
does, you find out it's deprecated, and you should use other macros that are
even further down.

Since I don't believe macro definition order matters, I'd probably reorganize a
bit so the more commonly used and easier explained things (e.g., BPF_TEST_C) are
defined first, and then after that we can start explaining more
advanced/flexible functions in terms of it (e.g., BPF_DEATH_TEST_C allows
specifying a custom death predicate; BPF_TEST_D allows specifying an entirely
custom delegate implementation; ...).

https://codereview.chromium.org/269303002/diff/1/sandbox/linux/seccomp-bpf/bp...
sandbox/linux/seccomp-bpf/bpf_tests.h:105: // This is the prefered format for
new BPF tests. It takes a class name
Nit: Preferred has two 'r's.

[jln (very slow on Chromium), 2014-05-07 01:07:37]

Thanks. I re-ordered the macros and tweaked the comments a little.

Feel free to take another look :)

https://chromiumcodereview.appspot.com/269303002/diff/1/sandbox/linux/seccomp...
File sandbox/linux/seccomp-bpf/bpf_tester_compatibility_delegate.h (right):

https://chromiumcodereview.appspot.com/269303002/diff/1/sandbox/linux/seccomp...
sandbox/linux/seccomp-bpf/bpf_tester_compatibility_delegate.h:20: // This
templated class allows to build a BPFTesterDelegate from a
On 2014/05/07 00:57:39, mdempsky wrote:
> I'd say "allows building" instead of "allows to build".

Done.

https://chromiumcodereview.appspot.com/269303002/diff/1/sandbox/linux/seccomp...
File sandbox/linux/seccomp-bpf/bpf_tests.h (right):

https://chromiumcodereview.appspot.com/269303002/diff/1/sandbox/linux/seccomp...
sandbox/linux/seccomp-bpf/bpf_tests.h:32: // This form of BPF_TEST is now
discouraged (but still allowed) in favor of
On 2014/05/07 00:57:39, mdempsky wrote:
> Doesn't need to be addressed in this CL, but the order that things are defined
> and documented in is getting a bit confusing, I think.  E.g., if you start
> reading from the top, you first learn that BPF_DEATH_TEST is like BPF_TEST,
but
> you don't know what that is yet... then scrolling down to find out what
BPF_TEST
> does, you find out it's deprecated, and you should use other macros that are
> even further down.
> 
> Since I don't believe macro definition order matters, I'd probably reorganize
a
> bit so the more commonly used and easier explained things (e.g., BPF_TEST_C)
are
> defined first, and then after that we can start explaining more
> advanced/flexible functions in terms of it (e.g., BPF_DEATH_TEST_C allows
> specifying a custom death predicate; BPF_TEST_D allows specifying an entirely
> custom delegate implementation; ...).

Done, thanks!

https://chromiumcodereview.appspot.com/269303002/diff/1/sandbox/linux/seccomp...
sandbox/linux/seccomp-bpf/bpf_tests.h:105: // This is the prefered format for
new BPF tests. It takes a class name
On 2014/05/07 00:57:39, mdempsky wrote:
> Nit: Preferred has two 'r's.

Done.

[jln (very slow on Chromium), 2014-05-07 01:42:05]

Matthew: the last patchset adds assertion that the policy is created and used in
the same process. Do you mind taking a look at that last diff?

https://chromiumcodereview.appspot.com/269303002/diff/50001/sandbox/linux/sec...
File sandbox/linux/seccomp-bpf/bpf_tests_unittest.cc (right):

https://chromiumcodereview.appspot.com/269303002/diff/50001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf/bpf_tests_unittest.cc:101: BPF_ASSERT_EQ(my_pid_,
syscall(__NR_getpid));
I added these checks in the last patch set.

[mdempsky, 2014-05-07 02:18:34]

lgtm

[jln (very slow on Chromium), 2014-05-07 04:39:02]

The CQ bit was checked by jln@chromium.org

[jln (very slow on Chromium), 2014-05-07 04:41:48]

Committed patchset #4 manually as r268668 (presubmit successful).