2 years agoMake Maps constructor explicit. master
hans@chromium.org [Wed, 12 Dec 2012 18:14:34 +0000 (18:14 +0000)]
Make Maps constructor explicit.

Review URL: https://codereview.chromium.org/11547014

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@189 55e79e8e-603c-11de-8c10-5fe6993ea61f

2 years agoWork-around for an erroneous warning message in gcc-4.7
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

2 years agoAdd out-of-line copy ctor to Maps::Iterator.
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).


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

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@187 55e79e8e-603c-11de-8c10-5fe6993ea61f

3 years agoI am somewhat reluctantly changing the API so that setProcFD is no longer
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.

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

3 years agoGet rid of a call to dup() that is no longer needed with the way the API works
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
Review URL: https://chromiumcodereview.appspot.com/10399109

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@185 55e79e8e-603c-11de-8c10-5fe6993ea61f

3 years agoChange the sandbox API to require passing in a copy of /proc instead of
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

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

3 years agoPaX-enabled systems require us to mark our executables so that mprotect() doesn't get
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.

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

3 years agoInclude <unistd.h>, as we are otherwise missing definitions on some systems.
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.

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

3 years agoImport <sys/types.h>, as that is the canonical location of "ssize_t".
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".

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

3 years agoMake a small change to the API. Instead of passing in a file
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).

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

3 years agoAdd test for doing syscalls via "int $0" git-svn
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).

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

3 years agoAdd logic for patching calls to the x86-64 vsyscall page
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.

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

3 years agoAdd test for patching a system call instruction
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.


Review URL: http://codereview.chromium.org/8596009

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@177 55e79e8e-603c-11de-8c10-5fe6993ea61f

3 years agoAdd tests for glibc functions which sometimes use the x86-64 vsyscall page
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.

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

3 years agoTests: Split generic test runner out into test_runner.{cc,h}
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.

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

3 years agoTest runner: Make test discovery work within C++ (or C)
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

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.

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

3 years agoAdd a tool for patching ELF libraries/executables offline
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.

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

3 years agoRemove some unused code in the library patcher
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.

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

3 years agoAdd an ELF loader that can load and run an executable in the sandbox
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

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

3 years agoIn 32bit mode, removed all assembly addressing modes that are incompatible with
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.
Review URL: http://codereview.chromium.org/8036047

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@170 55e79e8e-603c-11de-8c10-5fe6993ea61f

3 years agoOriginal changelist: http://codereview.chromium.org/7326013/
zodiac@gmail.com [Fri, 16 Sep 2011 18:43:25 +0000 (18:43 +0000)]
Original changelist: codereview.chromium.org/7326013/

TEST=make test

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@169 55e79e8e-603c-11de-8c10-5fe6993ea61f

3 years agoCommitted clang-related fix, as requested by Nico Weber.
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

3 years agoAllow the ftruncate() system call from within the sandbox.
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

3 years agoAllow writev() and readv() system calls
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().

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

3 years agox86-32: Fix the 6th argument on syscalls invoked via "int $0"
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

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

3 years agoWhile the seccomp sandbox appears to work correctly when built with "clang",
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

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

3 years agoFix build: Add file missed in previous commit, tls_setup.h
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

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

3 years agoImplement TLS initialisation system calls for i386 and x86-64
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

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

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.

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

3 years agoFix the makefile to work after the previous change, r160
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.

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

3 years agoFinally eliminated the dirty hack that we used for populating the system
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.

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

3 years agoMoved complex constructors/destructors out of header files.
zodiac@gmail.com [Mon, 18 Jul 2011 19:16:21 +0000 (19:16 +0000)]
Moved complex constructors/destructors out of header files.

Review URL: http://codereview.chromium.org/7324003

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@159 55e79e8e-603c-11de-8c10-5fe6993ea61f

3 years agoUnconditionally allow p{read,write}{,v}().
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
Review URL: http://codereview.chromium.org/7387002

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@158 55e79e8e-603c-11de-8c10-5fe6993ea61f

3 years agoAssortment of fixes to make the code work with (very recent) versions of
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

3 years agoA couple of changes that make "clang" happier. We still make use of other
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.

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

4 years agoAllow nanosleep() system call, and add a policy setting that allows
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.

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

4 years agoAdd explicit size suffix to ambiguous "cmp" instructions
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.

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

4 years agoEvan Martin asked for another fix in his series of changes to BranchTargets.
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

4 years agoCommitted another iteration of http://codereview.chromium.org/5599002/
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

4 years agoCommitting evan's changelist: http://codereview.chromium.org/5599002/
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

4 years agoGcc as shipped by Fedora 14 prints a bogus warning message when passing
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.

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

4 years agoSplit sandbox.cc's asm code into .S files, part 2 of 2
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

Remove the original copy of the code in sandbox.cc.
Remove the throwaway extraction script.

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

4 years agoSplit sandbox.cc's asm code into .S files, part 1 of 2
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

The new files will be made functional in the next commit.

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

4 years agoOn older 32bit kernels (e.g. Ubuntu Hardy), the seccomp sandbox fails to handle
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.

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

4 years agoStyle: Fix recently-added tabs; convert to spaces
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.

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

4 years agoFactor out duplicated code for checking for protected memory mappings
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().

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

4 years agoTidy: Remove no-op address adjustment in arguments for locked syscalls
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.

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

4 years agoFix deadlock between recvmsg() and sendmsg() on i386
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

