hans@chromium.org [Wed, 12 Dec 2012 18:14:34 +0000 (18:14 +0000)]
Make Maps constructor explicit.
BUG=https://code.google.com/p/chromium/issues/detail?id=163357
Review URL: https://codereview.chromium.org/
11547014
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@189
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Thu, 25 Oct 2012 04:21:39 +0000 (04:21 +0000)]
Work-around for an erroneous warning message in gcc-4.7
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@188
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Tue, 14 Aug 2012 00:47:30 +0000 (00:47 +0000)]
Add out-of-line copy ctor to Maps::Iterator.
Chrome's style checking plugin complains that this class needs an explicit
out-of-line copy constructor.
So far, the style plugin has not warned about nested classes because of a bug,
but we have fixed that bug and are now cleaning up all the cases where it warns.
Committed on behalf of Hans Wennborg (hans@chromium.org).
BUG=http://code.google.com/p/chromium/issues/detail?id=139346
Review URL: https://chromiumcodereview.appspot.com/
10824229
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@187
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Mon, 21 May 2012 23:06:45 +0000 (23:06 +0000)]
I am somewhat reluctantly changing the API so that setProcFD is no longer
idempotent with regards to global state. But Julien Tinnes convinced me that
we should try to hold on to /proc for as short a time as possible.
This change potentially makes things a little more difficult for callers, as
they can now no longer re-use the file descriptor passed into setProcFd(), so
I updated the comments to warn about the changed semantics.
BUG=n/a
TEST=make test
Review URL: https://chromiumcodereview.appspot.com/
10399114
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@186
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Mon, 21 May 2012 20:43:17 +0000 (20:43 +0000)]
Get rid of a call to dup() that is no longer needed with the way the API works
now. It was harmless, but it certainly confuses my poor code reviewers :-)
TEST=make test
BUG=n/a
Review URL: https://chromiumcodereview.appspot.com/
10399109
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@185
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Fri, 18 May 2012 01:35:32 +0000 (01:35 +0000)]
Change the sandbox API to require passing in a copy of /proc instead of
/proc/self. This allows "SupportsSeccompSandbox()" to work correctly, even
after it had to "fork()". Otherwise, the old "/proc/self" would point to
the parent process, and some kernels don't allow accessing it from the
child (even though it is still a valid file descriptor; it just stops
working).
BUG=n/a
TEST=make test
Review URL: https://chromiumcodereview.appspot.com/
10389201
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@184
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Tue, 15 May 2012 01:10:37 +0000 (01:10 +0000)]
PaX-enabled systems require us to mark our executables so that mprotect() doesn't get
restricted. If "paxctl" is available at build-time, run it on all of our binaries.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=20
TEST=make test
Review URL: https://chromiumcodereview.appspot.com/
10384175
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@183
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Tue, 15 May 2012 01:09:39 +0000 (01:09 +0000)]
Include <unistd.h>, as we are otherwise missing definitions on some systems.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=19
TEST=make test
Review URL: https://chromiumcodereview.appspot.com/
10384174
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@182
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Tue, 15 May 2012 01:06:42 +0000 (01:06 +0000)]
Import <sys/types.h>, as that is the canonical location of "ssize_t".
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=18
TEST=make test
Review URL: https://chromiumcodereview.appspot.com/
10388133
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@181
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Tue, 1 May 2012 00:41:51 +0000 (00:41 +0000)]
Make a small change to the API. Instead of passing in a file
descriptor to /proc/self/maps, we now pass in a file descriptor
to /proc/self and open "maps" ourselves. This is a more generic
API that will make it easier to add other features in the future
(e.g. merge the setuid sandbox into the seccomp sandbox, if the
kernel allow unprivileged calls to chroot).
BUG=none
TEST=make test
Review URL: https://chromiumcodereview.appspot.com/
10178029
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@180
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Wed, 30 Nov 2011 05:34:02 +0000 (05:34 +0000)]
Add test for doing syscalls via "int $0"
This is a test for the fix in r165 (which fixed the 6th syscall
argument on x86-32).
BUG=none
TEST="make test"
Review URL: http://codereview.chromium.org/8727036
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@179
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Sat, 19 Nov 2011 03:28:04 +0000 (03:28 +0000)]
Add logic for patching calls to the x86-64 vsyscall page
This is necessary on Linux 3.1 because the vsyscalls now make real
syscalls to the kernel, rather than just reading memory as they
usually did before, which means they fail in seccomp mode.
Although the vsyscall page is deprecated, glibc still contains some
calls to it. We detect and patch the instruction sequence that
matters. (We do this because, unfortunately, the kernel does not let
us change the permissions on the vsyscall page to patch it.)
glibc still contains a code path that could call vgettimeofday via a
different instruction sequence, which is much harder to patch, and we
don't try to. libc.so has code to store vgettimeofday's address
(0xffffffffff600000) in TLS, but in practice this code path is not
used when the vdso is present.
To apply the patch we replace the instructions with a syscall, which
later gets re-patched to be a jump.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=17
TEST=test_patching_vsyscall_* on any Linux version,
plus test_time and test_sched_getcpu on Linux 3.1
Review URL: http://codereview.chromium.org/8605003
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@178
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Fri, 18 Nov 2011 21:17:05 +0000 (21:17 +0000)]
Add test for patching a system call instruction
This tests patching a specific, fixed instruction sequence, whereas
the existing tests just test whatever is in the version of glibc on
the system.
Change library.cc to make the patching code easier to test: Split out
a patchSystemCallsInRange() method, since the existing methods assume
they are operating on a whole dynamically-loaded ELF object. Change
maps_ to be per-instance so that creating Library objects with
different Maps objects works.
This doesn't try to cover the various corner cases in library.cc yet.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=17
TEST=test_patching_syscall
Review URL: http://codereview.chromium.org/8596009
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@177
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Fri, 18 Nov 2011 18:04:10 +0000 (18:04 +0000)]
Add tests for glibc functions which sometimes use the x86-64 vsyscall page
test_time and test_sched_getcpu reproduce a problem on Linux 3.1
(e.g. with the current version of Ubuntu Precise), which I'll fix in a
later change. These tests will hang, because the untrusted thread is
killed when it attempts to do a syscall.
test_gettimeofday is included for completeness.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=17
TEST=should fail on Linux 3.1 but pass on earlier kernel versions
Review URL: http://codereview.chromium.org/8558011
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@176
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Fri, 18 Nov 2011 16:20:26 +0000 (16:20 +0000)]
Tests: Split generic test runner out into test_runner.{cc,h}
This will allow test cases to be spread across multiple files rather
than putting them all in one big file as they are now.
BUG=none
TEST="make all test" plus run tests through Gyp too
Review URL: http://codereview.chromium.org/8558007
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@175
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Fri, 18 Nov 2011 00:24:33 +0000 (00:24 +0000)]
Test runner: Make test discovery work within C++ (or C)
Remove the hack I wrote that used a Python script to scrape the TEST()
function names. Replace it with a different hack: Use constructors to
register the test functions in a global list before main() gets
called.
This simplifies the build scripts. It will also make it easier to
split test_syscalls.cc into smaller files.
It also means that #ifs around test cases will work.
BUG=none
TEST="make all test"
Review URL: http://codereview.chromium.org/8586016
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@174
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Fri, 18 Nov 2011 00:16:38 +0000 (00:16 +0000)]
Add a tool for patching ELF libraries/executables offline
This is for patching ELF objects for use with elf_loader.cc. It
allows system call instructions to be patched before the library or
executable is loaded.
For simplicity, we replace system calls with "int $0", which is not
very fast at run time. A more sophisticated version could insert
jumps like library.cc does, but this would involve adding an extra
code segment to the ELF object.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=13
TEST=For example:
./patch_offline /lib/ld-linux.so.2 -o ld.so.patched
objdump -d ld.so.patched | grep -w int
Automated tests will be added in a later change.
Review URL: http://codereview.chromium.org/8589027
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@173
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Thu, 17 Nov 2011 00:55:26 +0000 (00:55 +0000)]
Remove some unused code in the library patcher
library.cc parses the PLT table and fills out plt_entries_, but this
is not subsequently used, so we can remove it. Also remove some other
unused definitions.
BUG=none
TEST="make all test"
Review URL: http://codereview.chromium.org/8578008
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@172
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Wed, 16 Nov 2011 16:16:06 +0000 (16:16 +0000)]
Add an ELF loader that can load and run an executable in the sandbox
This can load both statically-linked executables and ld.so (the
dynamic linker). These have to be pre-patched to do syscalls via
"int $0" - I will add a tool for doing this patching in another change.
The ELF loader itself also runs with sandboxing enabled.
Currently the loader is dynamically linked against glibc, which will
probably cause problems when another copy of libc gets loaded into the
process, so in the long term we will want to statically link the
loader.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=13
TEST=to be added in a later change
Review URL: http://codereview.chromium.org/7634024
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@171
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Tue, 27 Sep 2011 15:34:58 +0000 (15:34 +0000)]
In 32bit mode, removed all assembly addressing modes that are incompatible with
position independent code. Fedora is configured to refuse running binaries that
require relocations.
TEST=add -fPIC and -fPIE to CFLAGS and LDFLAGS respectively, then run "scanelf -qT ./preload32.so". There shouldn't be any output.
BUG=http://code.google.com/p/chromium/issues/detail?id=87704
Review URL: http://codereview.chromium.org/8036047
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@170
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Fri, 16 Sep 2011 18:43:25 +0000 (18:43 +0000)]
Original changelist: codereview.chromium.org/7326013/
TEST=make test
BUG=none
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@169
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Fri, 19 Aug 2011 02:44:25 +0000 (02:44 +0000)]
Committed clang-related fix, as requested by Nico Weber.
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@168
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Mon, 15 Aug 2011 17:31:27 +0000 (17:31 +0000)]
Allow the ftruncate() system call from within the sandbox.
Review URL: http://codereview.chromium.org/7650011
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@167
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Thu, 11 Aug 2011 07:06:14 +0000 (07:06 +0000)]
Allow writev() and readv() system calls
The dynamic linker (ld.so) uses writev() to print error messages.
Allow readv() for symmetry.
We already allow preadv() and pwritev().
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=13
TEST="make test"
Review URL: http://codereview.chromium.org/7462025
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@166
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Thu, 4 Aug 2011 19:24:37 +0000 (19:24 +0000)]
x86-32: Fix the 6th argument on syscalls invoked via "int $0"
The assembly-code signal handler restores %ebp (the 6th system call
argument) from the wrong offset in the signal stack frame. This
breaks the "offset" argument of the mmap() system call.
This change can be checked against /usr/include/asm/sigcontext.h.
"struct sigcontext" includes:
unsigned long edi; // offset 0xb4
unsigned long esi; // offset 0xb8
unsigned long ebp; // offset 0xbc
unsigned long esp; // offset 0xc0
unsigned long ebx; // offset 0xc4
unsigned long edx; // offset 0xc8
unsigned long ecx; // offset 0xcc
unsigned long eax; // offset 0xd0
BUG=none
TEST=Tested via running ld.so (the glibc dynamic linker) under the sandbox.
I'll add an automated test that does this in a future change.
Review URL: http://codereview.chromium.org/7564032
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@165
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Thu, 28 Jul 2011 23:16:35 +0000 (23:16 +0000)]
While the seccomp sandbox appears to work correctly when built with "clang",
the unittest was still failing. It turns out that clang's built-in assembler
turns all short jumps into long jumps. The unittest inspects the generated
instructions to verify that backtraces of instrumented system calls look the
way that we expect them to do. And of course, the signature no longer matches
if the assembler outputs a different sequence of bytes than what we tried to
tell it.
This changelist makes the unittest more tolerant of different instruction
streams.
BUG=http://code.google.com/p/chromium/issues/detail?id=70871
TEST=Build with clang, then run "make test"
Review URL: http://codereview.chromium.org/7533008
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@164
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Thu, 28 Jul 2011 21:08:53 +0000 (21:08 +0000)]
Fix build: Add file missed in previous commit, tls_setup.h
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=13
TEST="make clean test" in fresh checkout
Review URL: http://codereview.chromium.org/7531007
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@163
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Wed, 27 Jul 2011 19:57:21 +0000 (19:57 +0000)]
Implement TLS initialisation system calls for i386 and x86-64
This will allow ld.so (the dynamic linker) to start up under the
sandbox.
This is not straightforward because the set_thread_area()/arch_prctl()
syscalls modify a thread's state, so we cannot simply forward the
syscall to the trusted thread, because that would modify the wrong
thread's state.
Instead, we use the trick of using clone() to create a new thread that
picks up where the original thread left off, but with modified thread
state. Furthermore, clone() provides a built-in way of setting up
TLS.
This requires two further tricks:
* Capture a signal frame that we can use to return into the new thread.
* Do write() and then _exit() without using the stack inbetween,
because the stack is used by the new thread that write() spawns.
Pleasantly, this can all be done without changing the trusted code.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=13
TEST=test_tls_setup_via_arch_prctl, test_tls_setup_via_set_thread_area
Review URL: http://codereview.chromium.org/4848001
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@162
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Wed, 27 Jul 2011 19:29:04 +0000 (19:29 +0000)]
Fix the makefile to work after the previous change, r160
The previous change replaced syscall_table.c with system_call_table.cc.
BUG=none
TEST="make test"
Review URL: http://codereview.chromium.org/7514033
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@161
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Tue, 19 Jul 2011 21:49:35 +0000 (21:49 +0000)]
Finally eliminated the dirty hack that we used for populating the system
call table. This table needs to be in read-only memory, and it needs to
have all of the unused entries zero'd out. We used to rely a C having
designated initializers. And we also abused a gcc extension to move the
data structure into read-only memory.
Instead, we are now explicitly managing this data structure, which allows us
to remove our dependency on C. All code is now either C++ or assembly. This
had a trickle down effect of cleaning up a lot of other code and getting rid
of a lot of "asm" labels for identifiers that are no longer referenced by
anything but C++ code.
BUG=http://code.google.com/p/chromium/issues/detail?id=88578
TEST=make test
Review URL: http://codereview.chromium.org/7397023
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@160
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Mon, 18 Jul 2011 19:16:21 +0000 (19:16 +0000)]
Moved complex constructors/destructors out of header files.
BUG=http://code.google.com/p/chromium/issues/detail?id=88578
Review URL: http://codereview.chromium.org/7324003
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@159
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Fri, 15 Jul 2011 07:11:41 +0000 (07:11 +0000)]
Unconditionally allow p{read,write}{,v}().
TEST=make test, also run browser_tests in Chrome
BUG=http://code.google.com/p/chromium/issues/detail?id=89114
Review URL: http://codereview.chromium.org/7387002
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@158
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Thu, 30 Jun 2011 22:08:53 +0000 (22:08 +0000)]
Assortment of fixes to make the code work with (very recent) versions of
clang. The following issues were noticed:
- clang complains about unused values computed by C expressions
- clang makes extensive use of SSE registers. This requires us to
explicitly align the stack pointer in x86-32. It seems as if x86-64
already works correctly, but we might need to revisit this assessment
if we encounter problems.
- clang doesn't implicitly do a const_cast() when it does a
reinterpret_cast(). We now do a C-style cast instead, as that ends
up being more readable.
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@157
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Thu, 9 Jun 2011 22:34:12 +0000 (22:34 +0000)]
A couple of changes that make "clang" happier. We still make use of other
gcc-specific features that are unsupported in "clang". So, don't expect the
code to build and/or run with "clang" just yet.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=15
TEST=run "make" and notice fewer errors than before
Review URL: http://codereview.chromium.org/7134059
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@156
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Fri, 8 Apr 2011 19:47:24 +0000 (19:47 +0000)]
Allow nanosleep() system call, and add a policy setting that allows
SysV shared memory accesses to be completely unrestricted.
These changes are necessary to make the seccomp sandbox compatible
with some of the Chrome unittests.
BUG=http://code.google.com/p/chromium/issues/detail?id=66906
TEST=make test
Review URL: http://codereview.chromium.org/6720004
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@155
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Thu, 27 Jan 2011 22:41:42 +0000 (22:41 +0000)]
Add explicit size suffix to ambiguous "cmp" instructions
This fixes a bug on x86-64 where the signal handler function pointer
would be read as 32-bit instead of 64-bit and so only the lower 32
bits would be compared with 0 and 1 (SIG_DFL and SIG_IGN).
This also fixes warnings produced by Clang's assembler.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=14
TEST="make test" (with gcc) and also build with Clang (though this
produces various other warnings/errors)
Review URL: http://codereview.chromium.org/6264021
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@154
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Mon, 6 Dec 2010 21:53:05 +0000 (21:53 +0000)]
Evan Martin asked for another fix in his series of changes to BranchTargets.
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@153
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Mon, 6 Dec 2010 19:09:46 +0000 (19:09 +0000)]
Committed another iteration of codereview.chromium.org/5599002/
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@152
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Thu, 2 Dec 2010 23:12:17 +0000 (23:12 +0000)]
Committing evan's changelist: codereview.chromium.org/5599002/
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@151
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Fri, 12 Nov 2010 23:26:21 +0000 (23:26 +0000)]
Gcc as shipped by Fedora 14 prints a bogus warning message when passing
NULL to a templatized function. Casting NULL to "void *" will hopefully
work around this problem.
N.b. we currently do not officially support compiling with anything newer
than gcc 4.4 and on anything other than Ubuntu. Regressions with regards to
compiler warnings are expected.
BUG=http://code.google.com/p/chromium/issues/detail?id=62916
TEST=none: not tested on gcc 4.5.1
Review URL: http://codereview.chromium.org/4900004
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@150
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Tue, 9 Nov 2010 19:47:43 +0000 (19:47 +0000)]
Split sandbox.cc's asm code into .S files, part 2 of 2
Update the code in the .S files with a symbol name and copyright
header, and word wrap some text.
We put the arch-specific conditionals into a .S file, as we did with
trusted_thread_asm.S.
Remove the original copy of the code in sandbox.cc.
Remove the throwaway extraction script.
BUG=none
TEST=make test all
Review URL: http://codereview.chromium.org/4243005
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@149
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Tue, 9 Nov 2010 19:45:56 +0000 (19:45 +0000)]
Split sandbox.cc's asm code into .S files, part 1 of 2
We use the same approach for splitting out inline assembly as for
trusted_thread.cc in r99.
In this first step we extract the i386/x86-64 assembly code from
sandbox.cc into separate fault_handler_*.S files. This is
semi-automatically extracted: the asm() quotes are removed and
register names are converted. We include the script that performed
the extraction, plus changes to sandbox.cc that helped with the
extraction.
The new files will be made functional in the next commit.
BUG=none
TEST=make test all
Review URL: http://codereview.chromium.org/4325001
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@148
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Tue, 9 Nov 2010 16:45:41 +0000 (16:45 +0000)]
On older 32bit kernels (e.g. Ubuntu Hardy), the seccomp sandbox fails to handle
signals correctly. This is primarily a result of the kernel not supporting
non-executable data segments. But it also runs into problems because the
format of the signal frame is subtly different and does not appear to always
include a "magic restorer function".
This changelist removes all dependencies on NX support from the 32bit version
of the code. And it eliminates the code that patches the restorer function.
Both of these features were originally added to make it easier for gdb to
debug code that runs inside of a signal handler. But given the observed problems
with this approach, it does not seem worth the effort.
64bit code seems unaffected by all of these problems -- presumably because
that architecture is a lot more recent. So, we'll not make any changes to it.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=5
TEST=make test
Review URL: http://codereview.chromium.org/4688002
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@147
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Tue, 9 Nov 2010 09:06:19 +0000 (09:06 +0000)]
Style: Fix recently-added tabs; convert to spaces
This fixes the tabs that I introduced accidentally in r139 because
I forgot to set
(set-default 'indent-tabs-mode nil)
in the instance of Emacs that I was using.
BUG=none
TEST=make all test
Review URL: http://codereview.chromium.org/3922004
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@146
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Wed, 20 Oct 2010 21:21:43 +0000 (21:21 +0000)]
Factor out duplicated code for checking for protected memory mappings
Introduce isRegionProtected() for the common code.
ipc.cc contained a check for an address being in the protected area,
but we convert this to an address range check.
Add tests for madvise().
BUG=none
TEST=make all test
Review URL: http://codereview.chromium.org/3951003
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@145
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Wed, 20 Oct 2010 18:18:34 +0000 (18:18 +0000)]
Tidy: Remove no-op address adjustment in arguments for locked syscalls
Replace "info->mem->pathname - (char*)info->mem + (char*)info->mem->self"
with "info->mem->pathname", because info->mem == info->mem->self.
Presumably this arithmetic was intended to handle a case where the
secure memory areas are mapped at different addresses in the trusted
process and the sandboxed process, but that is not the case now.
BUG=none
TEST=make all test
Review URL: http://codereview.chromium.org/3929003
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@144
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Wed, 20 Oct 2010 08:04:34 +0000 (08:04 +0000)]
Fix deadlock between recvmsg() and sendmsg() on i386
Change recvmsg(), recvfrom() and recv() to use SEND_LOCKED_ASYNC on
i386, so that the trusted process does not block waiting for them to
complete.
The problem did not occur on x86-64 since the recv*() calls pass
arguments in registers there and so use SEND_UNLOCKED.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=2
TEST=make all test
Review URL: http://codereview.chromium.org/3874001
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@143
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Wed, 20 Oct 2010 07:58:30 +0000 (07:58 +0000)]
sendSystemCall(): Change the "locked" parameter from a bool to an enum
Change false to SEND_UNLOCKED
and true to SEND_LOCKED_SYNC
and add a third option, SEND_LOCKED_ASYNC.
This is for blocking system calls, where the trusted process cannot
wait for the syscall to complete because it would deadlock other
threads.
This change is a pure refactoring: Some syscalls will be changed to
use SEND_LOCKED_ASYNC in later changes.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=2
TEST=make all test
Review URL: http://codereview.chromium.org/3871001
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@142
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Wed, 20 Oct 2010 00:30:56 +0000 (00:30 +0000)]
Make sure that the code can be compiled both in release and debug mode.
We previously had an undefined reference when compiling in release mode.
TEST=make clean all CPPFLAGS=-DNDEBUG
BUG=none
Review URL: http://codereview.chromium.org/3898005
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@141
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Tue, 19 Oct 2010 16:48:52 +0000 (16:48 +0000)]
Factor out duplicated code for setsockopt()/getsockopt()
BUG=none
TEST=make all test
Review URL: http://codereview.chromium.org/3770020
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@140
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Tue, 19 Oct 2010 16:45:50 +0000 (16:45 +0000)]
Fix setsockopt() on i386 and add tests for setsockopt()/getsockopt()
process_socketcall() was reading the wrong message size. This could
lead to it expecting too much data (in which case the trusted process
will hang), or too little (in which case it will read bad data for the
next syscall).
Another possibility would have been to remove "OFF(setsockopt,
optval), OFF(setsockopt, optlen)" from socketCallArgInfo, since the
setsockopt() option value currently does not need to be validated.
BUG=none
TEST=make all test
Review URL: http://codereview.chromium.org/3804009
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@139
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Mon, 18 Oct 2010 23:44:28 +0000 (23:44 +0000)]
Allow fdatasync() inside of the sandbox. This is needed by SQLite.
TEST=make test
BUG=http://code.google.com/p/chromium/issues/detail?id=59420
Review URL: http://codereview.chromium.org/3796012
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@138
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Sun, 17 Oct 2010 18:19:44 +0000 (18:19 +0000)]
Debug mode: Remove "%gs == 0" check which is always true on x86-64
This fixes debug mode on x86-64 kernels where gettimeofday() does a
system call, which has been observed running Ubuntu Lucid in a VM.
Before this change, the code recurses infinitely between
gettimeofday() and Debug::syscall() and runs out of stack.
The check is incorrect because %fs and %gs are usually both 0 on
x86-64, even when TLS is initialised. Unlike on i386, their numeric
value is just a dummy value and does not act as a segment selector.
Instead of assigning %gs = 0 in the trusted process, disable debug
mode there.
Remove the TLS check from Debug::message() because this is called via
int $0 during sandbox startup before TLS is set up.
Make similar changes to i386 for consistency.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=4
TEST="make test" in a VirtualBox VM.
This can also be tested by changing
::gettimeofday(&tv, NULL);
in debug.cc to
::syscall(__NR_gettimeofday, &tv, NULL);
Review URL: http://codereview.chromium.org/3770012
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@137
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Sun, 17 Oct 2010 18:02:08 +0000 (18:02 +0000)]
Test runner: Set a limit on stack size to avoid accidental OOM
A bug in the debug code can trigger the OOM killer when running
test_debugging, which tends to kill processes at random. Set a limit
on the stack size in order to prevent this happening when running
tests. The bug will be fixed in a later change.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=4
TEST="make test" fails safely on an x86-64 system where gettimeofday()
causes a system call.
Review URL: http://codereview.chromium.org/3770011
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@136
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Thu, 14 Oct 2010 16:07:00 +0000 (16:07 +0000)]
Use one mutex per thread instead of a global syscall mutex
This will allow one thread to be blocked executing a locked syscall,
such as sendmsg(), while another thread also executes a locked
syscall. However, none of the syscalls have been changed to allow
this yet; this will be done in a subsequent change.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=2
TEST=make test all
Review URL: http://codereview.chromium.org/3522010
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@135
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Thu, 14 Oct 2010 15:49:54 +0000 (15:49 +0000)]
Change clone() to be an unlocked system call
Remove the need to unlock syscall_mutex_ while handling clone().
This is in preparation for changing the sandbox to use one mutex per
thread instead of one global mutex. If we kept clone()'s unlocking
code and unlocked the parent thread's mutex, we would need to figure
out what to do for the first thread, which has no parent. It is
simpler to remove the unlocking code instead.
This is safe because:
* clone() does not need to be locked because the kernel does not read
any of its arguments from memory.
* For any clone() arguments the trusted thread reads, it checks the
sequence number.
* The trusted thread does not read from the secure memory area after
its sendmsg() call, and the trusted process will not overwrite the
secure memory area until it receives this message.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=2
TEST=make test all
Review URL: http://codereview.chromium.org/3613004
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@134
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Wed, 6 Oct 2010 17:20:11 +0000 (17:20 +0000)]
Replace assert() with our own CHECK() macro(s). This means we no longer have
to worry about assert() getting optimized away and the tests succeeding
erroneously. It also gives us the ability to automatically print a little
additional information (such as expected vs. observed "errno" values).
TEST=make test
BUG=none
Review URL: http://codereview.chromium.org/3444013
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@133
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Mon, 4 Oct 2010 11:00:56 +0000 (11:00 +0000)]
test_syscalls.cc: Fix use of futex() when waiting for thread
FUTEX_WAIT will return EAGAIN if *tid_ptr != tid, though this is not
made clear on the man page. So change the code to handle the case
where the thread has exited between the "while" check and the futex()
call. Otherwise, the test could be flaky.
Found while working towards:
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=2
TEST=This can be tested by inserting "poll(0, 0, 500);" before the
futex() call. This affects test_clone and test_clone_preserves_registers.
Review URL: http://codereview.chromium.org/3592005
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@132
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Sun, 3 Oct 2010 18:08:47 +0000 (18:08 +0000)]
Remove cloneFdPub from SecureMem::Args and use the global variable instead
This simplifies the code a little. The same file descriptor is used
for all the threads anyway.
Found while working towards
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=2
TEST=make test all
Review URL: http://codereview.chromium.org/3618002
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@131
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Fri, 1 Oct 2010 22:09:36 +0000 (22:09 +0000)]
Always #include foo.h at the top of foo.cc
This convention should catch any header files that cannot be #included
on their own because they depend on something else being #included
first.
BUG=none
TEST=make test all
Review URL: http://codereview.chromium.org/3524008
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@130
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Fri, 1 Oct 2010 20:50:13 +0000 (20:50 +0000)]
Remove unused processFdPub from SecureMem::Args
The trusted thread does not use this file descriptor.
Found while working towards:
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=2
TEST=make test
Review URL: http://codereview.chromium.org/3599007
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@129
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Fri, 1 Oct 2010 16:10:35 +0000 (16:10 +0000)]
Whitespace fixes.
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/3602006
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@128
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Fri, 1 Oct 2010 15:12:09 +0000 (15:12 +0000)]
Refactor the code used to communicate from the trusted process to the other
parts of the sandbox. We are introducing a new ProcessRPCInfo object that
encapsulates all the information needed to do this. In particular, we now
pass the actual system call number from the trusted process's mainloop to
the process_XXX() function. This information is read-only and thus a lot less
likely to accidentally be set incorrectly.
This fixes a security-relevant bug where an attacker could control the system
call number that we would send from the trusted process to the trusted thread.
TEST=make test
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=6
Review URL: http://codereview.chromium.org/3414016
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@127
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Fri, 1 Oct 2010 15:12:01 +0000 (15:12 +0000)]
Make error messages look more like what the non-reference version of the code
prints. This will be helpful whenever tests look for sandbox failure.
Also, out of general principle, make CPU registers "unsigned long". That
makes it less likely we accidentally introduce integer overflow problems.
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/3574004
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@126
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Fri, 1 Oct 2010 08:37:03 +0000 (08:37 +0000)]
Fix vulnerability in process_sigaction()
This fixes a bug in which the trusted process will forward an
unchecked syscall number, received from an untrusted thread, for
execution by a trusted thread. Add a check for this syscall
number.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=6
TEST=None: Although we could write a test for this check, in the
longer term we should probably refactor the message format to
remove the duplicated syscall number.
Review URL: http://codereview.chromium.org/3542001
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@125
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Wed, 29 Sep 2010 13:20:54 +0000 (13:20 +0000)]
Fix hole that allows tampering with sendmsg() arguments
Ensure that "struct msghdr" is stored in a page that is mapped as
MAP_PRIVATE by reserving space in the executable/library's data
segment, which will be a protected mapping.
Before, the trusted thread used the stack provided by untrusted code,
which could have been mapped with MAP_SHARED and so could be shared
between the trusted fork()'d helper process and the untrusted threads.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=3
TEST=None: The bug involves a race condition which I haven't tried
to exploit.
Review URL: http://codereview.chromium.org/3532002
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@124
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Sat, 25 Sep 2010 11:16:25 +0000 (11:16 +0000)]
Trusted thread: Add error checks to all syscalls that should return zero
One reason for doing this is that a work-in-progress change I have for
fixing http://code.google.com/p/seccompsandbox/issues/detail?id=2 made
one of the mprotect() calls fail, but without making any of the tests
fail. We should make sure that such mistakes are caught by the tests.
Review URL: http://codereview.chromium.org/3384026
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@123
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Tue, 21 Sep 2010 12:09:57 +0000 (12:09 +0000)]
clone(): Copy registers across in untrusted code instead of trusted code
Copy register values to restore into the signal stack frame early on,
in sandbox_clone() (untrusted code). This means we don't need to
include the register values in the message sent to the trusted
process, and we don't need to copy them to and from the secure memory
area.
In the trusted thread code, we no longer need to store the
return-address-or-zero in %mm3 (i386) or %r15 (x86-64). The return
address is always in the signal stack frame, so we can remove two
conditionals for the special case of the first thread.
Review URL: http://codereview.chromium.org/3394009
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@122
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Mon, 20 Sep 2010 15:05:49 +0000 (15:05 +0000)]
Add test to check that clone() preserves registers
This is a bit of a hassle, because it requires an assembly code
helper, but it is an interesting exercise. In order to test this, we
dump registers into global variables. I have split out some of
test_clone into separate functions.
It is useful because current glibc versions do not rely on most of the
registers being preserved (so this is otherwise untested), but glibc
could rely on this in future.
Also I would like to simplify the register preservation code later,
and I want a test that ensures I don't break it.
Review URL: http://codereview.chromium.org/3443010
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@121
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Thu, 16 Sep 2010 09:57:06 +0000 (09:57 +0000)]
Add test for SysV shared memory
Bring reference_trusted_thread.cc more into line with the real
implementation in order to support this.
Review URL: http://codereview.chromium.org/3404004
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@120
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Wed, 15 Sep 2010 16:17:47 +0000 (16:17 +0000)]
Because of the way how we build our test applications, we can get name
collisions between system headers and local headers. Renaming syscall.h to
syscall_entrypoint.h avoids this problems and fixes a problem building the
sandbox as part of Chrome.
TEST=make chrome
BUG=none
Review URL: http://codereview.chromium.org/3392001
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@119
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Mon, 13 Sep 2010 22:14:00 +0000 (22:14 +0000)]
clone(): Rename arguments that were only named correctly for x86-64
The meaning of arguments 4 and 5 is swapped between i386 and x86-64.
Before, the variables were named after the x86-64 meanings. Rename
them to the more generic "arg4" and "arg5" so that the names cannot be
misleading. The arguments are not interpreted by the sandbox anyway.
Review URL: http://codereview.chromium.org/3369013
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@118
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Mon, 13 Sep 2010 22:00:41 +0000 (22:00 +0000)]
reference_trusted_thread.cc: Get clone() args from argN instead of saved regs
This brings reference_trusted_thread.cc more into line with the real
implementation.
Also make it clearer that the clone syscall's arguments are ordered
differently on i386 and x86-64.
Review URL: http://codereview.chromium.org/3295029
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@117
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Mon, 13 Sep 2010 21:57:51 +0000 (21:57 +0000)]
Use __NR_* symbols in .S files instead of hard-coded constants
Now that the trusted thread code has been moved to .S files from
inline assembly, we can use preprocessor features. Reducing the
number of hard-coded constants should make the assembly code easier to
read and maintain.
BUG=none
TEST=Check that the disassembled code has not changed
Review URL: http://codereview.chromium.org/3330022
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@116
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Mon, 13 Sep 2010 19:11:59 +0000 (19:11 +0000)]
A few small changes that are needed in order to build with very recent
releases of gcc.
Review URL: http://codereview.chromium.org/3338020
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@115
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Sun, 12 Sep 2010 22:37:55 +0000 (22:37 +0000)]
reference_trusted_thread.cc: Use sigreturn to return from clone()
This brings reference_trusted_thread.cc into line with the real
implementation.
It also fixes a race condition, in which HandleNewThread() would read
the parent thread's secureMem after the parent thread could have
exited and marked the secureMem as unreadable.
Review URL: http://codereview.chromium.org/3302023
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@114
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Sun, 12 Sep 2010 22:32:05 +0000 (22:32 +0000)]
Fix use of non-null-terminated strings in debug messages
Add explicit size argument.
BUG=none
TEST=Look at "Denying access" messages in the output from
"env SECCOMP_SANDBOX_DEBUGGING=1 ./run_tests_32"
Review URL: http://codereview.chromium.org/3348020
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@113
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Sun, 12 Sep 2010 22:30:37 +0000 (22:30 +0000)]
Rename label 25 to "fatal_error" to improve readability
This is the most commonly referenced label.
Review URL: http://codereview.chromium.org/3307023
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@112
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Fri, 10 Sep 2010 16:59:26 +0000 (16:59 +0000)]
Remove some more dependency files from "make clean" that had previously been
left behind.
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/3322019
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@111
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Fri, 10 Sep 2010 15:42:44 +0000 (15:42 +0000)]
- push a fake return address and set a magic framepointer whenever
we intercept system calls. This can later be used by breakpad to
clean up the stack that it records in crash dumps.
- set up a system call function pointer that can be used by
linux_syscall_support.h when making system calls. This is needed so
that we properly intercept and wrap system calls directly embedded in
the application.
BUG=37728
TEST=make test
Review URL: http://codereview.chromium.org/3305013
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@110
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Fri, 10 Sep 2010 15:41:56 +0000 (15:41 +0000)]
Fix newlines in string literals in the assembly code
The backslashes got converted incorrectly when I moved the assembly to
.S files.
TEST=Check the warning messages produced by
"env SECCOMP_SANDBOX_DEBUGGING=1 ./run_tests_32"
Review URL: http://codereview.chromium.org/3357027
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@109
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Thu, 9 Sep 2010 18:51:02 +0000 (18:51 +0000)]
Fix segfault in reference_trusted_thread.cc's __NR_exit handler
This fixes a test failure that was introduced in r104. Because
test_thread and test_clone contain a race condition, the failure only
occurred on some kernel versions (Ubuntu Lucid but not Ubuntu Hardy).
The failure was hidden when the first thread calls __NR_exit_group
before the second trusted thread's __NR_exit handler completed.
The bug was that we were reading memory (with memcpy()) that we had
made unreadable with mprotect()/PROT_NONE.
Fix by bringing reference_trusted_thread.cc into line with the real
implementation. Copy the register data before checking the syscall
number. Also check the sequence number twice.
Review URL: http://codereview.chromium.org/3358021
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@108
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Thu, 9 Sep 2010 17:28:48 +0000 (17:28 +0000)]
Enable support for injecting code into "long NOPs". The gas assembler generates
these instructions when asked to align the .text segment.
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/3311020
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@107
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Thu, 9 Sep 2010 17:27:31 +0000 (17:27 +0000)]
Minor fixes and cleanups:
- allow file accesses when running "make demo"
- mark a returned string as "const". That fixes a compiler warning on some
compilers, telling us not to use a string after it has gone out of scope.
BUG=none
TEST=make demo
Review URL: http://codereview.chromium.org/3298022
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@106
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Thu, 9 Sep 2010 17:14:47 +0000 (17:14 +0000)]
Make patchSystemCallsInFunction static. This allows it to be called without
a particular reference to a library, which is useful if we want to patch
locations in DSOs other than the small number of well-known system libraries.
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/3312016
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@105
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Tue, 7 Sep 2010 21:20:11 +0000 (21:20 +0000)]
Bring reference_trusted_thread.cc into line with real implementation
Only unlock syscall_mutex_ once the syscall has completed. The
exception is __NR_exit, since obviously we are not running after
executing this syscall.
Add missing mprotect() call in __NR_exit handler.
BUG=http://code.google.com/p/seccompsandbox/issues/detail?id=2
TEST="make test"
Review URL: http://codereview.chromium.org/3367015
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@104
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Mon, 6 Sep 2010 19:05:56 +0000 (19:05 +0000)]
Bring linux_syscall_support.h up-to-date and add a feature to optionally
jump to a helper function instead of invoking int0x80/syscall. The latter
is needed when the running inside of the seccomp sandbox.
TEST=depends on other changelists
BUG=37728
Review URL: http://codereview.chromium.org/3347010
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@103
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Mon, 6 Sep 2010 16:08:41 +0000 (16:08 +0000)]
Fix test_prctl so that it does not leave behind a stopped process
The stuck process means that running tests over ssh causes ssh to hang
when the tests are finished, because ssh never gets EOF from stdout.
The problem was apparently that PTRACE_KILL does not kill the
subprocess unless you have cleared the wait status queue for the
subprocess first.
Review URL: http://codereview.chromium.org/3363006
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@102
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Mon, 6 Sep 2010 16:04:41 +0000 (16:04 +0000)]
Add DEPFLAGS to makefile in places missed before
This means changes to sandbox_impl.h will correctly cause
syscall_table.c to be rebuilt.
Also remove HEADERS references in places where a merge re-introduced them.
Review URL: http://codereview.chromium.org/3290010
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@101
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Mon, 6 Sep 2010 09:51:48 +0000 (09:51 +0000)]
Split trusted_thread.cc's asm code into .S files, part 2 of 2
Fix up the code in the .S files to be callable as functions.
Add these files to the makefile.
We put the arch-specific conditionals into a .S file to avoid having
to put this logic into multiple build systems (the makefile and Gyp).
Remove the original copy of the code in trusted_thread.cc.
Remove the throwaway extraction script.
Review URL: http://codereview.chromium.org/1796007
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@100
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Mon, 6 Sep 2010 09:49:41 +0000 (09:49 +0000)]
Split trusted_thread.cc's asm code into .S files, part 1 of 2
This is the first step in extracting the i386/x86-64 assembly code in
trusted_thread.cc into separate .S files.
We add the new .S files containing semi-automatically-extracted code,
with the asm()-quotes removed and register names converted. These
files will be made functional in the next commit. We include the
script that performed the extraction, as well as changes to
trusted_thread.cc that helped with the extraction.
Review URL: http://codereview.chromium.org/1795012
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@99
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Mon, 6 Sep 2010 09:38:33 +0000 (09:38 +0000)]
Add a codereview.settings file
This means reviews will go to the right place by default when using
gcl/git-cl, and reviews will be updated with a ViewVC URL on commit.
Review URL: http://codereview.chromium.org/3322008
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@98
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Fri, 3 Sep 2010 16:57:53 +0000 (16:57 +0000)]
Add #include dependency tracking to the makefile
This ensures that header file changes trigger recompiles. I noticed
that HEADERS was intended to do this but wasn't set right. Use gcc's
-M options because they are more general: This will work if we use
#ifdefs to choose arch-specific .S files in a later change.
Review URL: http://codereview.chromium.org/3346007
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@97
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Thu, 2 Sep 2010 22:10:28 +0000 (22:10 +0000)]
Libc usually enforces alignment on the arguments for socket system calls. We
didn't used to do this. It is unclear whether the kernel requires the alignment
but it doesn't hurt to do so.
BUG=37728
TEST=none
Review URL: http://codereview.chromium.org/3340009
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@96
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Thu, 2 Sep 2010 22:08:17 +0000 (22:08 +0000)]
Add support for prctl(PR_SETDUMPABLE) and a few other benign system calls.
BUG=37728
TEST=run_tests_{32,64} test_prctl
Review URL: http://codereview.chromium.org/3293008
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@95
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Thu, 2 Sep 2010 21:17:12 +0000 (21:17 +0000)]
Add missing -m64 arguments to makefile
Before this change, doing "make test" on a 32-bit system fails when
building the tests rather than when running them, with the unexpected
error:
/usr/bin/ld: i386 architecture of input file `allocator.o64' is
incompatible with i386:x86-64 output
Review URL: http://codereview.chromium.org/3342008
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@94
55e79e8e-603c-11de-8c10-
5fe6993ea61f
zodiac@gmail.com [Thu, 2 Sep 2010 20:37:43 +0000 (20:37 +0000)]
Properly initialize segment registers. This fixes a problem with debug messages
not being available in x86-32 and instead causing a crash.
TEST=run_tests_32 test_debugging
BUG=37728
Review URL: http://codereview.chromium.org/3344003
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@93
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Wed, 1 Sep 2010 14:12:28 +0000 (14:12 +0000)]
Factor out duplicated syscall forwarding logic
Introduce "struct RequestHeader" and forwardSyscall() for sending
syscalls to the trusted process for execution and for receiving the
result.
Review URL: http://codereview.chromium.org/3259007
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@92
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Mon, 30 Aug 2010 18:11:09 +0000 (18:11 +0000)]
Link reference_trusted_thread.cc only into the tests
This file is only for testing purposes, so we stop linking it into the
main seccomp-sandbox library. To avoid using an #if, we now enable
reference_trusted_thread.cc via dependency injection.
In order for createTrustedThread() to have the same type as its
testing counterpart, we remove the two FD arguments and get them via
the existing global variables instead.
Review URL: http://codereview.chromium.org/3287001
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@91
55e79e8e-603c-11de-8c10-
5fe6993ea61f
mseaborn@chromium.org [Fri, 27 Aug 2010 23:12:59 +0000 (23:12 +0000)]
Add Gyp rules for building the test suite
This is from Chromium commit r48043.
Also add a missing source file to seccomp.gyp so that the test builds.
This does not include an action for running the tests, since having
such an action is not common in Chromium's Gyp build.
BUG=none
TEST=seccomp_tests Gyp target
Review URL: http://codereview.chromium.org/3264002
git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@90
55e79e8e-603c-11de-8c10-
5fe6993ea61f