New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 653930 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit 16 days ago
Closed: Nov 2016
EstimatedDays: ----
NextAction: ----
OS: Linux , Chrome
Pri: 3
Type: Bug



Sign in to add a comment

MessageLoopLibevent opens a pipe without CLOEXEC

Project Member Reported by lhchavez@chromium.org, Oct 7 2016

Issue description

This causes child processes to be able to access these fds and do annoying things, like spuriously wake up a parent's message loop.
 
This is the public version of b/32005517.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9c8562e55b669915faf87d8d7eda32ae62bf661

commit f9c8562e55b669915faf87d8d7eda32ae62bf661
Author: lhchavez <lhchavez@chromium.org>
Date: Fri Oct 07 17:46:21 2016

Open MessagePumpLibevent's pipe with O_CLOEXEC

This prevents the pipe's fds from being leaked into child processes.

TEST=git cl try
BUG= 653930 

Review-Url: https://codereview.chromium.org/2394413002
Cr-Commit-Position: refs/heads/master@{#423899}

[modify] https://crrev.com/f9c8562e55b669915faf87d8d7eda32ae62bf661/base/files/file_util.h
[modify] https://crrev.com/f9c8562e55b669915faf87d8d7eda32ae62bf661/base/files/file_util_posix.cc
[modify] https://crrev.com/f9c8562e55b669915faf87d8d7eda32ae62bf661/base/message_loop/message_pump_libevent.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c231da96b4586157979d9e2ec41561b7b115b2e2

commit c231da96b4586157979d9e2ec41561b7b115b2e2
Author: nasko <nasko@chromium.org>
Date: Fri Oct 07 21:17:41 2016

Revert of Open MessagePumpLibevent's pipe with O_CLOEXEC (patchset #4 id:60001 of https://codereview.chromium.org/2394413002/ )

Reason for revert:
Reverting since it breaks NaCL tests.

https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29%2832%29/builds/34211

failures:
NaClBrowserTestNonSfiMode.Irt
NaClBrowserTestNonSfiMode.Messaging

Original issue's description:
> Open MessagePumpLibevent's pipe with O_CLOEXEC
>
> This prevents the pipe's fds from being leaked into child processes.
>
> TEST=git cl try
> BUG= 653930 
>
> Committed: https://crrev.com/f9c8562e55b669915faf87d8d7eda32ae62bf661
> Cr-Commit-Position: refs/heads/master@{#423899}

TBR=dcheng@chromium.org,lhchavez@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 653930 

Review-Url: https://codereview.chromium.org/2401863004
Cr-Commit-Position: refs/heads/master@{#423966}

[modify] https://crrev.com/c231da96b4586157979d9e2ec41561b7b115b2e2/base/files/file_util.h
[modify] https://crrev.com/c231da96b4586157979d9e2ec41561b7b115b2e2/base/files/file_util_posix.cc
[modify] https://crrev.com/c231da96b4586157979d9e2ec41561b7b115b2e2/base/message_loop/message_pump_libevent.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/80c77bfd8614a09e83b618a819bef076508bdcf8

commit 80c77bfd8614a09e83b618a819bef076508bdcf8
Author: lhchavez <lhchavez@chromium.org>
Date: Sat Oct 08 00:50:53 2016

Reland "Open MessagePumpLibevent's pipe with O_CLOEXEC"

This prevents the pipe's fds from being leaked into child processes.

TEST=git cl try
BUG= 653930 

Review-Url: https://codereview.chromium.org/2398253004
Cr-Commit-Position: refs/heads/master@{#424030}

[modify] https://crrev.com/80c77bfd8614a09e83b618a819bef076508bdcf8/base/files/file_util.h
[modify] https://crrev.com/80c77bfd8614a09e83b618a819bef076508bdcf8/base/files/file_util_posix.cc
[modify] https://crrev.com/80c77bfd8614a09e83b618a819bef076508bdcf8/base/message_loop/message_pump_libevent.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/external/libchrome/+/b86156c4a5e2cb10b69a2bccbade9b8d359f3662

commit b86156c4a5e2cb10b69a2bccbade9b8d359f3662
Author: Luis Hector Chavez <lhchavez@google.com>
Date: Fri Oct 07 18:08:30 2016

Open MessagePumpLibevent's pipe with O_CLOEXEC

This prevents the pipe's fds from being leaked into child processes.
This is a cherry-pick of https://crrev.com/80c77bfd86 .

TEST=git cl try
BUG= chromium:653930 

Change-Id: I1d9ea26e41a23dc7ca5cd6d4b2133a2e69b1259c
Reviewed-on: https://chromium-review.googlesource.com/395446
Commit-Ready: Luis Hector Chavez <lhchavez@google.com>
Tested-by: Luis Hector Chavez <lhchavez@google.com>
Reviewed-by: Luis Hector Chavez <lhchavez@google.com>

[modify] https://crrev.com/b86156c4a5e2cb10b69a2bccbade9b8d359f3662/base/files/file_util_posix.cc
[modify] https://crrev.com/b86156c4a5e2cb10b69a2bccbade9b8d359f3662/base/message_loop/message_pump_libevent.cc
[modify] https://crrev.com/b86156c4a5e2cb10b69a2bccbade9b8d359f3662/base/files/file_util.h

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/43a95d996169ce928f8607d2b11b074601f9b89f

commit 43a95d996169ce928f8607d2b11b074601f9b89f
Author: Luis Hector Chavez <lhchavez@google.com>
Date: Wed Oct 12 16:03:51 2016

Uprev libchrome to include the O_CLOEXEC patch

This allows session_manager to start containers without leaking the file
descriptors used to signal the main MessagePump.

BUG= chromium:653930 
TEST=ARC still starts.

Change-Id: I6dfd574c0b23c7b36d35a58307c6be357ee9e38e
Reviewed-on: https://chromium-review.googlesource.com/397338
Commit-Ready: Luis Hector Chavez <lhchavez@google.com>
Tested-by: Luis Hector Chavez <lhchavez@google.com>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>

[modify] https://crrev.com/43a95d996169ce928f8607d2b11b074601f9b89f/chromeos-base/libchrome/libchrome-395517.ebuild
[rename] https://crrev.com/43a95d996169ce928f8607d2b11b074601f9b89f/chromeos-base/libchrome/libchrome-395517-r4.ebuild

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 11 2016

Labels: merge-merged-release-R55-8872.B
The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/external/libchrome/+/162c93f827df8a33705dd0ff16c9a949373dfc75

commit 162c93f827df8a33705dd0ff16c9a949373dfc75
Author: Luis Hector Chavez <lhchavez@google.com>
Date: Fri Oct 07 18:08:30 2016

Open MessagePumpLibevent's pipe with O_CLOEXEC

This prevents the pipe's fds from being leaked into child processes.
This is a cherry-pick of https://crrev.com/80c77bfd86 .

TEST=git cl try
BUG= chromium:653930 

Change-Id: I1d9ea26e41a23dc7ca5cd6d4b2133a2e69b1259c
Reviewed-on: https://chromium-review.googlesource.com/395446
Commit-Ready: Luis Hector Chavez <lhchavez@google.com>
Tested-by: Luis Hector Chavez <lhchavez@google.com>
Reviewed-by: Luis Hector Chavez <lhchavez@google.com>
(cherry picked from commit b86156c4a5e2cb10b69a2bccbade9b8d359f3662)

[modify] https://crrev.com/162c93f827df8a33705dd0ff16c9a949373dfc75/base/files/file_util_posix.cc
[modify] https://crrev.com/162c93f827df8a33705dd0ff16c9a949373dfc75/base/message_loop/message_pump_libevent.cc
[modify] https://crrev.com/162c93f827df8a33705dd0ff16c9a949373dfc75/base/files/file_util.h

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/d9ef03cdc3cc057415c2ef11fcd4701854fe5aae

commit d9ef03cdc3cc057415c2ef11fcd4701854fe5aae
Author: Luis Hector Chavez <lhchavez@google.com>
Date: Wed Oct 12 16:03:51 2016

Uprev libchrome to include the O_CLOEXEC patch

This allows session_manager to start containers without leaking the file
descriptors used to signal the main MessagePump.

BUG= chromium:653930 
TEST=ARC still starts.
BUG=b:32005517

Change-Id: I6dfd574c0b23c7b36d35a58307c6be357ee9e38e
Reviewed-on: https://chromium-review.googlesource.com/397338
Commit-Ready: Luis Hector Chavez <lhchavez@google.com>
Tested-by: Luis Hector Chavez <lhchavez@google.com>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
(cherry picked from commit 43a95d996169ce928f8607d2b11b074601f9b89f)
Reviewed-on: https://chromium-review.googlesource.com/411404
Commit-Queue: Luis Hector Chavez <lhchavez@google.com>

[modify] https://crrev.com/d9ef03cdc3cc057415c2ef11fcd4701854fe5aae/chromeos-base/libchrome/libchrome-395517.ebuild
[rename] https://crrev.com/d9ef03cdc3cc057415c2ef11fcd4701854fe5aae/chromeos-base/libchrome/libchrome-395517-r4.ebuild

Status: Verified (was: Started)
This has been merged to all needed places.

Sign in to add a comment