Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(167)

Issue 6685075: Add TPM version checking (Closed)

Created:
9 years, 9 months ago by Randall Spangler
Modified:
9 years, 7 months ago
CC:
chromium-os-reviews_chromium.org, Randall Spangler, gauravsh, Bill Richardson
Visibility:
Public.

Description

Add TPM version checking Change-Id: Ic32b7bcf0bc5501e21dc84e79419a256d9b0d095 R=semenzato@chromium.org,reinauer@chromium.org BUG=chrome-os-partner:2832 TEST=manual crossystem tpm_fwver tpm_kernver On a debug system, this will return 0x00010001 0x00010001 Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=5ac39bf

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+76 lines, -35 lines) Patch
M firmware/include/vboot_struct.h View 1 chunk +5 lines, -1 line 0 comments Download
M firmware/lib/include/rollback_index.h View 2 chunks +6 lines, -2 lines 0 comments Download
M firmware/lib/rollback_index.c View 3 chunks +37 lines, -30 lines 0 comments Download
M firmware/lib/vboot_firmware.c View 2 chunks +2 lines, -0 lines 0 comments Download
M firmware/lib/vboot_kernel.c View 3 chunks +10 lines, -0 lines 0 comments Download
M firmware/linktest/main.c View 1 chunk +1 line, -0 lines 0 comments Download
M host/lib/crossystem.c View 3 chunks +13 lines, -2 lines 0 comments Download
M utility/crossystem_main.c View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Randall Spangler
9 years, 9 months ago (2011-03-17 23:48:44 UTC) #1
Stefan Reinauer
LGTM. Thank you!
9 years, 9 months ago (2011-03-18 00:10:45 UTC) #2
Luigi Semenzato
LGTM for the part that interacts with the TPM, but I don't understand the logic ...
9 years, 9 months ago (2011-03-18 00:48:42 UTC) #3
Randall
9 years, 9 months ago (2011-03-18 00:57:06 UTC) #4
On Thu, Mar 17, 2011 at 5:48 PM, <semenzato@chromium.org> wrote:

> LGTM for the part that interacts with the TPM, but I don't understand the
> logic
> behind TPM_version.  Since this change is somewhat incremental, perhaps it
> is
> explained in comments that I have missed?  And if so can you indicate where
> they
> are?  Otherwise you may 1. consider adding more comments to the code or to
> this
> CL; 2.  just live with this partial LGTM; 3. Give me more time to read the
> code.
>

The idea is to be able to read the TPM version from all modes, so autoupdate
can determine if the firmware+kernel it has will meet the rollback
protection requirements from the current versions in the TPM.

I'll add more comments in a follow-on CL.

There are some big functions in there.  For instance, LoadKernel is 450
> lines.
>

Yes, ripe for refactoring.

Would Gaurav be a better reviewer for this?  I don't mind spending more time
> on
> this though.


>
> http://codereview.chromium.org/6685075/
>

Powered by Google App Engine
This is Rietveld 408576698