On Wed, Oct 16, 2024 at 01:51:29PM -0400, Stefan Berger wrote:
> Error responses from swtpm are typically only 4 bytes long with the
> exception of a few commands that return more bytes. Therefore, read the
> entire response in 2 steps and stop if the first few bytes indicate an
> error response with no subsequent bytes readable. Read the rest in a 2nd
> step, if needed. This avoids getting stuck while waiting for too many
> bytes in case of an error. 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 <stef...@linux.ibm.com>
> ---
> backends/tpm/tpm_emulator.c | 61 +++++++++++++++++++++++++++----------
> 1 file changed, 45 insertions(+), 16 deletions(-)
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
>
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index b0e2fb3fc7..8ad54f49a5 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -123,12 +123,14 @@ static const char *tpm_emulator_strerror(uint32_t
> tpm_result)
> }
>
> static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void
> *msg,
> - size_t msg_len_in, size_t msg_len_out)
> + size_t msg_len_in, size_t msg_len_out_err,
> + size_t msg_len_out_total)
> {
> CharBackend *dev = &tpm->ctrl_chr;
> uint32_t cmd_no = cpu_to_be32(cmd);
> ssize_t n = sizeof(uint32_t) + msg_len_in;
> uint8_t *buf = NULL;
> + ptm_res res;
>
> WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
> buf = g_alloca(n);
> @@ -140,8 +142,24 @@ 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);
> + if (msg_len_out_total > 0) {
> + assert(msg_len_out_total >= msg_len_out_err);
> +
> + n = qemu_chr_fe_read_all(dev, (uint8_t *)msg, msg_len_out_err);
> + if (n <= 0) {
> + return -1;
> + }
> + if (msg_len_out_err == msg_len_out_total) {
> + return 0;
> + }
> + /* result error code is always in the first 4 bytes */
> + memcpy(&res, msg, sizeof(res));
Before this memcpy we need
assert(sizeof(res) <= msg_len_out_err);
to sanity-check we're not reading un-initialized memory in 'msg',
or worse, reading out of bounds.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
