Masterbug for shill sand-boxing breaks networking and causes shill/chrome crashes |
|||||||||||||||||
Issue descriptionWe have too many bugs to track, so why not add one more... (in all seriousness, the idea is to have a central location for updates and track merged patches). Potential dup-ed bugs against this one: crbug.com/867171 : Network configurations not loaded correctly. crbug.com/867685 : Lost the wireless networks list during disable/enable Wi-Fi from uber tray crbug.com/867680 : UI disconnecting and connecting to VPN is very finicky in M69. crbug.com/866696 : Browser crash: __construct_node<const std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > &> crbug.com/865442 : Chrome Crash: 085bd65c-exit143-shill Brian, Micah and I met and agreed to revert shill sandboxing on R69 to give us breathing room. We'll figure out the right fixes and tests on R70.
,
Aug 1
If it's P0, it should block a release. If you do not think this should block a release, change the label and the priority.
,
Aug 2
,
Aug 3
Issue 870433 has been merged into this issue.
,
Aug 3
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/f5495fb885dc294352c16376e60736ee12197cbf commit f5495fb885dc294352c16376e60736ee12197cbf Author: Micah Morton <mortonm@chromium.org> Date: Fri Aug 03 00:58:23 2018 Revert "Reland "shill: start running shill process tree in a minijail"" This reverts commit 8751e2a8474c895d11d56ca45e4f71e808b25c27. Reason for revert: Reverting this so the revert can be merged back into M69. We believe we have all the networking stuff fixed as of https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/1153841, but are doing this revert just to give us some breathing room by punting landing the shill sandboxing from M69 to M70, in case there's anything thats still broken that we don't know about. Original change's description: > Reland "shill: start running shill process tree in a minijail" > > This is a reland of aad3747780f4f81a971878773b79635e1b3dc001 > > The 2 known issues which caused problems the first time around have been > addressed by CL:1130017 and CL:1130474. I've done some more thorough > testing (including with elm board, which is arm processor and marvell wifi) > and haven't discovered any new issues. > > CQ-DEPEND=CL:1130017,CL:1130474 > > Original change's description: > > shill: start running shill process tree in a minijail > > > > Kick off shill sandboxing (we'll see if it sticks). This simple change to > > the init script as well as the autotest change to > > security_SandboxedServices are the only 2 things that need to be > > reverted to go back to running the shill process tree as root. > > > > CQ-DEPEND=CL:1082958 > > BUG=chromium:649417 > > TEST=observed shill process tree running as non-root. no autotests > > break. > > > > Change-Id: Ice5ed2cbbc64281ba8f136e5157023ff355245d6 > > Reviewed-on: https://chromium-review.googlesource.com/1087540 > > Commit-Ready: Micah Morton <mortonm@chromium.org> > > Tested-by: Micah Morton <mortonm@chromium.org> > > Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> > > Bug: chromium:649417 > Change-Id: I3fc5506ee8886db8d6a4d7d155216aba9e20b7f8 > Reviewed-on: https://chromium-review.googlesource.com/1132174 > Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> > Tested-by: Micah Morton <mortonm@chromium.org> > Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> > Reviewed-by: Mike Frysinger <vapier@chromium.org> TBR=benchan@chromium.org,vapier@chromium.org,jorgelo@chromium.org,briannorris@chromium.org,mnissler@chromium.org,mortonm@chromium.org,chromiumos-cl-exonerator@appspot.gserviceaccount.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: chromium:649417, chromium:870076 CQ-DEPEND=CL:1159403 Change-Id: I60a5fd7aac0d39038ed41c6a1aef834a243c6f94 Reviewed-on: https://chromium-review.googlesource.com/1159481 Commit-Ready: Micah Morton <mortonm@chromium.org> Tested-by: Micah Morton <mortonm@chromium.org> Reviewed-by: Micah Morton <mortonm@chromium.org> Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org> Reviewed-by: Brian Norris <briannorris@chromium.org> [modify] https://crrev.com/f5495fb885dc294352c16376e60736ee12197cbf/init/shill.sh
,
Aug 3
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/d374e92693a8352daccd772e2b724f1f9fd14f21 commit d374e92693a8352daccd772e2b724f1f9fd14f21 Author: Micah Morton <mortonm@chromium.org> Date: Fri Aug 03 00:58:23 2018 Revert "Reland "security_SandboxedServices: reflect shill sandboxing changes"" This reverts commit a248a9ab77c19be3afff948de8151e03b25fd619. Reason for revert: Reverting this so the revert can be merged back into M69. We believe we have all the networking stuff fixed as of https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/1153841, but are doing this revert just to give us some breathing room by punting landing the shill sandboxing from M69 to M70, in case there's anything thats still broken that we don't know about. Original change's description: > Reland "security_SandboxedServices: reflect shill sandboxing changes" > > This is a reland of aa6a1634eeb3751f2b10fc40a8b31d19c0ea0c8a > > The 2 known issues which caused problems the first time around have been > addressed by CL:1130017 and CL:1130474. I've done some more thorough > testing (including with elm board, which is arm processor and marvell wifi) > and haven't discovered any new issues. > > CQ-DEPEND=CL:1130017,CL:1130474 > > Original change's description: > > security_SandboxedServices: reflect shill sandboxing changes > > > > This CL should land along with enabling spawning of shill in a minijail > > as a non-root user with -n (no_new_privs). This change and the dependent > > CL are the only 2 things that need to be reverted to go back to running > > the shill process tree as root. > > > > CQ-DEPEND=CL:1087540 > > BUG=chromium:649417 > > TEST=tests passes alongside larger sandbox shill debug CL > > > > Change-Id: I7a1145ff8ad9f0a2d39d1c432220dc628f3d39e5 > > Reviewed-on: https://chromium-review.googlesource.com/1082958 > > Commit-Ready: Micah Morton <mortonm@chromium.org> > > Tested-by: Micah Morton <mortonm@chromium.org> > > Reviewed-by: Mattias Nissler <mnissler@chromium.org> > > Bug: chromium:649417 > Change-Id: I98ecbe6b7af10a9c5af208f964d03807e95a9253 > Reviewed-on: https://chromium-review.googlesource.com/1132194 > Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> > Tested-by: Micah Morton <mortonm@chromium.org> > Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> Bug: chromium:649417, chromium:870076 CQ-DEPEND=CL:1159481 Change-Id: I4589919143cde10a288d000309b17b3b0dd7b3ee Reviewed-on: https://chromium-review.googlesource.com/1159403 Commit-Ready: Micah Morton <mortonm@chromium.org> Tested-by: Micah Morton <mortonm@chromium.org> Reviewed-by: Brian Norris <briannorris@chromium.org> [modify] https://crrev.com/d374e92693a8352daccd772e2b724f1f9fd14f21/client/site_tests/security_SandboxedServices/baseline
,
Aug 3
Approving revert for M69.
,
Aug 3
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/0978741140dac324ad0f3c7312f4e5bc4cb6bdf6 commit 0978741140dac324ad0f3c7312f4e5bc4cb6bdf6 Author: Micah Morton <mortonm@chromium.org> Date: Thu Aug 02 22:25:08 2018 Revert "Reland "shill: start running shill process tree in a minijail"" This reverts commit 8751e2a8474c895d11d56ca45e4f71e808b25c27. Reason for revert: Reverting this so the revert can be merged back into M69. We believe we have all the networking stuff fixed as of https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/1153841, but are doing this revert just to give us some breathing room by punting landing the shill sandboxing from M69 to M70, in case there's anything thats still broken that we don't know about. Original change's description: > Reland "shill: start running shill process tree in a minijail" > > This is a reland of aad3747780f4f81a971878773b79635e1b3dc001 > > The 2 known issues which caused problems the first time around have been > addressed by CL:1130017 and CL:1130474. I've done some more thorough > testing (including with elm board, which is arm processor and marvell wifi) > and haven't discovered any new issues. > > CQ-DEPEND=CL:1130017,CL:1130474 > > Original change's description: > > shill: start running shill process tree in a minijail > > > > Kick off shill sandboxing (we'll see if it sticks). This simple change to > > the init script as well as the autotest change to > > security_SandboxedServices are the only 2 things that need to be > > reverted to go back to running the shill process tree as root. > > > > CQ-DEPEND=CL:1082958 > > BUG=chromium:649417 > > TEST=observed shill process tree running as non-root. no autotests > > break. > > > > Change-Id: Ice5ed2cbbc64281ba8f136e5157023ff355245d6 > > Reviewed-on: https://chromium-review.googlesource.com/1087540 > > Commit-Ready: Micah Morton <mortonm@chromium.org> > > Tested-by: Micah Morton <mortonm@chromium.org> > > Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> > > Bug: chromium:649417 > Change-Id: I3fc5506ee8886db8d6a4d7d155216aba9e20b7f8 > Reviewed-on: https://chromium-review.googlesource.com/1132174 > Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> > Tested-by: Micah Morton <mortonm@chromium.org> > Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> > Reviewed-by: Mike Frysinger <vapier@chromium.org> TBR=benchan@chromium.org,vapier@chromium.org,jorgelo@chromium.org,briannorris@chromium.org,mnissler@chromium.org,mortonm@chromium.org,chromiumos-cl-exonerator@appspot.gserviceaccount.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: chromium:649417, chromium:870076 CQ-DEPEND=CL:1159403 Change-Id: I60a5fd7aac0d39038ed41c6a1aef834a243c6f94 [modify] https://crrev.com/0978741140dac324ad0f3c7312f4e5bc4cb6bdf6/init/shill.sh
,
Aug 3
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/c043374682e274459c46d6fc583597f7f113cd33 commit c043374682e274459c46d6fc583597f7f113cd33 Author: Micah Morton <mortonm@chromium.org> Date: Fri Aug 03 16:17:21 2018 Revert "Reland "security_SandboxedServices: reflect shill sandboxing changes"" This reverts commit a248a9ab77c19be3afff948de8151e03b25fd619. Reason for revert: Reverting this so the revert can be merged back into M69. We believe we have all the networking stuff fixed as of https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/1153841, but are doing this revert just to give us some breathing room by punting landing the shill sandboxing from M69 to M70, in case there's anything thats still broken that we don't know about. Original change's description: > Reland "security_SandboxedServices: reflect shill sandboxing changes" > > This is a reland of aa6a1634eeb3751f2b10fc40a8b31d19c0ea0c8a > > The 2 known issues which caused problems the first time around have been > addressed by CL:1130017 and CL:1130474. I've done some more thorough > testing (including with elm board, which is arm processor and marvell wifi) > and haven't discovered any new issues. > > CQ-DEPEND=CL:1130017,CL:1130474 > > Original change's description: > > security_SandboxedServices: reflect shill sandboxing changes > > > > This CL should land along with enabling spawning of shill in a minijail > > as a non-root user with -n (no_new_privs). This change and the dependent > > CL are the only 2 things that need to be reverted to go back to running > > the shill process tree as root. > > > > CQ-DEPEND=CL:1087540 > > BUG=chromium:649417 > > TEST=tests passes alongside larger sandbox shill debug CL > > > > Change-Id: I7a1145ff8ad9f0a2d39d1c432220dc628f3d39e5 > > Reviewed-on: https://chromium-review.googlesource.com/1082958 > > Commit-Ready: Micah Morton <mortonm@chromium.org> > > Tested-by: Micah Morton <mortonm@chromium.org> > > Reviewed-by: Mattias Nissler <mnissler@chromium.org> > > Bug: chromium:649417 > Change-Id: I98ecbe6b7af10a9c5af208f964d03807e95a9253 > Reviewed-on: https://chromium-review.googlesource.com/1132194 > Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> > Tested-by: Micah Morton <mortonm@chromium.org> > Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> Bug: chromium:649417, chromium:870076 CQ-DEPEND=CL:1159481 Change-Id: I4589919143cde10a288d000309b17b3b0dd7b3ee Reviewed-on: https://chromium-review.googlesource.com/1161421 Reviewed-by: Brian Norris <briannorris@chromium.org> Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org> Commit-Queue: Micah Morton <mortonm@chromium.org> Tested-by: Micah Morton <mortonm@chromium.org> Trybot-Ready: Micah Morton <mortonm@chromium.org> [modify] https://crrev.com/c043374682e274459c46d6fc583597f7f113cd33/client/site_tests/security_SandboxedServices/baseline
,
Aug 6
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 6
,
Aug 6
Changed to RBS per Omri.
,
Aug 31
Closing this for now. We can reopen if more bugs surface on M70, even with the fixes that went in as a response to breakages in M69.
,
Sep 20
,
Sep 20
Let's move this to M70, as sandboxing is gone on M69. This almost feels like it could be a dupe of bug 649417. I guess we can just make it 'blocking'. Also, we should be "blocked on" our sub-bugs, not "blocking."
,
Sep 20
,
Sep 22
,
Oct 1
Friendly ping, please provide an update on this issue. Is this still RBS (if not please remove the label)? When is the ETA on the work that needs to be done to resolve this? Thanks.
,
Oct 4
,
Oct 9
As of M70 image 11021.43.0, we have shill sandboxing disabled by default (although its still enable-able through Finch) and a flag in the chrome://flags page that can turn sandboxing on/off. At this point we are good to go ahead and turn the 50:50 shill sandboxing experiment off on Finch for beta/stable if we want to punt to M71. Hopefully we get one more beta push for M70 (with an image >= 11021.43.0) so we can see evidence in UMA of us turning the feature off on Finch and all devices on the M70 beta channel going back to running shill as root.
,
Oct 12
Latest M70 Beta has been pushed. I will check in with folks on Monday once we get more data.
,
Oct 12
We should watch M-70 stats to be sure, but Micah disabled shill sandboxing on M-70 (actually, all beta and stable). We'll see about M-71 or M-72 then.
,
Oct 16
,
Nov 8
<Bulk edit> Reminder M71 Stable is approaching. Please review this bug and assess if this is indeed a RBS. If not, please remove the RBS label. If so, please make sure any planned work will be tested in Beta and verified before the Stable date. Thanks
,
Nov 8
Punted the feature to M-72
,
Nov 8
Actually, we have no more known blockers. If new ones pop up, we can either reopen or block bug 649417. Do shout if I'm wrong.
,
Nov 9
Thanks Brian, SGTM |
|||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||
Comment 1 by kirtika@google.com
, Aug 1