Change in coreboot[master]: vboot: Upgrade kernel space to v1.0

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Change in coreboot[master]: Update...

Daisuke Nojiri (Code Review)

26 Feb 2020 26 Feb '20

6:10 p.m.

Daisuke Nojiri has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

vboot: Upgrade kernel space to v1.0

This patch upgrades the kernel space to v1.0 to accommodate EC hash, which is used for CrOS EC's early firmware selection.

BUG=chromium:1045217 BRANCH=none TEST=Boot Helios. Verify software sync works.

Change-Id: I525f1551afd1853cae826e87198057410167b239 Cq-Depend: chromium:2041695 Signed-off-by: dnojiri dnojiri@chromium.org --- M src/security/vboot/secdata_tpm.c 1 file changed, 4 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39137/1

diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index 96fac29..a01cf0f 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -201,7 +201,8 @@ static uint32_t set_kernel_space(const void *kernel_blob) { return set_space("kernel", KERNEL_NV_INDEX, kernel_blob, - VB2_SECDATA_KERNEL_SIZE, rw_space_attributes, NULL, 0); + VB2_SECDATA_KERNEL_SIZE_V10, rw_space_attributes, + NULL, 0); }

static uint32_t set_rec_hash_space(const uint8_t *data) @@ -329,10 +330,10 @@ /* Define and write secdata_kernel space. */ RETURN_ON_FAILURE(safe_define_space(KERNEL_NV_INDEX, TPM_NV_PER_PPWRITE, - VB2_SECDATA_KERNEL_SIZE)); + VB2_SECDATA_KERNEL_SIZE_V10)); RETURN_ON_FAILURE(write_secdata(KERNEL_NV_INDEX, ctx->secdata_kernel, - VB2_SECDATA_KERNEL_SIZE)); + VB2_SECDATA_KERNEL_SIZE_V10));

