sandbox arm64: align stack 16 bytes

sandbox arm64: align stack 16 bytes

chrome fails to start with user namespace sandboxing:

[866:866:0120/134742:FATAL:zygote_host_impl_linux.cc(182)] Check failed: process.IsValid().
Failed to launch zygote process

With strace..

clone(child_stack=0x7ffd28fd98, flags=CLONE_NEWUSER|CLONE_NEWPID|CLONE_NEWNET|SIGCHLD) = -1 EINVAL (Invalid argument)

Which compared to kernel sources[1], shows that stack needs to be 16
bytes aligned. because stack grows downward, this patch assumes
PTHREAD_STACK_MIN is dividable by 16 too.

[1] http://lxr.free-electrons.com/source/arch/arm64/kernel/process.c#L267

BUG=581018
R=keescook@chromium.org,jln@chromium.org,rsesek@chromium.org,thakis@chromium.org
TEST=base_unittests ProcessUtilTest.* and sandbox_linux_unittests NamespaceSandboxTest.*

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

[Kees Cook, 2016-01-21 19:25:48]

lgtm

[Nico, 2016-01-21 19:39:00]

lgtm

(is there any harm in always aligning this?)

[rickyz (no longer on Chrome), 2016-01-22 04:19:46]

rickyz@chromium.org changed reviewers:
+ rickyz@chromium.org

[rickyz (no longer on Chrome), 2016-01-22 04:19:47]

lgtm as well, and +1 to just always aligning it.

[Riku Voipio, 2016-01-25 13:29:19]

Description was changed from

==========
sandbox arm64: align stack 16 bytes

chrome fails to start with user namespace sandboxing:

[866:866:0120/134742:FATAL:zygote_host_impl_linux.cc(182)] Check failed:
process.IsValid().
Failed to launch zygote process

With strace..

clone(child_stack=0x7ffd28fd98,
flags=CLONE_NEWUSER|CLONE_NEWPID|CLONE_NEWNET|SIGCHLD) = -1 EINVAL (Invalid
argument)

Which compared to kernel sources[1], shows that stack needs to be 16
bytes aligned. because stack grows downward, this patch assumes
PTHREAD_STACK_MIN is dividable by 16 too.

[1] http://lxr.free-electrons.com/source/arch/arm64/kernel/process.c#L267

BUG=
R=keescook@chromium.org,jln@chromium.org,rsesek@chromium.org,thakis@chromium.org
TEST=base_unittests ProcessUtilTest.* and sandbox_linux_unittests
NamespaceSandboxTest.*
==========

to

==========
sandbox arm64: align stack 16 bytes

chrome fails to start with user namespace sandboxing:

[866:866:0120/134742:FATAL:zygote_host_impl_linux.cc(182)] Check failed:
process.IsValid().
Failed to launch zygote process

With strace..

clone(child_stack=0x7ffd28fd98,
flags=CLONE_NEWUSER|CLONE_NEWPID|CLONE_NEWNET|SIGCHLD) = -1 EINVAL (Invalid
argument)

Which compared to kernel sources[1], shows that stack needs to be 16
bytes aligned. because stack grows downward, this patch assumes
PTHREAD_STACK_MIN is dividable by 16 too.

[1] http://lxr.free-electrons.com/source/arch/arm64/kernel/process.c#L267

BUG=581018
R=keescook@chromium.org,jln@chromium.org,rsesek@chromium.org,thakis@chromium.org
TEST=base_unittests ProcessUtilTest.* and sandbox_linux_unittests
NamespaceSandboxTest.*
==========

[Riku Voipio, 2016-01-25 13:30:22]

v2 patch with 16 aligned everywhere.

[Nico, 2016-01-25 15:13:24]

lgtm

https://codereview.chromium.org/1617763002/diff/20001/base/process/launch_pos...
File base/process/launch_posix.cc (right):

https://codereview.chromium.org/1617763002/diff/20001/base/process/launch_pos...
base/process/launch_posix.cc:739: char stack_buf[PTHREAD_STACK_MIN]
__attribute__((aligned(16)));
If you want you can spell this ALIGNAS(16) (from base/compiler_specific.h) too;
up to you

[Riku Voipio, 2016-01-27 13:59:55]

On 2016/01/25 15:13:24, Nico wrote:
> lgtm
> 
>
https://codereview.chromium.org/1617763002/diff/20001/base/process/launch_pos...
> File base/process/launch_posix.cc (right):
> 
>
https://codereview.chromium.org/1617763002/diff/20001/base/process/launch_pos...
> base/process/launch_posix.cc:739: char stack_buf[PTHREAD_STACK_MIN]
> __attribute__((aligned(16)));
> If you want you can spell this ALIGNAS(16) (from base/compiler_specific.h)
too;
> up to you

updated, and added reviewer for the base/process/launch_posix.cc as well.

[Nico, 2016-01-27 14:44:25]

base/ still lgtm

