Fix logic for checking chrome-sandbox setuid binary

Fix logic for checking chrome-sandbox setuid binary

We only need to check for chrome-sandbox if we plan on launching
zygotes with the setuid sandbox or we need it to adjust OOM scores.
Also, the error only needs to be fatal when we want the setuid
sandbox.

BUG=598454
Committed: https://crrev.com/5ba7c75f39ad5e4f6bf67efb9f8d0b84355d4c96
Cr-Commit-Position: refs/heads/master@{#399361}

[mdempsky, 2016-05-13 23:32:44]

mdempsky@chromium.org changed reviewers:
+ rickyz@chromium.org, thomasanderson@chromium.org

[mdempsky, 2016-05-13 23:32:45]

rickyz: PTAL. Confirmed that this seems to work for me, and I think the new
logic is much clearer than what we had before.

thomasanderson: Please confirm that this works for you too.

dpranke/kerrnel/jln: FYI

[Tom (Use chromium acct), 2016-05-16 16:23:31]

On 2016/05/13 23:32:45, mdempsky wrote:
> rickyz: PTAL. Confirmed that this seems to work for me, and I think the new
> logic is much clearer than what we had before.
> 
> thomasanderson: Please confirm that this works for you too.
> 
> dpranke/kerrnel/jln: FYI

lgtm, and patch works locally

[mdempsky, 2016-05-18 19:12:50]

rickyz/jln: ping?

[rickyz (no longer on Chrome), 2016-05-20 23:41:43]

rickyz@chromium.org changed reviewers:
+ thomasanderson@google.com

[rickyz (no longer on Chrome), 2016-05-20 23:41:44]

lgtm

This is much nicer, thanks!

https://codereview.chromium.org/1976403002/diff/40001/content/browser/zygote_...
File content/browser/zygote_host/zygote_host_impl_linux.cc (right):

https://codereview.chromium.org/1976403002/diff/40001/content/browser/zygote_...
content/browser/zygote_host/zygote_host_impl_linux.cc:34: char buf[expect_len +
1];
Didn't originate in your change, but I wonder why there was ever a + 1 here.

[mdempsky, 2016-05-21 00:05:07]

https://codereview.chromium.org/1976403002/diff/40001/content/browser/zygote_...
File content/browser/zygote_host/zygote_host_impl_linux.cc (right):

https://codereview.chromium.org/1976403002/diff/40001/content/browser/zygote_...
content/browser/zygote_host/zygote_host_impl_linux.cc:34: char buf[expect_len +
1];
On 2016/05/20 23:41:44, rickyz wrote:
> Didn't originate in your change, but I wonder why there was ever a + 1 here.

With framed-message socket types like SOCK_DATAGRAM and SOCK_SEQPACKET, recvmsg
truncates the messages to fit the buffer. If we had allocated the buffer to be
exactly expect_len bytes, then we wouldn't be able to detect whether:

  1) the sender sent us exactly expect_len bytes, or
  2) the sender sent us more than expect_len bytes, but recvmsg truncated it to
expect_len bytes.

By allocating an extra byte, we're able to reliably distinguish those two cases.

Added a comment to explain.

[mdempsky, 2016-05-21 00:05:16]

The CQ bit was checked by mdempsky@chromium.org

[mdempsky, 2016-05-21 00:05:16]

The patchset sent to the CQ was uploaded after l-g-t-m from rickyz@chromium.org,
thomasanderson@google.com
Link to the patchset: https://codereview.chromium.org/1976403002/#ps60001
(title: "rickyz feedback")

[commit-bot: I haz the power, 2016-05-21 00:05:38]

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

[commit-bot: I haz the power, 2016-05-21 00:12:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-21 00:12:32]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[mdempsky, 2016-05-21 00:14:50]

Description was changed from

==========
Fix logic for checking chrome-sandbox setuid binary