The problem did not occur on x86-64 since the recv*() calls pass
arguments in registers there and so use SEND_UNLOCKED.

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

4 years agosendSystemCall(): Change the "locked" parameter from a bool to an enum
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

This change is a pure refactoring:  Some syscalls will be changed to
use SEND_LOCKED_ASYNC in later changes.

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

4 years agoMake sure that the code can be compiled both in release and debug mode.
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.

Review URL: http://codereview.chromium.org/3898005

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@141 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoFactor out duplicated code for setsockopt()/getsockopt()
mseaborn@chromium.org [Tue, 19 Oct 2010 16:48:52 +0000 (16:48 +0000)]
Factor out duplicated code for setsockopt()/getsockopt()

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

4 years agoFix setsockopt() on i386 and add tests for setsockopt()/getsockopt()
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.

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

4 years agoAllow fdatasync() inside of the sandbox. This is needed by SQLite.
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
Review URL: http://codereview.chromium.org/3796012

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@138 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoDebug mode: Remove "%gs == 0" check which is always true on x86-64
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.

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

4 years agoTest runner: Set a limit on stack size to avoid accidental OOM
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.

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

4 years agoUse one mutex per thread instead of a global syscall mutex
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.

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

4 years agoChange clone() to be an unlocked system call
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.

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

4 years agoReplace assert() with our own CHECK() macro(s). This means we no longer have
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
Review URL: http://codereview.chromium.org/3444013

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@133 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agotest_syscalls.cc: Fix use of futex() when waiting for thread
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:
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

4 years agoRemove cloneFdPub from SecureMem::Args and use the global variable instead
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
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

4 years agoAlways #include foo.h at the top of foo.cc
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

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

4 years agoRemove unused processFdPub from SecureMem::Args
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:
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

4 years agoWhitespace fixes.
zodiac@gmail.com [Fri, 1 Oct 2010 16:10:35 +0000 (16:10 +0000)]
Whitespace fixes.

Review URL: http://codereview.chromium.org/3602006

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@128 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoRefactor the code used to communicate from the trusted process to the other
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
Review URL: http://codereview.chromium.org/3414016

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@127 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoMake error messages look more like what the non-reference version of the code
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.

Review URL: http://codereview.chromium.org/3574004

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@126 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoFix vulnerability in process_sigaction()
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

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

4 years agoFix hole that allows tampering with sendmsg() arguments
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.

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

4 years agoTrusted thread: Add error checks to all syscalls that should return zero
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

4 years agoclone(): Copy registers across in untrusted code instead of trusted code
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

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

4 years agoAdd test to check that clone() preserves registers
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

4 years agoAdd test for SysV shared memory
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

4 years agoBecause of the way how we build our test applications, we can get name
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

Review URL: http://codereview.chromium.org/3392001

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@119 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoclone(): Rename arguments that were only named correctly for x86-64
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

4 years agoreference_trusted_thread.cc: Get clone() args from argN instead of saved regs
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

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

4 years agoUse __NR_* symbols in .S files instead of hard-coded constants
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.

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

4 years agoA few small changes that are needed in order to build with very recent
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

4 years agoreference_trusted_thread.cc: Use sigreturn to return from clone()
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

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

4 years agoFix use of non-null-terminated strings in debug messages
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.

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

4 years agoRename label 25 to "fatal_error" to improve readability
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

4 years agoRemove some more dependency files from "make clean" that had previously been
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.

Review URL: http://codereview.chromium.org/3322019

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@111 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years ago- push a fake return address and set a magic framepointer whenever
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.

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

4 years agoFix newlines in string literals in the assembly code
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

4 years agoFix segfault in reference_trusted_thread.cc's __NR_exit handler
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

4 years agoEnable support for injecting code into "long NOPs". The gas assembler generates
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.

Review URL: http://codereview.chromium.org/3311020

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@107 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoMinor fixes and cleanups:
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.

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

4 years agoMake patchSystemCallsInFunction static. This allows it to be called without
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.

Review URL: http://codereview.chromium.org/3312016

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@105 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoBring reference_trusted_thread.cc into line with real implementation
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.

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

4 years agoBring linux_syscall_support.h up-to-date and add a feature to optionally
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
Review URL: http://codereview.chromium.org/3347010

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@103 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoFix test_prctl so that it does not leave behind a stopped process
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

4 years agoAdd DEPFLAGS to makefile in places missed before
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

4 years agoSplit trusted_thread.cc's asm code into .S files, part 2 of 2
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

4 years agoSplit trusted_thread.cc's asm code into .S files, part 1 of 2
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

4 years agoAdd a codereview.settings file
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

4 years agoAdd #include dependency tracking to the makefile
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

4 years agoLibc usually enforces alignment on the arguments for socket system calls. We
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.

Review URL: http://codereview.chromium.org/3340009

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@96 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoAdd support for prctl(PR_SETDUMPABLE) and a few other benign system calls.
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.

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

4 years agoAdd missing -m64 arguments to makefile
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

/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

4 years agoProperly initialize segment registers. This fixes a problem with debug messages
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
Review URL: http://codereview.chromium.org/3344003

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@93 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoFactor out duplicated syscall forwarding logic
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

Review URL: http://codereview.chromium.org/3259007

git-svn-id: http://seccompsandbox.googlecode.com/svn/trunk@92 55e79e8e-603c-11de-8c10-5fe6993ea61f

4 years agoLink reference_trusted_thread.cc only into the tests
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

4 years agoAdd Gyp rules for building the test suite
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.

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