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

Issue 699224 link

Starred by 2 users
Status: Archived
Owner:
apronin@chromium.org
Closed: Apr 2017
Cc:
█ dkrahn@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

tpm2: tpm daemons should retry on TPM_RC_RETRY

Project Member Reported by apronin@chromium.org, Mar 7 2017 
When a handle-altering command is received while the system is supposed to be suspended, trunksd returns TPM_RC_RETRY from ResourceManager.

The daemons that sent the command often don't recognize this as a special case and fail the operation. They should retry instead.

Example from the logs:
2017-02-28T15:12:35.313290+00:00 WARNING trunksd[2260]: Received command CC 0x157 while suspended.
2017-02-28T15:12:35.313333+00:00 WARNING trunksd[2260]: Blocking command while suspended.
2017-02-28T15:12:35.314331+00:00 ERR tpm_managerd[2311]: CreateSaltingKey: Error loading salting key: TPM_RC_RETRY
2017-02-28T15:12:35.314410+00:00 ERR tpm_managerd[2311]: TakeOwnership: Error creating salting key: TPM_RC_RETRY
2017-02-28T15:12:35.314447+00:00 ERR tpm_managerd[2311]: Error taking ownership of TPM2.0
2017-02-28T15:12:35.315383+00:00 ERR cryptohomed[3382]: Take Ownership failed

Comment 1 by apronin@chromium.org, Mar 11 2017 

To deal with this issue, the most straightfoward centralized approach is implementing retries in trunks::TpmUtility. Raw low-level functions provided by trunks::Tpm shouldn't try interpret the received response codes. 

Other daemons usually access TPMs in 2.0 case through TpmUtility, with a few exceptions:
1) FlushContextSync called directly from cryptohome, attestation, chaps.
2-4) ActivateCredentialSync, CertifySync, QuoteSync called directly from attestation.

The first step is thus implementing these functions in TpmUtility and re-routing other daemons to use those instead of calling trunks::Tpm directly.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 22 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/tpm/+/a38354f4a6c78e266a950bf2ca3cd763e84424fa

commit a38354f4a6c78e266a950bf2ca3cd763e84424fa
Author: Andrey Pronin <apronin@chromium.org>
Date: Wed Mar 22 01:30:34 2017

trunks: retry synchronous commands if necessary

Retry commands sent by Tpm::*Sync() functions using SendCommandAndWait()
method if TPM_RC_RETRY or TPM_RC_NV_RATE response code is received.

BUG= chromium:699224 
TEST=Unit tests (new added)

Change-Id: Ie79a0df93c7957d0ae9de7c428b29c20972bc34d
Reviewed-on: https://chromium-review.googlesource.com/455217
Commit-Ready: Andrey Pronin <apronin@chromium.org>
Tested-by: Andrey Pronin <apronin@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/a38354f4a6c78e266a950bf2ca3cd763e84424fa/trunks/Android.mk
[modify] https://crrev.com/a38354f4a6c78e266a950bf2ca3cd763e84424fa/trunks/trunks_factory_impl.cc
[add] https://crrev.com/a38354f4a6c78e266a950bf2ca3cd763e84424fa/trunks/trunks_factory_test.cc
[modify] https://crrev.com/a38354f4a6c78e266a950bf2ca3cd763e84424fa/trunks/trunks_factory_impl.h
[modify] https://crrev.com/a38354f4a6c78e266a950bf2ca3cd763e84424fa/trunks/trunks.gyp

Comment 3 by apronin@chromium.org, Mar 27 2017

Labels: -Pri-2 Merge-Request-58 M-58 Pri-1
Project Member

Comment 4 by sheriffbot@chromium.org, Mar 28 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

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

Labels: merge-merged-release-R58-9334.B
The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/tpm/+/aaf2276665a5d51f64cc893da264f1a5df1aaf25

commit aaf2276665a5d51f64cc893da264f1a5df1aaf25
Author: Andrey Pronin <apronin@chromium.org>
Date: Tue Mar 28 19:58:32 2017

trunks: retry synchronous commands if necessary

Retry commands sent by Tpm::*Sync() functions using SendCommandAndWait()
method if TPM_RC_RETRY or TPM_RC_NV_RATE response code is received.

BUG= chromium:699224 
TEST=Unit tests (new added)

Change-Id: Ie79a0df93c7957d0ae9de7c428b29c20972bc34d
Reviewed-on: https://chromium-review.googlesource.com/455217
Commit-Ready: Andrey Pronin <apronin@chromium.org>
Tested-by: Andrey Pronin <apronin@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>
(cherry picked from commit a38354f4a6c78e266a950bf2ca3cd763e84424fa)

[modify] https://crrev.com/aaf2276665a5d51f64cc893da264f1a5df1aaf25/trunks/Android.mk
[modify] https://crrev.com/aaf2276665a5d51f64cc893da264f1a5df1aaf25/trunks/trunks_factory_impl.cc
[add] https://crrev.com/aaf2276665a5d51f64cc893da264f1a5df1aaf25/trunks/trunks_factory_test.cc
[modify] https://crrev.com/aaf2276665a5d51f64cc893da264f1a5df1aaf25/trunks/trunks_factory_impl.h
[modify] https://crrev.com/aaf2276665a5d51f64cc893da264f1a5df1aaf25/trunks/trunks.gyp
Project Member

Comment 6 by sheriffbot@chromium.org, Apr 3 2017 

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

Comment 7 by apronin@chromium.org, Apr 3 2017

Labels: -Merge-Approved-58 Merge-Merged

Comment 8 by apronin@chromium.org, Apr 3 2017

Status: Fixed (was: Assigned)

Comment 9 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)
►

Sign in to add a comment