Re: [Qemu-devel] [PATCH 3/5] tpm: tpm_passthrough: Read the buffer size from the host device

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

Hi Stefan,

On Mon, Nov 06, 2017 at 07:58:54PM -0500, Stefan Berger wrote:
> Rather than hard coding the buffer size in the tpm_passthrough
> backend read the TPM I/O buffer size from the host device.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  hw/tpm/tpm_int.h         |   9 ++++
>  hw/tpm/tpm_passthrough.c |  11 ++++-
>  hw/tpm/tpm_util.c        | 116 +++++++++++++++++++++++++++++++++++++++++++++++
>  hw/tpm/tpm_util.h        |   3 ++
>  4 files changed, 138 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
> index 1df5883..f838535 100644
> --- a/hw/tpm/tpm_int.h
> +++ b/hw/tpm/tpm_int.h
> @@ -45,11 +45,20 @@ struct tpm_resp_hdr {
>  
>  #define TPM_ORD_ContinueSelfTest  0x53
>  #define TPM_ORD_GetTicks          0xf1
> +#define TPM_ORD_GetCapability     0x65
>  
> +#define TPM_CAP_PROPERTY          0x05
> +
> +#define TPM_CAP_PROP_INPUT_BUFFER 0x124
>  
>  /* TPM2 defines */
>  #define TPM2_ST_NO_SESSIONS       0x8001
>  
>  #define TPM2_CC_ReadClock         0x00000181
> +#define TPM2_CC_GetCapability     0x0000017a
> +
> +#define TPM2_CAP_TPM_PROPERTIES   0x6
> +
> +#define TPM2_PT_INPUT_BUFFER      0x10d
>  
>  #endif /* TPM_TPM_INT_H */
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 7ff9249..ec755fe 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -57,6 +57,7 @@ struct TPMPassthruState {
>      int cancel_fd;
>  
>      TPMVersion tpm_version;
> +    uint32_t tpm_buffersize;
>  };
>  
>  typedef struct TPMPassthruState TPMPassthruState;
> @@ -201,7 +202,15 @@ static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb)
>  
>  static uint32_t tpm_passthrough_get_buffer_size(TPMBackend *tb)
>  {
> -    return 4096;
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> +    int ret;
> +
> +    ret = tpm_util_get_buffer_size(tpm_pt->tpm_fd, tpm_pt->tpm_version,
> +                                   &tpm_pt->tpm_buffersize);
> +    if (ret < 0) {
> +        tpm_pt->tpm_buffersize = 4096;
> +    }
> +    return tpm_pt->tpm_buffersize;
>  }
>  
>  /*
> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
> index 396e793..3c861ab 100644
> --- a/hw/tpm/tpm_util.c
> +++ b/hw/tpm/tpm_util.c
> @@ -20,10 +20,19 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
>  #include "tpm_util.h"
>  #include "tpm_int.h"
>  #include "exec/memory.h"
>  
> +#define DEBUG_TPM 0
> +
> +#define DPRINTF(fmt, ...) do { \
> +    if (DEBUG_TPM) { \
> +        fprintf(stderr, "tpm-util:"fmt"\n", ## __VA_ARGS__); \
> +    } \
> +} while (0)
> +
>  /*
>   * Write an error message in the given output buffer.
>   */
> @@ -170,3 +179,110 @@ int tpm_util_test_tpmdev(int tpm_fd, TPMVersion *tpm_version)
>  
>      return 1;
>  }
> +
> +int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
> +                             uint32_t *buffersize)
> +{
> +    unsigned char buf[1024];
> +    const struct tpm_req_get_buffer_size {
> +        struct tpm_req_hdr hdr;
> +        uint32_t capability;
> +        uint32_t len;
> +        uint32_t subcap;
> +    } QEMU_PACKED tpm_get_buffer_size = {
> +        .hdr = {
> +            .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
> +            .len = cpu_to_be32(sizeof(tpm_get_buffer_size)),
> +            .ordinal = cpu_to_be32(TPM_ORD_GetCapability),
> +        },
> +        .capability = cpu_to_be32(TPM_CAP_PROPERTY),
> +        .len = cpu_to_be32(sizeof(uint32_t)),
> +        .subcap = cpu_to_be32(TPM_CAP_PROP_INPUT_BUFFER),
> +    };
> +    const struct tpm2_req_get_buffer_size {
> +        struct tpm_req_hdr hdr;
> +        uint32_t capability;
> +        uint32_t property;
> +        uint32_t count;
> +    } QEMU_PACKED tpm2_get_buffer_size = {
> +        .hdr = {
> +            .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> +            .len = cpu_to_be32(sizeof(tpm2_get_buffer_size)),
> +            .ordinal = cpu_to_be32(TPM2_CC_GetCapability),
> +        },
> +        .capability = cpu_to_be32(TPM2_CAP_TPM_PROPERTIES),
> +        .property = cpu_to_be32(TPM2_PT_INPUT_BUFFER),
> +        .count = cpu_to_be32(1),
> +    };
> +    struct tpm_resp_get_buffer_size {
> +        struct tpm_resp_hdr hdr;
> +        uint32_t len;
> +        uint32_t buffersize;
> +    } QEMU_PACKED *tpm_resp = (struct tpm_resp_get_buffer_size *)buf;
> +    struct tpm2_resp_get_buffer_size {
> +        struct tpm_resp_hdr hdr;
> +        uint8_t more;
> +        uint32_t capability;
> +        uint32_t count;
> +        uint32_t property;
> +        uint32_t value;
> +    } QEMU_PACKED *tpm2_resp = (struct tpm2_resp_get_buffer_size *)buf;
> +    unsigned char *request = NULL;
> +    size_t requestlen;
> +    int ret;
> +
> +    switch (tpm_version) {

It may be more readable to split the function in 2 halfs, v1 & v2 versions.

> +    case TPM_VERSION_1_2:
> +        request = (unsigned char *)&tpm_get_buffer_size;
> +        requestlen = sizeof(tpm_get_buffer_size);
> +        break;
> +    case TPM_VERSION_2_0:
> +        request = (unsigned char *)&tpm2_get_buffer_size;
> +        requestlen = sizeof(tpm2_get_buffer_size);
> +        break;
> +    case TPM_VERSION_UNSPEC:
> +        return -EFAULT;
> +    }
> +
> +    ret = tpm_util_tx(tpm_fd, request, requestlen, buf, sizeof(buf));
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    switch (tpm_version) {
> +    case TPM_VERSION_1_2:
> +        if (be32_to_cpu(tpm_resp->hdr.len) != sizeof(*tpm_resp) ||
> +            be32_to_cpu(tpm_resp->len) != sizeof(uint32_t)) {
> +            DPRINTF("tpm_resp->hdr.len = %u, expected = %zu\n",
> +                    be32_to_cpu(tpm_resp->hdr.len), sizeof(*tpm_resp));
> +            DPRINTF("tpm_resp->len = %u, expected = %zu\n",
> +                    be32_to_cpu(tpm_resp->len), sizeof(uint32_t));
> +            error_report("tpm_util: Got malformed response to "
> +                         "TPM_GetCapability; errcode: 0x%x",
> +                         be32_to_cpu(tpm_resp->hdr.errcode));
> +            return -EFAULT;
> +        }
> +        *buffersize = be32_to_cpu(tpm_resp->buffersize);
> +        break;
> +    case TPM_VERSION_2_0:
> +        if (be32_to_cpu(tpm2_resp->hdr.len) != sizeof(*tpm2_resp) ||
> +            be32_to_cpu(tpm2_resp->count) != 1) {
> +            DPRINTF("tpm2_resp->hdr.len = %u, expected = %zu\n",
> +                    be32_to_cpu(tpm2_resp->hdr.len), sizeof(*tpm2_resp));
> +            DPRINTF("tpm2_resp->len = %u, expected = %u\n",
> +                    be32_to_cpu(tpm2_resp->count), 1);
> +            error_report("tpm_util: Got malformed response to "
> +                         "TPM2_GetCapability; errcode: 0x%x",
> +                         be32_to_cpu(tpm2_resp->hdr.errcode));
> +            return -EFAULT;
> +        }
> +        *buffersize = be32_to_cpu(tpm2_resp->value);
> +        break;
> +    case TPM_VERSION_UNSPEC:
> +        break;
> +    }
> +
> +    DPRINTF("buffersize of device: %u\n", *buffersize);
> +
> +    return 0;
> +}
> diff --git a/hw/tpm/tpm_util.h b/hw/tpm/tpm_util.h
> index aca10c9..e4fba32 100644
> --- a/hw/tpm/tpm_util.h
> +++ b/hw/tpm/tpm_util.h
> @@ -36,4 +36,7 @@ static inline uint32_t tpm_cmd_get_size(const void *b)
>      return be32_to_cpu(*(const uint32_t *)(b + 2));
>  }
>  
> +int tpm_util_get_buffer_size(int tpm_fd, TPMVersion tpm_version,
> +                             uint32_t *buffersize);
> +
>  #endif /* TPM_TPM_UTIL_H */
> -- 
> 2.5.5
> 
> 

Since you pointed out some issues with this patch, I'll review the
next iteration.