Re: [Qemu-devel] [PATCH] Fix piix4_pm savevm buffer overflow.

[Paolo Bonzini]

Il 08/11/2012 19:21, Gerd Hoffmann ha scritto:
> vmstate will read/write 8 bytes for the gpe bits, whereas only two bytes
> are allocated (and used).  So make sure we allocate enougth to not
> overrun the buffer on savevm and loadvm.
> 
> Fixing vmstate would probably be better but that has the unpleasent side
> effect of breaking migration.
> 
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  hw/acpi.c |   10 ++++++++--
>  savevm.c  |    6 ++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi.c b/hw/acpi.c
> index f4aca49..6ed76ff 100644
> --- a/hw/acpi.c
> +++ b/hw/acpi.c
> @@ -406,9 +406,15 @@ void acpi_pm1_cnt_reset(ACPIREGS *ar)
>  /* ACPI GPE */
>  void acpi_gpe_init(ACPIREGS *ar, uint8_t len)
>  {
> +    /*
> +     * Hack alert: Although we are using only two bytes (GPE_LEN / 2)
> +     * for each of "sts" and "en" we have to allocate more because
> +     * VMSTATE_GPE_ARRAY() writes 8 bytes (GPE_LEN * sizeof(uint16_t))
> +     * to the vmstate stream.
> +     */
>      ar->gpe.len = len;
> -    ar->gpe.sts = g_malloc0(len / 2);
> -    ar->gpe.en = g_malloc0(len / 2);
> +    ar->gpe.sts = g_malloc0(len * 2);
> +    ar->gpe.en = g_malloc0(len * 2);

If the other 6 bytes are really unused, please use VMSTATE_UNUSED or
VMSTATE_UNUSED_V to skip them in the vmstate stream.

(Also, debug stuff below).

Paolo

>  }
>  
>  void acpi_gpe_blk(ACPIREGS *ar, uint32_t blk)
> diff --git a/savevm.c b/savevm.c
> index 5d04d59..be0257e 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1511,6 +1511,8 @@ void vmstate_save_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>  {
>      VMStateField *field = vmsd->fields;
>  
> +    fprintf(stderr, "%s: %s\n", __func__, vmsd->name);
> +
>      if (vmsd->pre_save) {
>          vmsd->pre_save(opaque);
>      }
> @@ -1521,6 +1523,8 @@ void vmstate_save_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>              int i, n_elems = 1;
>              int size = field->size;
>  
> +            fprintf(stderr, "%s:   %s\n", __func__, field->name);
> +
>              if (field->flags & VMS_VBUFFER) {
>                  size = *(int32_t *)(opaque+field->size_offset);
>                  if (field->flags & VMS_MULTIPLY) {
> @@ -1550,6 +1554,8 @@ void vmstate_save_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>                  if (field->flags & VMS_STRUCT) {
>                      vmstate_save_state(f, field->vmsd, addr);
>                  } else {
> +                    fprintf(stderr, "%s:     a %p s %d %d/%d\n", __func__,
> +                            addr, size, i, n_elems);
>                      field->info->put(f, addr, size);
>                  }
>              }
>