* [Qemu-devel] [PATCH v3] ARM: ACPI: Fix use-after-free due to memory realloc
@ 2018-05-30 7:05 Shannon Zhao
2018-05-30 9:03 ` Auger Eric
0 siblings, 1 reply; 3+ messages in thread
From: Shannon Zhao @ 2018-05-30 7:05 UTC (permalink / raw)
To: qemu-arm
Cc: peter.maydell, eric.auger, f4bug, qemu-devel, shannon.zhaosl,
Shannon Zhao
acpi_data_push uses g_array_set_size to resize the memory size. If there
is no enough contiguous memory, the address will be changed. So previous
pointer could not be used any more. It must update the pointer and use
the new one.
Also, previous codes wrongly use le32 conversion of iort->node_offset
for subsequent computations that will result incorrect value if host is
not litlle endian. So use the non-converted one instead.
Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
---
V3: Fix typo and add some words in commit message to explain another bug
---
hw/arm/virt-acpi-build.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 92ceee9..74f5744 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -400,7 +400,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
AcpiIortItsGroup *its;
AcpiIortTable *iort;
AcpiIortSmmu3 *smmu;
- size_t node_size, iort_length, smmu_offset = 0;
+ size_t node_size, iort_node_offset, iort_length, smmu_offset = 0;
AcpiIortRC *rc;
iort = acpi_data_push(table_data, sizeof(*iort));
@@ -413,7 +413,12 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
iort_length = sizeof(*iort);
iort->node_count = cpu_to_le32(nb_nodes);
- iort->node_offset = cpu_to_le32(sizeof(*iort));
+ /*
+ * Use a copy in case table_data->data moves during acpi_data_push
+ * operations.
+ */
+ iort_node_offset = sizeof(*iort);
+ iort->node_offset = cpu_to_le32(iort_node_offset);
/* ITS group node */
node_size = sizeof(*its) + sizeof(uint32_t);
@@ -429,7 +434,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
int irq = vms->irqmap[VIRT_SMMU];
/* SMMUv3 node */
- smmu_offset = iort->node_offset + node_size;
+ smmu_offset = iort_node_offset + node_size;
node_size = sizeof(*smmu) + sizeof(*idmap);
iort_length += node_size;
smmu = acpi_data_push(table_data, node_size);
@@ -450,7 +455,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
idmap->id_count = cpu_to_le32(0xFFFF);
idmap->output_base = 0;
/* output IORT node is the ITS group node (the first node) */
- idmap->output_reference = cpu_to_le32(iort->node_offset);
+ idmap->output_reference = cpu_to_le32(iort_node_offset);
}
/* Root Complex Node */
@@ -479,9 +484,14 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
idmap->output_reference = cpu_to_le32(smmu_offset);
} else {
/* output IORT node is the ITS group node (the first node) */
- idmap->output_reference = cpu_to_le32(iort->node_offset);
+ idmap->output_reference = cpu_to_le32(iort_node_offset);
}
+ /*
+ * Update the pointer address in case table_data->data moves during above
+ * acpi_data_push operations.
+ */
+ iort = (AcpiIortTable *)(table_data->data + iort_start);
iort->length = cpu_to_le32(iort_length);
build_header(linker, table_data, (void *)(table_data->data + iort_start),
--
2.0.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3] ARM: ACPI: Fix use-after-free due to memory realloc
2018-05-30 7:05 [Qemu-devel] [PATCH v3] ARM: ACPI: Fix use-after-free due to memory realloc Shannon Zhao
@ 2018-05-30 9:03 ` Auger Eric
2018-05-31 13:42 ` Peter Maydell
0 siblings, 1 reply; 3+ messages in thread
From: Auger Eric @ 2018-05-30 9:03 UTC (permalink / raw)
To: Shannon Zhao, qemu-arm; +Cc: peter.maydell, qemu-devel, f4bug, shannon.zhaosl
Hi Shannon,
On 05/30/2018 09:05 AM, Shannon Zhao wrote:
> acpi_data_push uses g_array_set_size to resize the memory size. If there
> is no enough contiguous memory, the address will be changed. So previous
> pointer could not be used any more. It must update the pointer and use
> the new one.
>
> Also, previous codes wrongly use le32 conversion of iort->node_offset
> for subsequent computations that will result incorrect value if host is
> not litlle endian. So use the non-converted one instead.
>
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> V3: Fix typo and add some words in commit message to explain another bug
> ---
> hw/arm/virt-acpi-build.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 92ceee9..74f5744 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -400,7 +400,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> AcpiIortItsGroup *its;
> AcpiIortTable *iort;
> AcpiIortSmmu3 *smmu;
> - size_t node_size, iort_length, smmu_offset = 0;
> + size_t node_size, iort_node_offset, iort_length, smmu_offset = 0;
> AcpiIortRC *rc;
>
> iort = acpi_data_push(table_data, sizeof(*iort));
> @@ -413,7 +413,12 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>
> iort_length = sizeof(*iort);
> iort->node_count = cpu_to_le32(nb_nodes);
> - iort->node_offset = cpu_to_le32(sizeof(*iort));
> + /*
> + * Use a copy in case table_data->data moves during acpi_data_push
> + * operations.
> + */
> + iort_node_offset = sizeof(*iort);
> + iort->node_offset = cpu_to_le32(iort_node_offset);
>
> /* ITS group node */
> node_size = sizeof(*its) + sizeof(uint32_t);
> @@ -429,7 +434,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> int irq = vms->irqmap[VIRT_SMMU];
>
> /* SMMUv3 node */
> - smmu_offset = iort->node_offset + node_size;
> + smmu_offset = iort_node_offset + node_size;
> node_size = sizeof(*smmu) + sizeof(*idmap);
> iort_length += node_size;
> smmu = acpi_data_push(table_data, node_size);
> @@ -450,7 +455,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> idmap->id_count = cpu_to_le32(0xFFFF);
> idmap->output_base = 0;
> /* output IORT node is the ITS group node (the first node) */
> - idmap->output_reference = cpu_to_le32(iort->node_offset);
> + idmap->output_reference = cpu_to_le32(iort_node_offset);
> }
>
> /* Root Complex Node */
> @@ -479,9 +484,14 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> idmap->output_reference = cpu_to_le32(smmu_offset);
> } else {
> /* output IORT node is the ITS group node (the first node) */
> - idmap->output_reference = cpu_to_le32(iort->node_offset);
> + idmap->output_reference = cpu_to_le32(iort_node_offset);
> }
>
> + /*
> + * Update the pointer address in case table_data->data moves during above
> + * acpi_data_push operations.
> + */
> + iort = (AcpiIortTable *)(table_data->data + iort_start);
> iort->length = cpu_to_le32(iort_length);
>
> build_header(linker, table_data, (void *)(table_data->data + iort_start),
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3] ARM: ACPI: Fix use-after-free due to memory realloc
2018-05-30 9:03 ` Auger Eric
@ 2018-05-31 13:42 ` Peter Maydell
0 siblings, 0 replies; 3+ messages in thread
From: Peter Maydell @ 2018-05-31 13:42 UTC (permalink / raw)
To: Auger Eric
Cc: Shannon Zhao, qemu-arm, QEMU Developers,
Philippe Mathieu-Daudé, Shannon Zhao
On 30 May 2018 at 10:03, Auger Eric <eric.auger@redhat.com> wrote:
> Hi Shannon,
>
> On 05/30/2018 09:05 AM, Shannon Zhao wrote:
>> acpi_data_push uses g_array_set_size to resize the memory size. If there
>> is no enough contiguous memory, the address will be changed. So previous
>> pointer could not be used any more. It must update the pointer and use
>> the new one.
>>
>> Also, previous codes wrongly use le32 conversion of iort->node_offset
>> for subsequent computations that will result incorrect value if host is
>> not litlle endian. So use the non-converted one instead.
>>
>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-05-31 13:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-30 7:05 [Qemu-devel] [PATCH v3] ARM: ACPI: Fix use-after-free due to memory realloc Shannon Zhao
2018-05-30 9:03 ` Auger Eric
2018-05-31 13:42 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).