/* Define and write secdata_firmware space. */ RETURN_ON_FAILURE(safe_define_space(FIRMWARE_NV_INDEX,

Attachments:

attachment.htm (text/html — 3.5 KB)

Show replies by date

Julius Werner (Code Review)

26 Feb 26 Feb

7:16 p.m.

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39137/1/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/39137/1/src/security/vboot/secdata_... PS1, Line 204: VB2_SECDATA_KERNEL_SIZE_V10 So this is actually a good indication that we should also make a VB2_SECDATA_KERNEL_SIZE constant (without version) which will always be updated to match the size of the latest version. This code shouldn't need to change when vboot adds new fields.

https://review.coreboot.org/c/coreboot/+/39137/1/src/security/vboot/secdata_... PS1, Line 382: vb2api_secdata_kernel_create Hmm... this is a problem I didn't think about: in the v1 space we removed the UID field because it's no longer needed for TPM 2.0. But this code is still used by some people running TPM 1.2, so we cannot switch those to the v1 struct. We'll need to make a separate vb2api_secdata_kernel_create_v0() function for that case.

You'll have to move this call into _factory_initialize_tpm() so you can create a different struct depending on whether we're in the TPM2 path or not.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 1 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 26 Feb 2020 19:16:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Daisuke Nojiri (Code Review)

10:38 p.m.

Hello Aaron Durbin, Julius Werner, build bot (Jenkins), Joel Kitching,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39137

to look at the new patch set (#2).

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

vboot: Upgrade kernel space to v1.0

This patch upgrades the kernel space to v1.0 to accommodate EC hash, which is used for CrOS EC's early firmware selection.

BUG=chromium:1045217 BRANCH=none TEST=Boot Helios. Verify software sync works.

Cq-Depend: chromium:2041695 Change-Id: I525f1551afd1853cae826e87198057410167b239 Signed-off-by: dnojiri dnojiri@chromium.org --- M src/security/vboot/secdata_tpm.c 1 file changed, 6 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39137/2

Daisuke Nojiri (Code Review)

10:39 p.m.

Daisuke Nojiri has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39137/1/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/39137/1/src/security/vboot/secdata_... PS1, Line 204: VB2_SECDATA_KERNEL_SIZE_V10

...

So this is actually a good indication that we should also make a VB2_SECDATA_KERNEL_SIZE constant (w […]

Done

https://review.coreboot.org/c/coreboot/+/39137/1/src/security/vboot/secdata_... PS1, Line 382: vb2api_secdata_kernel_create

...

Hmm... […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 2 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 26 Feb 2020 22:39:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner jwerner@chromium.org Gerrit-MessageType: comment

Joel Kitching (Code Review)

27 Feb 27 Feb

5:50 a.m.

Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/c/coreboot/+/39137/2/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/39137/2/src/security/vboot/secdata_... PS2, Line 94: VB2_SECDATA_MAX_SIZE I think this is more of an implementation detail, and vboot should not have to worry about it. How about:

max(VB2_SECDATA_KERNEL_SIZE, VB2_SECDATA_FIRMWARE_SIZE)

(I think we can use KERNEL_SIZE here -- the only reason I could think of using KERNEL_MAX_SIZE is if our vb2_secdata_kernel structure *shrinks* in a minor uprev. Julius, do you think we need to be worried about that case?)

https://review.coreboot.org/c/coreboot/+/39137/2/src/security/vboot/secdata_... PS2, Line 336: VB2_SECDATA_KERNEL_SIZE This should use VB2_SECDATA_KERNEL_SIZE_V10, no?

https://review.coreboot.org/c/coreboot/+/39137/2/src/security/vboot/secdata_... PS2, Line 383: values value

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 2 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 27 Feb 2020 05:50:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Daisuke Nojiri (Code Review)

5:01 p.m.

Hello Aaron Durbin, Julius Werner, build bot (Jenkins), Joel Kitching,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39137

to look at the new patch set (#3).

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

vboot: Upgrade kernel space to v1.0

This patch upgrades the kernel space to v1.0 to accommodate EC hash, which is used for CrOS EC's early firmware selection.

BUG=chromium:1045217 BRANCH=none TEST=Boot Helios. Verify software sync works.

Cq-Depend: chromium:2041695 Change-Id: I525f1551afd1853cae826e87198057410167b239 Signed-off-by: dnojiri dnojiri@chromium.org --- M src/mainboard/google/hatch/Kconfig M src/security/vboot/secdata_tpm.c 2 files changed, 8 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39137/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 3 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Daisuke Nojiri (Code Review)

5:04 p.m.

Daisuke Nojiri has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39137/2/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/39137/2/src/security/vboot/secdata_... PS2, Line 336: VB2_SECDATA_KERNEL_SIZE

...

This should use VB2_SECDATA_KERNEL_SIZE_V10, no?

Done

https://review.coreboot.org/c/coreboot/+/39137/2/src/security/vboot/secdata_... PS2, Line 383: values

...

value

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 3 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 27 Feb 2020 17:04:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Joel Kitching kitching@google.com Gerrit-MessageType: comment

Daisuke Nojiri (Code Review)

5:38 p.m.

Daisuke Nojiri has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39137/3/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/39137/3/src/security/vboot/secdata_... PS3, Line 336: VB2_SECDATA_KERNEL_SIZE_V02 Need https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_referen... before updating 3rdparty/vboot.

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 3 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 27 Feb 2020 17:38:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Julius Werner (Code Review)

10:11 p.m.

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 3:

(2 comments)

LGTM except for the file that doesn't belong there.

https://review.coreboot.org/c/coreboot/+/39137/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/Kconfig:

https://review.coreboot.org/c/coreboot/+/39137/3/src/mainboard/google/hatch/... PS3, Line 130: select VBOOT_EARLY_EC_SYNC ???

https://review.coreboot.org/c/coreboot/+/39137/3/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/39137/3/src/security/vboot/secdata_... PS3, Line 383: /* Set initial values of secdata_firmware space. */ nit: maybe mention that kernel space is created in _factory_initialize_tpm()?

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 3 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 27 Feb 2020 22:11:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Daisuke Nojiri (Code Review)

2 Mar 2 Mar

6:18 p.m.

Hello Aaron Durbin, Julius Werner, build bot (Jenkins), Joel Kitching,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/39137

to look at the new patch set (#4).

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

vboot: Upgrade kernel space to v1.0

This patch upgrades the kernel space to v1.0 to accommodate EC hash, which is used for CrOS EC's early firmware selection.

BUG=chromium:1045217 BRANCH=none TEST=Boot Helios. Verify software sync works.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39137/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 4 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Daisuke Nojiri (Code Review)

6:19 p.m.

Daisuke Nojiri has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/39137/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/Kconfig:

https://review.coreboot.org/c/coreboot/+/39137/3/src/mainboard/google/hatch/... PS3, Line 130: select VBOOT_EARLY_EC_SYNC

...

???

restored.

https://review.coreboot.org/c/coreboot/+/39137/3/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/39137/3/src/security/vboot/secdata_... PS3, Line 383: /* Set initial values of secdata_firmware space. */

...

nit: maybe mention that kernel space is created in _factory_initialize_tpm()?

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 4 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 02 Mar 2020 18:19:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner jwerner@chromium.org Gerrit-MessageType: comment

Julius Werner (Code Review)

8:45 p.m.

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 4 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 02 Mar 2020 20:45:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Daisuke Nojiri (Code Review)

6 Mar 6 Mar

1:03 a.m.

Daisuke Nojiri has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39137/2/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/39137/2/src/security/vboot/secdata_... PS2, Line 94: VB2_SECDATA_MAX_SIZE

...

I think this is more of an implementation detail, and vboot should not have to worry about it. […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 5 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 06 Mar 2020 01:03:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Joel Kitching kitching@google.com Gerrit-MessageType: comment

Daisuke Nojiri (Code Review)

1:06 a.m.

Daisuke Nojiri has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39137/3/src/security/vboot/secdata_... File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/39137/3/src/security/vboot/secdata_... PS3, Line 336: VB2_SECDATA_KERNEL_SIZE_V02

...

Need https://chromium-review.googlesource. […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 5 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 06 Mar 2020 01:06:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Daisuke Nojiri dnojiri@chromium.org Gerrit-MessageType: comment

Joel Kitching (Code Review)

7:38 a.m.

Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 5: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 5 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 06 Mar 2020 07:38:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Georgi (Code Review)

7 Mar 7 Mar

8:45 p.m.

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39137/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/Kconfig:

https://review.coreboot.org/c/coreboot/+/39137/3/src/mainboard/google/hatch/... PS3, Line 130: select VBOOT_EARLY_EC_SYNC

...

restored.

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 5 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 07 Mar 2020 20:45:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner jwerner@chromium.org Comment-In-Reply-To: Daisuke Nojiri dnojiri@chromium.org Gerrit-MessageType: comment

Patrick Georgi (Code Review)

8:45 p.m.

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39137 )

Change subject: vboot: Upgrade kernel space to v1.0 ......................................................................

vboot: Upgrade kernel space to v1.0

This patch upgrades the kernel space to v1.0 to accommodate EC hash, which is used for CrOS EC's early firmware selection.

BUG=chromium:1045217 BRANCH=none TEST=Boot Helios. Verify software sync works.

Cq-Depend: chromium:2041695 Change-Id: I525f1551afd1853cae826e87198057410167b239 Signed-off-by: dnojiri dnojiri@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/39137 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Joel Kitching kitching@google.com Reviewed-by: Julius Werner jwerner@chromium.org --- M src/security/vboot/secdata_tpm.c 1 file changed, 11 insertions(+), 5 deletions(-)

Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Joel Kitching: Looks good to me, approved

diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index 96fac29..0ae9562 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -91,7 +91,7 @@ const uint8_t *secdata, uint32_t len) { - uint8_t sd[32]; + uint8_t sd[MAX(VB2_SECDATA_KERNEL_SIZE, VB2_SECDATA_FIRMWARE_SIZE)]; uint32_t rv; int attempts = 3;

@@ -214,6 +214,8 @@

static uint32_t _factory_initialize_tpm(struct vb2_context *ctx) { + vb2api_secdata_kernel_create(ctx); + RETURN_ON_FAILURE(tlcl_force_clear());

/* @@ -296,6 +298,8 @@ TPM_PERMANENT_FLAGS pflags; uint32_t result;

+ vb2api_secdata_kernel_create_v0(ctx); + result = tlcl_get_permanent_flags(&pflags); if (result != TPM_SUCCESS) return result; @@ -329,10 +333,10 @@ /* Define and write secdata_kernel space. */ RETURN_ON_FAILURE(safe_define_space(KERNEL_NV_INDEX, TPM_NV_PER_PPWRITE, - VB2_SECDATA_KERNEL_SIZE)); + VB2_SECDATA_KERNEL_SIZE_V02)); RETURN_ON_FAILURE(write_secdata(KERNEL_NV_INDEX, ctx->secdata_kernel, - VB2_SECDATA_KERNEL_SIZE)); + VB2_SECDATA_KERNEL_SIZE_V02));

/* Define and write secdata_firmware space. */ RETURN_ON_FAILURE(safe_define_space(FIRMWARE_NV_INDEX, @@ -376,9 +380,11 @@ { uint32_t result;

- /* Set initial values of secdata_firmware and secdata_kernel spaces. */ + /* + * Set initial values of secdata_firmware space. + * kernel space is created in _factory_initialize_tpm(). + */ vb2api_secdata_firmware_create(ctx); - vb2api_secdata_kernel_create(ctx);

VBDEBUG("TPM: factory initialization\n");

-- To view, visit https://review.coreboot.org/c/coreboot/+/39137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I525f1551afd1853cae826e87198057410167b239 Gerrit-Change-Number: 39137 Gerrit-PatchSet: 6 Gerrit-Owner: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Daisuke Nojiri dnojiri@chromium.org Gerrit-Reviewer: Joel Kitching kitching@google.com Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1668

days inactive

1678

days old

coreboot-gerrit@coreboot.org

16 comments

4 participants

tags (0)

participants (4)

Daisuke Nojiri (Code Review)
Joel Kitching (Code Review)
Julius Werner (Code Review)
Patrick Georgi (Code Review)