Re: [PATCH 2/2] tpm_emulator: Read control channel response in 2 passes

[Stefan Berger <stefanb@linux.ibm.com>]

On 10/11/24 6:35 PM, Stefan Berger wrote:
> Error responses from swtpm are always only 4 bytes long. Therefore, read
> the entire response in 2 passes and stop if the first 4 bytes indicate an
> error response with no subsequent bytes readable. Read the rest in a 2nd
> pass, if needed. This avoids getting stuck while waiting for too many bytes
> if only 4 bytes were sent due to an error message. The 'getting stuck'
> condition has not been observed in practice so far, though.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2615
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>   backends/tpm/tpm_emulator.c | 25 +++++++++++++++++++++++--
>   1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index b0e2fb3fc7..1b2e890668 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -129,6 +129,9 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void *msg,
>       uint32_t cmd_no = cpu_to_be32(cmd);
>       ssize_t n = sizeof(uint32_t) + msg_len_in;
>       uint8_t *buf = NULL;
> +    ptm_res res;
> +    off_t o = 0;
> +    int to_read;
>   
>       WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
>           buf = g_alloca(n);
> @@ -140,11 +143,29 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void *msg,
>               return -1;
>           }
>   
> -        if (msg_len_out != 0) {
> -            n = qemu_chr_fe_read_all(dev, msg, msg_len_out);
> +        /*
> +         * Any response will be at least 4 bytes long. Error responses are only
> +         * 4 bytes (=sizeof(ptm_res)), though. Therefore, read response in 2
> +         * passes, returning when an error response is encountered.
> +         */

v2: We need one exception here and that's CMD_GET_STATEBLOB since it has 
been sending a longer header since basically 'forever'.

> +        to_read = sizeof(res);
> +        while (msg_len_out > 0) {
> +            n = qemu_chr_fe_read_all(dev, (uint8_t *)msg + o, to_read);
>               if (n <= 0) {
>                   return -1;
>               }
> +            msg_len_out -= to_read;
> +            if (msg_len_out == 0) {
> +                return 0;
> +            }
> +
> +            memcpy(&res, msg, sizeof(res));
> +            if (res) {
> +                return 0;
> +            }
> +
> +            o = to_read;
> +            to_read = msg_len_out;
>           }
>       }
>