tpm: fixed be_buffer_size size in in tpm_crb

[PATCH] tpm: fixed be_buffer_size size in in tpm_crb

Posted by Yuri Konotopov 2 years, 4 months ago

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) {
-- 
2.32.0

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

Posted by Philippe Mathieu-Daudé 2 years, 4 months ago

+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));
---

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

Posted by Stefan Berger 2 years, 4 months ago

On 12/25/21 07: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".

I suppose the host has a TIS interface.

The reason it gives this message is that the response this TPM may send 
back could be 4096 bytes in size but the CRB of the VM can only catch 
3968 bytes, so there's a mismatch. You may not be able to use the CRB in 
passthrough mode. I would try to have the VM use the TIS.

    Stefan


> 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) {

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

Posted by Stefan Berger 2 years, 4 months ago

On 12/26/21 21:24, Stefan Berger wrote:
>
> On 12/25/21 07: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".
>
> I suppose the host has a TIS interface.
>
> The reason it gives this message is that the response this TPM may 
> send back could be 4096 bytes in size but the CRB of the VM can only 
> catch 3968 bytes, so there's a mismatch. You may not be able to use 
> the CRB in passthrough mode. I would try to have the VM use the TIS.


For TPM passthrough the host TPM's buffer size basically has to match 
the VM's buffer size so that

- apps inside the VM cannot create longer commands than what the host 
device can accept

- apps inside the VM cannot create commands that cause the TPM to return 
responses that are bigger than what the VM's TPM buffer can accept

   Stefan

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

Posted by Yuri Konotopov 2 years, 4 months ago

27.12.2021 06:24, Stefan Berger пишет:
> I suppose the host has a TIS interface.

Hello, Stefan.


I do not think so. There is only tpm_crb tpm kernel module compiled in 
my system

# systemd-cryptenroll --tpm2-device=list
PATH        DEVICE      DRIVER
/dev/tpmrm0 MSFT0101:00 tpm_crb


>
> The reason it gives this message is that the response this TPM may 
> send back could be 4096 bytes in size but the CRB of the VM can only 
> catch 3968 bytes, so there's a mismatch. You may not be able to use 
> the CRB in passthrough mode. I would try to have the VM use the TIS.

-- 
Best regards, Yuri Konotopov