From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52383) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TWjjI-0006d5-5v for qemu-devel@nongnu.org; Fri, 09 Nov 2012 03:14:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TWjjG-0003qA-RA for qemu-devel@nongnu.org; Fri, 09 Nov 2012 03:14:20 -0500 Received: from mail-we0-f173.google.com ([74.125.82.173]:57655) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TWjjG-0003q5-KN for qemu-devel@nongnu.org; Fri, 09 Nov 2012 03:14:18 -0500 Received: by mail-we0-f173.google.com with SMTP id t11so1583604wey.4 for ; Fri, 09 Nov 2012 00:14:18 -0800 (PST) Sender: Paolo Bonzini Message-ID: <509CBB58.1090600@redhat.com> Date: Fri, 09 Nov 2012 09:14:16 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1352398871-21705-1-git-send-email-kraxel@redhat.com> In-Reply-To: <1352398871-21705-1-git-send-email-kraxel@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Fix piix4_pm savevm buffer overflow. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org 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 > --- > 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); > } > } >