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

Issue 872088 link

Starred by 8 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

OOBE: Device is stuck permanently on 'Enable debugging features' screen

Project Member Reported by sdantul...@chromium.org, Aug 8

Issue description

ChromeOS  10895.16.0, 69.0.3497.29  dev-channel  peppy

What steps will reproduce the problem?
1. Make sure that device is in developer mode
2. Powerwash the device to get to OOBE screens
3. Click on 'Enable debugging features' link
4. On the confirmation screen, click on 'Proceed' button

What is the expected result?
The device reboots and displays a dialog with password prompt.

What happens instead?
Device reboots and displays confirmation screen again. 
Clicking on 'Proceed' button reboots the device and displays confirmation screen again. Clicking on 'Cancel' does not dismiss the screen.

Device is stuck on this screen permanently and the only way out of it is to install recovery image with usb stick.

Issue also reproduced on cave, coral devices.
 
Description: Show this description
Labels: -ReleaseBlock-Beta ReleaseBlock-Stable
+rkc, alemate, jdufault can you take a look.

Moving to RBS, developer mode will not block Beta.
Cc: r...@chromium.org alemate@chromium.org jdufault@chromium.org
Adding devs mentioned in c#3
Seems like OOBE is just re-entering the screen

[833:833:0807/173728.595365:VERBOSE1:webui_login_view.cc(614)] Login WebUI >> login-prompt-visible
[833:833:0807/173730.757969:VERBOSE1:wizard_controller.cc(1388)] Wizard screen exit code: ENABLE_DEBUGGING_CANCELED
[833:833:0807/173730.758021:VERBOSE1:wizard_controller.cc(1222)] SetCurrentScreenSmooth: connect
[833:833:0807/173730.758169:VERBOSE1:wizard_controller.cc(608)] Showing enable developer features screen.
[833:833:0807/173730.758193:VERBOSE1:wizard_controller.cc(1222)] SetCurrentScreenSmooth: debugging

That is, ENABLE_DEBUGGING_CANCELLED -> connect -> debugging
Owner: agawronska@chromium.org
Status: Assigned (was: Untriaged)
Handling to agawronska@ who has been modifying wizard_controller.cc lately
Possible fallout from https://chromium.googlesource.com/chromium/src/+/1001fd375442428751b18f3fed991c77874b92eb?

