xen/arch/x86/tpm.c: add CRB interface support + response validation fix#28
xen/arch/x86/tpm.c: add CRB interface support + response validation fix#28accek-itl wants to merge 2 commits into
Conversation
| * header (10) | parameterSize (4) | TPML_DIGEST_VALUES | authArea | ||
| * finish_r overlays: h (10) | paramSize (4) | hashCount (4) | hashes[] | ||
| */ | ||
| if ( o_size < sizeof(cmd_rsp.finish_r) + sizeof(uint32_t) ) |
There was a problem hiding this comment.
Why sizeof(uint32_t)? That's first hash alg and 2 first bytes of the hash.
There was a problem hiding this comment.
Rewrote these checks for o_size. Also changed the returned value to -1 to make it consistent with the other tmp2_hash_extend below.
| uint32_t data_buf_pa = TPM_BASE + CRB_DATA_BUFFER_(loc); | ||
| unsigned expected; | ||
|
|
||
| crb_cmd_ready(loc); |
There was a problem hiding this comment.
Not sure why, but coreboot's driver follows this with a wait for no error in CRB_CTRL_STS_ and then a wait for CRB_CTRL_START_ being zero.
There was a problem hiding this comment.
Hmm, doubt if transitioning to ready state could end up with a non-recoverable error, but why not check it. Better than an infinite loop waiting for successful start below, so added here too.
For wait for start being zero - it may be more coreboot-specific, as this is a wait for a previous operation to complete. Incomplete operation should not happen after successful SINIT. But added nevertheless, doesn't hurt.
| tpm_write32(CRB_CTRL_RSP_ADDR_(loc), data_buf_pa); | ||
| tpm_write32(CRB_CTRL_RSP_ADDR_(loc) + 4, 0); | ||
|
|
||
| memcpy(__va(data_buf_pa), buf, i_size); |
There was a problem hiding this comment.
No check for i_size being too large.
There was a problem hiding this comment.
Fixed (added at the beginning of the function).
| } | ||
|
|
||
| /* Read header to learn the response length. */ | ||
| memcpy(buf, __va(data_buf_pa), sizeof(struct tpm_rsp_hdr)); |
There was a problem hiding this comment.
*o_size could be too small here.
There was a problem hiding this comment.
Fixed (added at the beginning of the function).
| static void crb_send_cmd(unsigned loc, uint8_t *buf, unsigned i_size, | ||
| unsigned *o_size) | ||
| { | ||
| uint32_t data_buf_pa = TPM_BASE + CRB_DATA_BUFFER_(loc); |
There was a problem hiding this comment.
Should make no difference but upstream will likely prefer this version:
| uint32_t data_buf_pa = TPM_BASE + CRB_DATA_BUFFER_(loc); | |
| paddr_t data_buf_pa = TPM_BASE + CRB_DATA_BUFFER_(loc); |
Assisted-by: Claude:claude-opus-4-6 Signed-off-by: Szymon Acedański <accek@invisiblethingslab.com>
Assisted-by: Claude:claude-opus-4-6 Signed-off-by: Szymon Acedański <accek@invisiblethingslab.com>
3 participants
No description provided.