Reland: Introduce sys_sigprocmask and sys_sigaction. (patchset #4 id:80001 of https://codereview.ch…

Reland: Introduce sys_sigprocmask and sys_sigaction. (patchset #4 id:80001 of https://codereview.chromium.org/1092153005/)

The last version was broken for x86_64 DEBUG build.
On x86_64, it is necessary to set sa_restorer appropriately,
if not specified. What we need in the function is just invoking
the rt_sigreturn system call. However, it is special system call
which touches user land stack, so that a function prologue will
break the program.
In RELEASE build, that prologue is gone (including syscall() invocation)
by optimization. However, in DEBUG build, there is the prologue.
Unfortunately, all CQ and trybot do not run DEBUG tests so that it
was not catched.

Also, it turned out that MSAN does not work with direct sigaction syscall, unfortunately. (In more precise, it does not work if we *mix* direct sigaction syscall and sigaction() in libc).

This version fixes them.

TEST=Ran sandbox_linux_tests with DEBUG build locally. Ran msan tests, too. Ran trybots including 64 debug bot and msan ones.
BUG=358465
CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:linux_chromium_trusty32_rel,linux_arm,linux_chromium_trusty_dbg,linux_chromium_dbg_ng,linux_chromium_msan_rel_ng

Committed: https://crrev.com/a926d0b8cd3d2a3175ef518d8987c7511b083303
Cr-Commit-Position: refs/heads/master@{#326893}

[hidehiko, 2015-04-23 10:33:53]

Patchset #4 (id:60001) has been deleted

[hidehiko, 2015-04-23 10:34:06]

Patchset #3 (id:40001) has been deleted

[hidehiko, 2015-04-23 10:34:16]

Patchset #2 (id:20001) has been deleted

[hidehiko, 2015-04-23 15:59:55]

hidehiko@chromium.org changed reviewers:
+ mdempsky@chromium.org

[hidehiko, 2015-04-23 15:59:56]

Hi Matthew,

Third trial... Sorry for inconvenience.
Now DEBUG build and MSAN passed successfully...
PS1 is the original CL, and diff between PS1 and PS2 is the new code.

Thank you for review in advance,
- hidehiko

https://codereview.chromium.org/1099263003/diff/1/sandbox/linux/seccomp-bpf/t...
File sandbox/linux/seccomp-bpf/trap.cc (right):

https://codereview.chromium.org/1099263003/diff/1/sandbox/linux/seccomp-bpf/t...
sandbox/linux/seccomp-bpf/trap.cc:125: if (info) {
Note: I temporarily reverted this part, because we starts to use sigaction() in
libc if MSAN is enabled. This will be back when we start to compile this with
PNaCl toolchain actually. Please see my comment/plan in syscall_wrappers.cc.

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/BUILD.gn
File sandbox/linux/BUILD.gn (right):

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/BUILD.gn#...
sandbox/linux/BUILD.gn:120: "seccomp-bpf/trap_unittest.cc",
Note: this was missing in the previous CL...

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/services/...
File sandbox/linux/services/syscall_wrappers.cc (right):

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/services/...
sandbox/linux/services/syscall_wrappers.cc:151: #if defined(MEMORY_SANITIZER)
Note: I'll add (|| !defined(OS_NACL_NONSFI)) in a later CL to this condition
(i.e., with PNaCl toolchain we need to use direct system call).
This is because:
- The signal ABIs provided by PNaCl toolchian is incompatible with Linux's. So,
we always need to use direct syscall. In other words, sigaction() in libc should
not be called while the program runs.
- It is for nacl_helper in Non-SFI mode, so practically the problem described
below does not happen.
- In addition, we need to revive the MSAN unpoisonize in Trap::SigSysAction,
because the callback will not be wrapped as MSAN does.

[mdempsky, 2015-04-23 19:39:36]

lgtm

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/services/...
File sandbox/linux/services/syscall_wrappers.cc (right):

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/services/...
sandbox/linux/services/syscall_wrappers.cc:151: #if defined(MEMORY_SANITIZER)
On 2015/04/23 15:59:56, hidehiko wrote:
> Note: I'll add (|| !defined(OS_NACL_NONSFI)) in a later CL to this condition
> (i.e., with PNaCl toolchain we need to use direct system call).

Is it an option to just add a

  #if defined(OS_NACL_NONSFI)
  #error MSAN and non-SFI are incompatible
  #endif

check in here?  Or do we require the ability to compile the non-SFI code with
MSAN?

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/services/...
sandbox/linux/services/syscall_wrappers.cc:185: static __attribute__((naked))
void sys_rt_sigreturn() {
Hm, the GCC manual says "This attribute is available on the ARM, AVR, MCORE,
MSP430, NDS32, RL78, RX and SPU ports."  But I don't see any reason it shouldn't
work on x86-64 too.

[hidehiko, 2015-04-24 17:35:01]

Patchset #4 (id:120001) has been deleted

[hidehiko, 2015-04-24 17:38:38]

Thank you for review.
PTAL, just in case.

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/services/...
File sandbox/linux/services/syscall_wrappers.cc (right):

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/services/...
sandbox/linux/services/syscall_wrappers.cc:151: #if defined(MEMORY_SANITIZER)
On 2015/04/23 19:39:36, mdempsky wrote:
> On 2015/04/23 15:59:56, hidehiko wrote:
> > Note: I'll add (|| !defined(OS_NACL_NONSFI)) in a later CL to this condition
> > (i.e., with PNaCl toolchain we need to use direct system call).
> 
> Is it an option to just add a
> 
>   #if defined(OS_NACL_NONSFI)
>   #error MSAN and non-SFI are incompatible
>   #endif
> 
> check in here?  Or do we require the ability to compile the non-SFI code with
> MSAN?

I think we can use ifdef as you said here. Please let me check it in the later
CL.

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/services/...
sandbox/linux/services/syscall_wrappers.cc:185: static __attribute__((naked))
void sys_rt_sigreturn() {
On 2015/04/23 19:39:36, mdempsky wrote:
> Hm, the GCC manual says "This attribute is available on the ARM, AVR, MCORE,
> MSP430, NDS32, RL78, RX and SPU ports."  But I don't see any reason it
shouldn't
> work on x86-64 too.

Good catch! GCC does not support naked, but clang does.
Fixed GCC build by falling back to libc's sigaction(), as there seems no simple
way to define naked function with GCC (or need huge inline asm code).

[mdempsky, 2015-04-24 18:03:02]

The CQ bit was checked by mdempsky@chromium.org

[mdempsky, 2015-04-24 18:03:02]

lgtm

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/services/...
File sandbox/linux/services/syscall_wrappers.cc (right):

https://codereview.chromium.org/1099263003/diff/80001/sandbox/linux/services/...
sandbox/linux/services/syscall_wrappers.cc:151: #if defined(MEMORY_SANITIZER)
On 2015/04/24 17:38:38, hidehiko wrote:
> I think we can use ifdef as you said here. Please let me check it in the later
> CL.

Sure.

https://codereview.chromium.org/1099263003/diff/140001/sandbox/linux/services...
File sandbox/linux/services/syscall_wrappers.cc (right):

https://codereview.chromium.org/1099263003/diff/140001/sandbox/linux/services...
sandbox/linux/services/syscall_wrappers.cc:152: (defined(ARCH_CPU_X86_64) &&
defined(__GNUC__) && !defined(__clang__))
I'd probably drop the defined(__GNUC__).  Theoretically if we compiled this code
with some other compiler, we wouldn't a priori know that naked works with it
either.

But since currently there's only GCC and Clang at the moment, I'm fine with
deferring that to a followup CL too.

[commit-bot: I haz the power, 2015-04-24 18:05:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1099263003/140001

[commit-bot: I haz the power, 2015-04-24 21:55:28]

Committed patchset #4 (id:140001)

[commit-bot: I haz the power, 2015-04-24 21:56:48]

Patchset 4 (id:??) landed as
https://crrev.com/a926d0b8cd3d2a3175ef518d8987c7511b083303
Cr-Commit-Position: refs/heads/master@{#326893}