[coreboot-gerrit] Change in coreboot[master]: security/tpm: TCPA log follow up

[Julius Werner (Code Review)]

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

Change subject: security/tpm: TCPA log follow up
......................................................................


Patch Set 1:

(4 comments)

https://review.coreboot.org/#/c/27769/1/src/security/tpm/tspi/log.c
File src/security/tpm/tspi/log.c:

https://review.coreboot.org/#/c/27769/1/src/security/tpm/tspi/log.c@24
PS1, Line 24: static
Does this build correctly for x86 verstage? I guess it might get optimized out. Still, I think it's safer to use MAYBE_STATIC (and explicitly initialize to NULL) so correctness is not dependent on such details.


https://review.coreboot.org/#/c/27769/1/src/security/tpm/tspi/log.c@31
PS1, Line 31: NULL
Doesn't this need to be 'return ce'?


https://review.coreboot.org/#/c/27769/1/src/security/tpm/tspi/log.c@57
PS1, Line 57: 		printk(BIOS_ERR, "ERROR: No TCPA log table found\n");
Now you'll have the problem Furquan fixed again. I think you probably just want to move this error message back into tcpa_log_init() and only print it for the cbmem_add() failure (which is the only real error, all other code paths are WAI).


https://review.coreboot.org/#/c/27769/1/src/security/tpm/tspi/log.c@68
PS1, Line 68: TCPA_PCR_HASH_NAME
You need to subtract 1 here or you might not write the null terminator and get weird output in cbmem. (Or you can fix the cbmem implementation to not read more than TCPA_PCR_HASH_NAME chars from this, which I don't think it currently checks for.)



-- 
To view, visit https://review.coreboot.org/27769
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I215d79eed7ad17c6ab87f0c4b14a282e519ef07d
Gerrit-Change-Number: 27769
Gerrit-PatchSet: 1
Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki at gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Wed, 01 Aug 2018 05:02:22 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180801/0b7178cb/attachment.html>