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

Issue 612966 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

ec: Don't kick off RW hashing in keyboard-triggered recovery mode

Project Member Reported by sha...@chromium.org, May 18 2016

Issue description

EC RW vboot hashing is normally kicked off from a hook_init routine. The RW hash isn't normally useful in recovery mode since we won't do software sync, and it has created a bad RO FW bug in the past.

Let's not kick off the hashing process in recovery mode. We can probably assume we're in recovery mode if the ESC button is held at boot time (from which we set EC_HOST_EVENT_KEYBOARD_RECOVERY), and our current image is RO, and we didn't sysjump into RO. If we guess wrong and fail to kick off hashing in non-recovery mode, I think the only consequence will be delayed boot (since system FW will request hashing begin though EC_VBOOT_HASH_START), but I will confirm.
 
Yes, the only consequence should be a delayed boot if we guess wrong.

We should abstract this:

	if (boot_key_value == BOOT_KEY_ESC)
		host_set_single_event(EC_HOST_EVENT_KEYBOARD_RECOVERY);

To something like this:

	if (boot_key_value == BOOT_KEY_ESC)
                keyboard_recovery();

and then do both
		host_set_single_event(EC_HOST_EVENT_KEYBOARD_RECOVERY);
and cancelling the hashing in that function.

Comment 2 by gkihumba@google.com, Mar 31 2017

Status: Assigned (was: Untriaged)
Is this fixed?

Comment 3 by sha...@chromium.org, Jun 19 2017

CL is up here, but I'm not yet convinced that this change is worthwhile.

https://chromium-review.googlesource.com/#/c/540695/
I think it's worthwhile.  In general, recovery should do as little as possible so we avoid potential bricking bugs.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/75edd8984f360ec1f46906facb6e624404b6bc0e

commit 75edd8984f360ec1f46906facb6e624404b6bc0e
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Wed Jun 28 04:28:05 2017

vboot_hash: Don't auto-start hashing in recovery mode

If we have requested the host to go to recovery mode, the host will
usually not need an RW hash because it won't do EC SW sync. Therefore,
do not auto-start calculation of our hash if we've requested recovery.
This may avoid past recovery-breaking bugs due to unexpected RW contents.
If the host does need the hash after all, it will manually request that
the computation start.

BUG= chromium:612966 
BRANCH=None
TEST=Boot to recovery mode on kevin, verify that "hash start" print is
not seen on UART.

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Change-Id: I66d2d74398357dfe30e39882feec8cfba4cc945c
Reviewed-on: https://chromium-review.googlesource.com/540695
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Shawn N <shawnn@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>

[modify] https://crrev.com/75edd8984f360ec1f46906facb6e624404b6bc0e/common/vboot_hash.c

Comment 6 by sha...@chromium.org, Sep 25 2017

Status: Verified (was: Assigned)

Sign in to add a comment