Re: [PATCH] tpm: fixed be_buffer_size size in in tpm_crb

[Philippe Mathieu-Daudé]

+Marc-André

On 12/25/21 13:38, Yuri Konotopov wrote:
> Trying to boot VM with TPM 2.0 CRB in passthrough mode without this change
> I got "Requested buffer size of 3968 is smaller than host TPM's fixed
> buffer size of 4096".
> Looks like it can not be less than backend buffer size nor less than
> CRB_CTRL_CMD_SIZE.
> 
> Signed-off-by: Yuri Konotopov <ykonotopov@gnome.org>
> ---
>  hw/tpm/tpm_crb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index 58ebd1469c..8243645453 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -270,7 +270,7 @@ static void tpm_crb_reset(void *dev)
>      s->regs[R_CRB_CTRL_RSP_SIZE] = CRB_CTRL_CMD_SIZE;
>      s->regs[R_CRB_CTRL_RSP_ADDR] = TPM_CRB_ADDR_BASE + A_CRB_DATA_BUFFER;
>  
> -    s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->tpmbe),
> +    s->be_buffer_size = MAX(tpm_backend_get_buffer_size(s->tpmbe),
>                              CRB_CTRL_CMD_SIZE);
>
>      if (tpm_backend_startup_tpm(s->tpmbe, s->be_buffer_size) < 0) {

This doesn't look correct: if the backend buffer size is smaller,
we can not use a bigger size, otherwise we might end up overflowing
the buffer.

What about checking the backend buffer size at realization?
Could the backend change this size on reset?

-- >8 --
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index 58ebd1469c3..57346eaa857 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -270,9 +270,6 @@ static void tpm_crb_reset(void *dev)
     s->regs[R_CRB_CTRL_RSP_SIZE] = CRB_CTRL_CMD_SIZE;
     s->regs[R_CRB_CTRL_RSP_ADDR] = TPM_CRB_ADDR_BASE + A_CRB_DATA_BUFFER;

-    s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->tpmbe),
-                            CRB_CTRL_CMD_SIZE);
-
     if (tpm_backend_startup_tpm(s->tpmbe, s->be_buffer_size) < 0) {
         exit(1);
     }
@@ -290,6 +287,12 @@ static void tpm_crb_realize(DeviceState *dev, Error
**errp)
         error_setg(errp, "'tpmdev' property is required");
         return;
     }
+    s->be_buffer_size = tpm_backend_get_buffer_size(s->tpmbe);
+    if (s->be_buffer_size < CRB_CTRL_CMD_SIZE) {
+        error_setg(errp, "'tpmdev' buffer size too small (%zu, minimum:
%u)",
+                   s->be_buffer_size, CRB_CTRL_CMD_SIZE);
+        return;
+    }

     memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s,
         "tpm-crb-mmio", sizeof(s->regs));
---