[Qemu-devel] [PATCH] acpi: fix buffer overrun on migration

[Qemu-devel] [PATCH] acpi: fix buffer overrun on migration

[Michael S. Tsirkin]

ich calls acpi_gpe_init with length ICH9_PMIO_GPE0_LEN so
ICH9_PMIO_GPE0_LEN/2 bytes are allocated, but then the full
ICH9_PMIO_GPE0_LEN bytes are migrated.

As a quick work-around, allocate twice the memory.
We'll probably want to tweak code to avoid
migrating the extra ICH9_PMIO_GPE0_LEN/2 bytes,
but that is a bit trickier to do without breaking
migration compatibility.

Tested-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Reported-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index fe6215a..21e113d 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -625,8 +625,12 @@ void acpi_pm1_cnt_reset(ACPIREGS *ar)
 void acpi_gpe_init(ACPIREGS *ar, uint8_t len)
 {
     ar->gpe.len = len;
-    ar->gpe.sts = g_malloc0(len / 2);
-    ar->gpe.en = g_malloc0(len / 2);
+    /* Only first len / 2 bytes are ever used,
+     * but the caller in ich9.c migrates full len bytes.
+     * TODO: fix ich9.c and drop the extra allocation.
+     */
+    ar->gpe.sts = g_malloc0(len);
+    ar->gpe.en = g_malloc0(len);
 }
 
 void acpi_gpe_reset(ACPIREGS *ar)
-- 
MST

Re: [Qemu-devel] [PATCH] acpi: fix buffer overrun on migration

[Igor Mammedov]

On Thu, 19 Nov 2015 15:35:00 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> ich calls acpi_gpe_init with length ICH9_PMIO_GPE0_LEN so
> ICH9_PMIO_GPE0_LEN/2 bytes are allocated, but then the full
> ICH9_PMIO_GPE0_LEN bytes are migrated.
> 
> As a quick work-around, allocate twice the memory.
> We'll probably want to tweak code to avoid
> migrating the extra ICH9_PMIO_GPE0_LEN/2 bytes,
> but that is a bit trickier to do without breaking
> migration compatibility.
> 
> Tested-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Reported-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/acpi/core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index fe6215a..21e113d 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -625,8 +625,12 @@ void acpi_pm1_cnt_reset(ACPIREGS *ar)
>  void acpi_gpe_init(ACPIREGS *ar, uint8_t len)
>  {
>      ar->gpe.len = len;
> -    ar->gpe.sts = g_malloc0(len / 2);
> -    ar->gpe.en = g_malloc0(len / 2);
> +    /* Only first len / 2 bytes are ever used,
> +     * but the caller in ich9.c migrates full len bytes.
> +     * TODO: fix ich9.c and drop the extra allocation.
> +     */
> +    ar->gpe.sts = g_malloc0(len);
> +    ar->gpe.en = g_malloc0(len);
>  }
>  
>  void acpi_gpe_reset(ACPIREGS *ar)

Re: [Qemu-devel] [PATCH] acpi: fix buffer overrun on migration

[John Snow]

On 11/19/2015 08:35 AM, Michael S. Tsirkin wrote:
> ich calls acpi_gpe_init with length ICH9_PMIO_GPE0_LEN so
> ICH9_PMIO_GPE0_LEN/2 bytes are allocated, but then the full
> ICH9_PMIO_GPE0_LEN bytes are migrated.
> 
> As a quick work-around, allocate twice the memory.
> We'll probably want to tweak code to avoid
> migrating the extra ICH9_PMIO_GPE0_LEN/2 bytes,
> but that is a bit trickier to do without breaking
> migration compatibility.
> 
> Tested-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Reported-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/acpi/core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index fe6215a..21e113d 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -625,8 +625,12 @@ void acpi_pm1_cnt_reset(ACPIREGS *ar)
>  void acpi_gpe_init(ACPIREGS *ar, uint8_t len)
>  {
>      ar->gpe.len = len;
> -    ar->gpe.sts = g_malloc0(len / 2);
> -    ar->gpe.en = g_malloc0(len / 2);
> +    /* Only first len / 2 bytes are ever used,
> +     * but the caller in ich9.c migrates full len bytes.
> +     * TODO: fix ich9.c and drop the extra allocation.
> +     */
> +    ar->gpe.sts = g_malloc0(len);
> +    ar->gpe.en = g_malloc0(len);
>  }
>  
>  void acpi_gpe_reset(ACPIREGS *ar)
> 

Reviewed-by: John Snow <jsnow@redhat.com>