Reland: Introduce sys_sigprocmask and sys_sigaction. (patchset #5 id:160001 of https://codereview.c…

Reland: Introduce sys_sigprocmask and sys_sigaction. (patchset #5 id:160001 of https://codereview.chromium.org/1077143002/)

This CL is relanding the CL https://codereview.chromium.org/1077143002/,
which was reverted due to false alarm of the memory sanitizer.
Because the memory sanitizer does not know about direct kernel syscall
does, so it warns the arguments of the signal handler, unexpectedly.
This CL adds the msan unpoisoning to the arguemtns, so that the memory
sanitizer should understand it.

TEST=Ran sandbox_linux_unittests with msan=1. Ran bots.
BUG=358465
CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:linux_chromium_trusty32_rel,linux_arm

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

[hidehiko, 2015-04-21 06:07:18]

Patchset #1 (id:1) has been deleted

[hidehiko, 2015-04-21 14:15:18]

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

[hidehiko, 2015-04-21 14:15:19]

Trying to reland.
PTAL.

NOTE: msan error is reproduced on my local env, and it is fixed at PS2. PS1 is
the original CL.

Thank you for your review in advance,
- hidehiko

[mdempsky, 2015-04-21 17:37:33]

lgtm

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
File sandbox/linux/seccomp-bpf/trap.cc (right):

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf/trap.cc:125: if (info) MSAN_UNPOISON(info,
sizeof(*info));
nit: in sandbox, the prevailing convention is to always use

  if (foo) {
    bar();
  }

even for simple code like this.

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
File sandbox/linux/seccomp-bpf/trap_unittest.cc (right):

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf/trap_unittest.cc:23: // Note that, SigSysAction
handler would outputs an error message
nit: "would output"

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf/trap_unittest.cc:25: kill(getpid(), SIGSYS);
You can just use raise(SIGSYS) and remove the <sys/types.h> header.

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf/trap_unittest.cc:28: }
nit:  "}  // namespace"

[hidehiko, 2015-04-22 06:54:37]

Thank you for review. Submitting.

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
File sandbox/linux/seccomp-bpf/trap.cc (right):

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf/trap.cc:125: if (info) MSAN_UNPOISON(info,
sizeof(*info));
On 2015/04/21 17:37:33, mdempsky wrote:
> nit: in sandbox, the prevailing convention is to always use
> 
>   if (foo) {
>     bar();
>   }
> 
> even for simple code like this.

Done, though I borrowed this style from syscall_wrappers.cc. Maybe it's better
to standardize later?

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
File sandbox/linux/seccomp-bpf/trap_unittest.cc (right):

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf/trap_unittest.cc:23: // Note that, SigSysAction
handler would outputs an error message
On 2015/04/21 17:37:33, mdempsky wrote:
> nit: "would output"

Done.

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf/trap_unittest.cc:25: kill(getpid(), SIGSYS);
On 2015/04/21 17:37:33, mdempsky wrote:
> You can just use raise(SIGSYS) and remove the <sys/types.h> header.

Done.

https://codereview.chromium.org/1092153005/diff/40001/sandbox/linux/seccomp-b...
sandbox/linux/seccomp-bpf/trap_unittest.cc:28: }
On 2015/04/21 17:37:33, mdempsky wrote:
> nit:  "}  // namespace"

Done.

[hidehiko, 2015-04-22 06:54:44]

The CQ bit was checked by hidehiko@chromium.org

[hidehiko, 2015-04-22 06:54:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
mdempsky@chromium.org
Link to the patchset: https://codereview.chromium.org/1092153005/#ps80001
(title: " ")

[commit-bot: I haz the power, 2015-04-22 06:55:00]

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

[commit-bot: I haz the power, 2015-04-22 11:17:15]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-04-22 11:18:16]

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

[magjed_chromium, 2015-04-22 13:00:59]

A revert of this CL (patchset #4 id:80001) has been created in
https://codereview.chromium.org/1102543003/ by magjed@chromium.org.

The reason for reverting is: Causing Linux Tests to fail:
http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29...

sandbox_linux_unittests Trap.SigSysAction failed 14
failures:
Syscall.SyntheticSixArgs
SandboxBPF.PthreadEquality
SandboxBPF.Pread64
SandboxBPF.UnsafeTrapWithErrno
SandboxBPF.ForwardSyscall
SandboxBPF.BasicBlacklistWithSigsys
SandboxBPF.SigBus
Trap.SigSysAction
SandboxBPF.VerboseAPITesting
ParameterRestrictions.sched_getparam_allowed
SandboxBPF.UnsafeTrapWithCond
SandboxBPF.UseOpenBroker
SandboxBPF.GreyListedPolicy
SandboxBPF.PthreadBitMask

[ RUN      ] SandboxBPF.VerboseAPITesting
../../sandbox/linux/tests/unit_tests.cc:234: Failure
Value of: subprocess_terminated_normally
  Actual: false
Expected: true.

[hidehiko, 2015-04-22 16:37:44]

On 2015/04/22 13:00:59, magjed wrote:
> A revert of this CL (patchset #4 id:80001) has been created in
> https://codereview.chromium.org/1102543003/ by mailto:magjed@chromium.org.
> 
> The reason for reverting is: Causing Linux Tests to fail:
>
http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29...
> 
> sandbox_linux_unittests Trap.SigSysAction failed 14
> failures:
> Syscall.SyntheticSixArgs
> SandboxBPF.PthreadEquality
> SandboxBPF.Pread64
> SandboxBPF.UnsafeTrapWithErrno
> SandboxBPF.ForwardSyscall
> SandboxBPF.BasicBlacklistWithSigsys
> SandboxBPF.SigBus
> Trap.SigSysAction
> SandboxBPF.VerboseAPITesting
> ParameterRestrictions.sched_getparam_allowed
> SandboxBPF.UnsafeTrapWithCond
> SandboxBPF.UseOpenBroker
> SandboxBPF.GreyListedPolicy
> SandboxBPF.PthreadBitMask
> 
> [ RUN      ] SandboxBPF.VerboseAPITesting
> ../../sandbox/linux/tests/unit_tests.cc:234: Failure
> Value of: subprocess_terminated_normally
>   Actual: false
> Expected: true.

Ah... my bad... Sorry for inconvenience.
Matthew, I found the root cause, so please let me send another code review
request...

Regards,
- hidehiko

[mdempsky, 2015-04-22 19:57:23]

On 2015/04/22 16:37:44, hidehiko wrote:
> Matthew, I found the root cause, so please let me send another code review
> request...

SGTM, go ahead :)