[Riku Voipio, 2016-01-27 15:22:24]

The CQ bit was checked by riku.voipio@linaro.org

[Riku Voipio, 2016-01-27 15:22:24]

The patchset sent to the CQ was uploaded after l-g-t-m from rickyz@chromium.org,
keescook@chromium.org
Link to the patchset: https://codereview.chromium.org/1617763002/#ps40001
(title: "v3: changed to using ALIGNAS(16)")

[commit-bot: I haz the power, 2016-01-27 15:22:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1617763002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1617763002/40001

[commit-bot: I haz the power, 2016-01-27 17:14:03]

Description was changed from

==========
sandbox arm64: align stack 16 bytes

chrome fails to start with user namespace sandboxing:

[866:866:0120/134742:FATAL:zygote_host_impl_linux.cc(182)] Check failed:
process.IsValid().
Failed to launch zygote process

With strace..

clone(child_stack=0x7ffd28fd98,
flags=CLONE_NEWUSER|CLONE_NEWPID|CLONE_NEWNET|SIGCHLD) = -1 EINVAL (Invalid
argument)

Which compared to kernel sources[1], shows that stack needs to be 16
bytes aligned. because stack grows downward, this patch assumes
PTHREAD_STACK_MIN is dividable by 16 too.

[1] http://lxr.free-electrons.com/source/arch/arm64/kernel/process.c#L267

BUG=581018
R=keescook@chromium.org,jln@chromium.org,rsesek@chromium.org,thakis@chromium.org
TEST=base_unittests ProcessUtilTest.* and sandbox_linux_unittests
NamespaceSandboxTest.*
==========

to

==========
sandbox arm64: align stack 16 bytes

chrome fails to start with user namespace sandboxing:

[866:866:0120/134742:FATAL:zygote_host_impl_linux.cc(182)] Check failed:
process.IsValid().
Failed to launch zygote process

With strace..

clone(child_stack=0x7ffd28fd98,
flags=CLONE_NEWUSER|CLONE_NEWPID|CLONE_NEWNET|SIGCHLD) = -1 EINVAL (Invalid
argument)

Which compared to kernel sources[1], shows that stack needs to be 16
bytes aligned. because stack grows downward, this patch assumes
PTHREAD_STACK_MIN is dividable by 16 too.

[1] http://lxr.free-electrons.com/source/arch/arm64/kernel/process.c#L267

BUG=581018
R=keescook@chromium.org,jln@chromium.org,rsesek@chromium.org,thakis@chromium.org
TEST=base_unittests ProcessUtilTest.* and sandbox_linux_unittests
NamespaceSandboxTest.*
==========

[commit-bot: I haz the power, 2016-01-27 17:14:04]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-27 17:15:06]

Description was changed from

==========
sandbox arm64: align stack 16 bytes

chrome fails to start with user namespace sandboxing:

[866:866:0120/134742:FATAL:zygote_host_impl_linux.cc(182)] Check failed:
process.IsValid().
Failed to launch zygote process

With strace..

clone(child_stack=0x7ffd28fd98,
flags=CLONE_NEWUSER|CLONE_NEWPID|CLONE_NEWNET|SIGCHLD) = -1 EINVAL (Invalid
argument)

Which compared to kernel sources[1], shows that stack needs to be 16
bytes aligned. because stack grows downward, this patch assumes
PTHREAD_STACK_MIN is dividable by 16 too.

[1] http://lxr.free-electrons.com/source/arch/arm64/kernel/process.c#L267

BUG=581018
R=keescook@chromium.org,jln@chromium.org,rsesek@chromium.org,thakis@chromium.org
TEST=base_unittests ProcessUtilTest.* and sandbox_linux_unittests
NamespaceSandboxTest.*
==========

to

==========
sandbox arm64: align stack 16 bytes

chrome fails to start with user namespace sandboxing:

[866:866:0120/134742:FATAL:zygote_host_impl_linux.cc(182)] Check failed:
process.IsValid().
Failed to launch zygote process

With strace..

clone(child_stack=0x7ffd28fd98,
flags=CLONE_NEWUSER|CLONE_NEWPID|CLONE_NEWNET|SIGCHLD) = -1 EINVAL (Invalid
argument)

Which compared to kernel sources[1], shows that stack needs to be 16
bytes aligned. because stack grows downward, this patch assumes
PTHREAD_STACK_MIN is dividable by 16 too.

[1] http://lxr.free-electrons.com/source/arch/arm64/kernel/process.c#L267

BUG=581018
R=keescook@chromium.org,jln@chromium.org,rsesek@chromium.org,thakis@chromium.org
TEST=base_unittests ProcessUtilTest.* and sandbox_linux_unittests
NamespaceSandboxTest.*

Committed: https://crrev.com/f352d974e43a73fed311c60c8fcb4dd043b16093
Cr-Commit-Position: refs/heads/master@{#371809}
==========

[commit-bot: I haz the power, 2016-01-27 17:15:07]

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