diff mbox series

exec.c: Don't reallocate IOMMUNotifiers that are in use

Message ID 20190128174241.5860-1-peter.maydell@linaro.org
State New
Headers show
Series exec.c: Don't reallocate IOMMUNotifiers that are in use | expand

Commit Message

Peter Maydell Jan. 28, 2019, 5:42 p.m. UTC 
The tcg_register_iommu_notifier() code has a GArray of
TCGIOMMUNotifier structs which it has registered by passing
memory_region_register_iommu_notifier() a pointer to the embedded
IOMMUNotifier field. Unfortunately, if we need to enlarge the
array via g_array_set_size() this can cause a realloc(), which
invalidates the pointer that memory_region_register_iommu_notifier()
put into the MemoryRegion's iommu_notify list. This can result
in segfaults.

Switch the GArray to holding pointers to the TCGIOMMUNotifier
structs, so that we can individually allocate and free them.

Cc: qemu-stable@nongnu.org
Fixes: 1f871c5e6b0f30644a60a ("exec.c: Handle IOMMUs in address_space_translate_for_iotlb()")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I don't know why I've suddenly started running into this; possibly
it's because of a recent Ubuntu upgrade and so a newer glibc.
---
 exec.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Richard Henderson Jan. 29, 2019, 2:41 p.m. UTC | #1 
On 1/28/19 9:42 AM, Peter Maydell wrote:
> The tcg_register_iommu_notifier() code has a GArray of
> TCGIOMMUNotifier structs which it has registered by passing
> memory_region_register_iommu_notifier() a pointer to the embedded
> IOMMUNotifier field. Unfortunately, if we need to enlarge the
> array via g_array_set_size() this can cause a realloc(), which
> invalidates the pointer that memory_region_register_iommu_notifier()
> put into the MemoryRegion's iommu_notify list. This can result
> in segfaults.
> 
> Switch the GArray to holding pointers to the TCGIOMMUNotifier
> structs, so that we can individually allocate and free them.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 1f871c5e6b0f30644a60a ("exec.c: Handle IOMMUs in address_space_translate_for_iotlb()")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I don't know why I've suddenly started running into this; possibly
> it's because of a recent Ubuntu upgrade and so a newer glibc.
> ---
>  exec.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

It is odd that we have not seen this before...


r~
diff mbox series

Patch

diff --git a/exec.c b/exec.c
index 895449f9261..cf6bc72c976 100644
--- a/exec.c
+++ b/exec.c
@@ -665,7 +665,7 @@  static void tcg_register_iommu_notifier(CPUState *cpu,
     int i;
 
     for (i = 0; i < cpu->iommu_notifiers->len; i++) {
-        notifier = &g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier, i);
+        notifier = g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier *, i);
         if (notifier->mr == mr && notifier->iommu_idx == iommu_idx) {
             break;
         }
@@ -673,7 +673,8 @@  static void tcg_register_iommu_notifier(CPUState *cpu,
     if (i == cpu->iommu_notifiers->len) {
         /* Not found, add a new entry at the end of the array */
         cpu->iommu_notifiers = g_array_set_size(cpu->iommu_notifiers, i + 1);
-        notifier = &g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier, i);
+        notifier = g_new0(TCGIOMMUNotifier, 1);
+        g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier *, i) = notifier;
 
         notifier->mr = mr;
         notifier->iommu_idx = iommu_idx;
@@ -705,8 +706,9 @@  static void tcg_iommu_free_notifier_list(CPUState *cpu)
     TCGIOMMUNotifier *notifier;
 
     for (i = 0; i < cpu->iommu_notifiers->len; i++) {
-        notifier = &g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier, i);
+        notifier = g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier *, i);
         memory_region_unregister_iommu_notifier(notifier->mr, &notifier->n);
+        g_free(notifier);
     }
     g_array_free(cpu->iommu_notifiers, true);
 }
@@ -976,7 +978,7 @@  void cpu_exec_realizefn(CPUState *cpu, Error **errp)
         vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
     }
 
-    cpu->iommu_notifiers = g_array_new(false, true, sizeof(TCGIOMMUNotifier));
+    cpu->iommu_notifiers = g_array_new(false, true, sizeof(TCGIOMMUNotifier *));
 #endif
 }