I'm not sure if that CL is in 69 though
This cl did not go to 69 (I didn't want to put it just before branch):
https://chromiumdash.appspot.com/commits?commit=1001fd375442428751b18f3fed991c77874b92eb&repo=chromium&platform=Linux

Anyway I did some changes in 69 OOBE, so I will have a look. Thanks!
Please provide latest status and prioritize fix if needed. Thanks.
Reproduced this on an ASUS chromebox 3. USB recovery let me fix it (though the recovery said there was an unexpected error, but after a hard reset it showed the "firmware update" screen and then let me do a successful USB recovery)
Here is what happens on chromium side:
* Dialog to enabled debugging is shown
* Device is rebooted
* It is expected that rootfs verification will be removed at that point, but it never is. We are stuck at the following piece of code: https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/chromeos/login/enable_debugging_screen_handler.cc?rcl=b1a9183dd954533f233fe3fa51f88bc96b72b257&l=250

Re:Comment 5
The rootfs verification flag is never set, so we never clear kDebuggingFeaturesRequested pref. When cancel is pressed we go back to welcome screen, but welcome screen is checking the pref again and showing enable debugging screen again. I believe this is the correct behavior.

It looks like the problem lies on the platform side.
I was able to narrow it down to between 10738.0.0 and 10739.0.0.
Chromium version didn't change then and was 69.0.3445.0.

Changelog: https://crosland.corp.google.com/log/10738.0.0..10739.0.0
There is one change to debugd there [1], but it is outside of my area of expertise and it is hard to judge for me.

We need someone from platform team to have a look at it.
Cc: vapier@chromium.org lhchavez@chromium.org
Owner: benchan@chromium.org
Reassigning to benchan@ as per platform2/debugd/OWNERS.

Please have a look and help to triage this bug on the Platform side. 
One more thing.
On chromium end we could clear the pref when 'Cancel' button is pressed.
It would allow to escape the loop, but without fixing the underlying problem. The enable debugging feature would be still broken.
probably is related to the debugd minijail sandboxing

looking at the code, debugd is supposed to be returning errors.  is Chrome not checking for those and instead just waiting forever ?  that should be fixed.
the problem is due to https://chromium-review.googlesource.com/1053426 where we moved debugd into a restricted mount namespace

the org.chromium.debugd.QueryDevFeatures operation runs `/usr/libexec/debugd/helpers/dev_features_rootfs_verification -q` which boils down to a simple access("/", W_OK) call.  since we're now running in a restricted mount namespace where our / is (by design) always a read-only bind mount, this check always fails.  oops.

one way we can address this is by changing how dev_features_rootfs_verification is invoked in RemoveRootfsVerificationQuery.  if we change it to run in the root mount namespace, it'd be able to query / status correctly.  we'd have to plumb through a knob to set AllowAccessRootMountNamespace with the run process helpers.
Owner: lhchavez@chromium.org
Ben is out this week, the CL that broke this was authored by Luis, can you take a look at how best to fix this Luis?

We need this soon if this is to block 69 stable.
Another way to fix this without breaking the isolation is to inspect the superblock info of '/' in /proc/self/mountinfo:

localhost ~ # mount -o remount,ro /
localhost ~ # cat /proc/`pidof debugd`/mountinfo | head -n 1
159 58 179:5 / / ro,relatime - ext2 /dev/root ro,seclabel,block_validity,barrier,user_xattr,acl
#                                             ^^
localhost ~ # mount -o remount,rw /
localhost ~ # cat /proc/`pidof debugd`/mountinfo | head -n 1
159 58 179:5 / / ro,relatime - ext2 /dev/root rw,seclabel,block_validity,barrier,user_xattr,acl
#                                             ^^
note that the obvious solution of using statfs(2) won't work because on v4.4 (and previous kernel versions), the superblock ro bit is not considered[1], and in later kernels that do expose the bit from the superblock will be clobbered by the mount ro bit[1]. Reading /proc/self/mountinfo and doing some base::StringSplit() is unfortunately needed.

1: https://elixir.bootlin.com/linux/v4.4/source/fs/statfs.c#L33
2: https://elixir.bootlin.com/linux/v4.14/source/fs/statfs.c#L35
Owner: phshah@chromium.org
phshah@ volunteered to fix this.
And a trivial repro scenario:

localhost ~ # mount -o remount,rw /
localhost ~ # nsenter --mount -t `pidof debugd` -- /usr/libexec/debugd/helpers/dev_features_rootfs_verification -q ; echo $?
1   # should print 0
a simple hack would be to run the tool through `nsenter --mount -p1 ...`, but that would just be a stopgap to writing the C++ code directly
Status: Started (was: Assigned)
Fix has been reviewed and is in commit queue: https://crrev.com/c/1219975
Labels: Merge-Request-70 Merge-Request-69
Project Member

Comment 24 by sheriffbot@chromium.org, Sep 11

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-69 Merge-Approved-69
Merge approved, M69.
Project Member

Comment 26 by bugdroid1@chromium.org, Sep 11

Labels: merge-merged-release-R69-10895.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/89180cc21625eec2bc44678d23e6853db97cf8b5

commit 89180cc21625eec2bc44678d23e6853db97cf8b5
Author: Prameet Shah <phshah@google.com>
Date: Tue Sep 11 20:58:37 2018

debugd: Updated the logic that checks if '/' is writable.

This change makes dev_features_rootfs_verification check whether / is
writable in the init namespace, instead of the mount namespace that
debugd is sandboxed into.

BUG= chromium:872088 
TEST=Tested the commands below on kevin, and verified the return value is 0.
     $ mount -o remount,rw /
     $ nsenter --mount -t `pidof debugd` -- /usr/libexec/debugd/helpers/dev_features_rootfs_verification -q ; echo $?

Change-Id: I2eb721e6ecccbcf81d1a1203e195eb585f975774
Reviewed-on: https://chromium-review.googlesource.com/1219976
Commit-Queue: Prameet Shah <phshah@google.com>
Tested-by: Prameet Shah <phshah@google.com>
Trybot-Ready: Prameet Shah <phshah@google.com>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/89180cc21625eec2bc44678d23e6853db97cf8b5/debugd/src/helpers/dev_features_rootfs_verification.cc

Project Member

Comment 27 by sheriffbot@chromium.org, Sep 12

Labels: -Merge-Request-70 Hotlist-Merge-Approved Merge-Approved-70
Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -M-69 M-70
Moving milestone to 70 since this is merged in 69, if this still really happens in 69 please add the label back.
Project Member

Comment 29 by bugdroid1@chromium.org, Sep 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ee448ca6441b186bba36b03297c1a02ca52ec16a

commit ee448ca6441b186bba36b03297c1a02ca52ec16a
Author: Prameet Shah <phshah@google.com>
Date: Sat Sep 15 19:56:23 2018

debugd: Updated the logic that checks if '/' is writable.

This change makes dev_features_rootfs_verification check whether / is
writable in the init namespace, instead of the mount namespace that
debugd is sandboxed into.

BUG= chromium:872088 
TEST=Tested the commands below on kevin, and verified the return value is 0.
     $ mount -o remount,rw /
     $ nsenter --mount -t `pidof debugd` -- /usr/libexec/debugd/helpers/dev_features_rootfs_verification -q ; echo $?

Change-Id: I2eb721e6ecccbcf81d1a1203e195eb585f975774
Reviewed-on: https://chromium-review.googlesource.com/1219975
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Prameet Shah <phshah@google.com>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/ee448ca6441b186bba36b03297c1a02ca52ec16a/debugd/src/helpers/dev_features_rootfs_verification.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Sep 15

Labels: merge-merged-release-R70-11021.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/161d06e85f66526d433323ff4acabfd09b0bb042

commit 161d06e85f66526d433323ff4acabfd09b0bb042
Author: Prameet Shah <phshah@google.com>
Date: Sat Sep 15 23:14:37 2018

debugd: Updated the logic that checks if '/' is writable.

This change makes dev_features_rootfs_verification check whether / is
writable in the init namespace, instead of the mount namespace that
debugd is sandboxed into.

BUG= chromium:872088 
TEST=Tested the commands below on kevin, and verified the return value is 0.
     $ mount -o remount,rw /
     $ nsenter --mount -t `pidof debugd` -- /usr/libexec/debugd/helpers/dev_features_rootfs_verification -q ; echo $?

Change-Id: I2eb721e6ecccbcf81d1a1203e195eb585f975774
(cherry picked from commit ee448ca6441b186bba36b03297c1a02ca52ec16a)
Reviewed-on: https://chromium-review.googlesource.com/1227323
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/161d06e85f66526d433323ff4acabfd09b0bb042/debugd/src/helpers/dev_features_rootfs_verification.cc

Labels: -Merge-Approved-69 -Merge-Approved-70 Merge-Merged
Status: Fixed (was: Started)
Labels: M-69
Cc: benchan@chromium.org
 Issue 855644  has been merged into this issue.
Project Member

Comment 34 by bugdroid1@chromium.org, Oct 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/54583b50c9a6f971a677a74a76fc32369f4b7a1c

commit 54583b50c9a6f971a677a74a76fc32369f4b7a1c
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Oct 02 12:19:30 2018

debugd: configure ssh access in the root mount namespace

Since debugd always runs with / read-only (even if the underlying
filesystem is not), make sure we switch to the root mount namespace
before making filesystem changes to enable ssh access.

BUG= chromium:872088 , chromium:890898 
TEST=enabling debug features in OOBE works

Change-Id: Ic6be83fc3594b3f8c2f303d916ecd4e6f802d264
Reviewed-on: https://chromium-review.googlesource.com/1253002
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>

[modify] https://crrev.com/54583b50c9a6f971a677a74a76fc32369f4b7a1c/debugd/src/helpers/dev_features_ssh.cc

Project Member

Comment 35 by bugdroid1@chromium.org, Oct 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/13eb097f4e40b55fadb9144c08bb0a3186920121

commit 13eb097f4e40b55fadb9144c08bb0a3186920121
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Oct 02 17:23:00 2018

debugd: configure ssh access in the root mount namespace

Since debugd always runs with / read-only (even if the underlying
filesystem is not), make sure we switch to the root mount namespace
before making filesystem changes to enable ssh access.

BUG= chromium:872088 , chromium:890898 
TEST=enabling debug features in OOBE works

Change-Id: Ic6be83fc3594b3f8c2f303d916ecd4e6f802d264
(cherry picked from commit 54583b50c9a6f971a677a74a76fc32369f4b7a1c)
Reviewed-on: https://chromium-review.googlesource.com/1257307
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/13eb097f4e40b55fadb9144c08bb0a3186920121/debugd/src/helpers/dev_features_ssh.cc

Project Member

Comment 36 by bugdroid1@chromium.org, Oct 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/35c43b46619ee96565825ba6b71e3f1ee7447e01

commit 35c43b46619ee96565825ba6b71e3f1ee7447e01
Author: Mike Frysinger <vapier@chromium.org>
Date: Sat Oct 06 06:13:31 2018

debugd: configure the user password in the root mount namespace

Since debugd runs w/out mounting the stateful partition at all,
make sure we switch to the root mount namespace before checking
the dev mode password.

BUG= chromium:872088 , chromium:890898 
TEST=enabling debug features in OOBE works

Change-Id: I5e2bf940839d25005b37deff78e282590ce34a0d
Reviewed-on: https://chromium-review.googlesource.com/1255385
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>

[modify] https://crrev.com/35c43b46619ee96565825ba6b71e3f1ee7447e01/debugd/src/helpers/dev_features_password.cc

Project Member

Comment 37 by bugdroid1@chromium.org, Oct 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a3a4709947f92d278edc8002c3ad6fc1d685659f

commit a3a4709947f92d278edc8002c3ad6fc1d685659f
Author: Mike Frysinger <vapier@chromium.org>
Date: Sat Oct 06 06:25:30 2018

debugd: configure the user password in the root mount namespace

Since debugd runs w/out mounting the stateful partition at all,
make sure we switch to the root mount namespace before checking
the dev mode password.

BUG= chromium:872088 , chromium:890898 
TEST=enabling debug features in OOBE works

Change-Id: I5e2bf940839d25005b37deff78e282590ce34a0d
(cherry picked from commit 35c43b46619ee96565825ba6b71e3f1ee7447e01)
Reviewed-on: https://chromium-review.googlesource.com/c/1266975
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a3a4709947f92d278edc8002c3ad6fc1d685659f/debugd/src/helpers/dev_features_password.cc

Sign in to add a comment