On Wed, Sep 05, 2018 at 05:03:03PM +0200, Roberto Sassu wrote: > On 9/5/2018 3:43 PM, Jeremy Boone wrote: > > Some comments on tpm2_pcr_read below. > > > > The tpm2_pcr_read function uses TPM2_ST_NO_SESSIONS. This means that the response payload is not integrity protected with an HMAC. If there is a man-in-the-middle sitting on the serial bus that connects the TPM peripheral to the processor, they can tamper with the response parameters. > > > > In your changes to tpm2_pcr_read, the memcpy is now become a variable-length operation, instead of just copying a fixed number of bytes. If the MITM modifies the response field out->digest_size before it is received by the driver, they can make it a very large value, forcing a buffer overflow of the out->digest array. > > > > Adding a session to the PCR Read command seems like overkill in this case. I wouldn’t recommend that as a solution here. So to fix this I would suggest simply checking the digest size before the memcpy. > > Hi Jeremy > > ok, thanks. > > Roberto Yeah, definitely not in the scope of this patch set. James Bottomley was working on sessions at some point but I'm not sure if he is still continuing that work or not. In order to get sessions everywhere we would first need to get everything to use struct tpm_buf. Tomas Winkler was working on a patch set for this but that also somehow stagnated at some point. /Jarkko