We only need to check for chrome-sandbox if we plan on launching
zygotes with the setuid sandbox or we need it to adjust OOM scores.
Also, the error only needs to be fatal when we want the setuid
sandbox.

BUG=598454
==========

to

==========
Fix logic for checking chrome-sandbox setuid binary

We only need to check for chrome-sandbox if we plan on launching
zygotes with the setuid sandbox or we need it to adjust OOM scores.
Also, the error only needs to be fatal when we want the setuid
sandbox.

BUG=598454
==========

[mdempsky, 2016-05-21 00:15:28]

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

[mdempsky, 2016-05-21 00:15:28]

avi: PTAL at content/browser/browser_main_loop.cc for content/OWNERS.

[mdempsky, 2016-06-02 22:20:09]

On 2016/05/21 00:15:28, mdempsky wrote:
> avi: PTAL at content/browser/browser_main_loop.cc for content/OWNERS.

avi: Ping?

[Avi (use Gerrit), 2016-06-02 22:31:57]

LGTM stamp.

Feel free to ping me after two or three days.

[mdempsky, 2016-06-02 22:33:40]

On 2016/06/02 22:31:57, Avi wrote:
> LGTM stamp.

Thanks!

> Feel free to ping me after two or three days.

Yeah, I was out for jury service all last week and forgot about this CL. :)

[mdempsky, 2016-06-02 22:33:51]

The CQ bit was checked by mdempsky@chromium.org

[commit-bot: I haz the power, 2016-06-02 22:35:52]

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

[commit-bot: I haz the power, 2016-06-02 23:50:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-02 23:50:40]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Tom (Use chromium acct), 2016-06-11 00:11:18]

On 2016/06/02 23:50:40, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

Can we land this?  I reran mac_chromium_rel_ng which is passing now

[mdempsky, 2016-06-11 02:40:44]

The CQ bit was checked by mdempsky@chromium.org

[commit-bot: I haz the power, 2016-06-11 02:40:59]

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

[mdempsky, 2016-06-11 02:43:31]

On 2016/06/11 00:11:18, Tom Anderson wrote:
> Can we land this?  I reran mac_chromium_rel_ng which is passing now

Sorry, didn't notice the CQ flaked the first attempt.  Yes, I think this can and
should be landed.  Feel free to re-CQ as needed, or ping me again if it needs to
be manually landed.

[commit-bot: I haz the power, 2016-06-11 03:40:19]

Description was changed from

==========
Fix logic for checking chrome-sandbox setuid binary

We only need to check for chrome-sandbox if we plan on launching
zygotes with the setuid sandbox or we need it to adjust OOM scores.
Also, the error only needs to be fatal when we want the setuid
sandbox.

BUG=598454
==========

to

==========
Fix logic for checking chrome-sandbox setuid binary

We only need to check for chrome-sandbox if we plan on launching
zygotes with the setuid sandbox or we need it to adjust OOM scores.
Also, the error only needs to be fatal when we want the setuid
sandbox.

BUG=598454
==========

[commit-bot: I haz the power, 2016-06-11 03:40:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-11 03:40:23]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-11 03:42:19]

Description was changed from

==========
Fix logic for checking chrome-sandbox setuid binary

We only need to check for chrome-sandbox if we plan on launching
zygotes with the setuid sandbox or we need it to adjust OOM scores.
Also, the error only needs to be fatal when we want the setuid
sandbox.

BUG=598454
==========

to

==========
Fix logic for checking chrome-sandbox setuid binary

We only need to check for chrome-sandbox if we plan on launching
zygotes with the setuid sandbox or we need it to adjust OOM scores.
Also, the error only needs to be fatal when we want the setuid
sandbox.

BUG=598454

Committed: https://crrev.com/5ba7c75f39ad5e4f6bf67efb9f8d0b84355d4c96
Cr-Commit-Position: refs/heads/master@{#399361}
==========

[commit-bot: I haz the power, 2016-06-11 03:42:20]

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