qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [QEMU PATCH v5 07/13] softmmu/memory: enable automatic deallocation


From: Xenia Ragiadakou
Subject: Re: [QEMU PATCH v5 07/13] softmmu/memory: enable automatic deallocation of memory regions
Date: Tue, 19 Sep 2023 13:28:24 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0


On 15/9/23 18:11, Akihiko Odaki wrote:
On 2023/09/15 20:11, Huang Rui wrote:
From: Xenia Ragiadakou <xenia.ragiadakou@amd.com>

When the memory region has a different life-cycle from that of her parent, could be automatically released, once has been unparent and once all of her
references have gone away, via the object's free callback.

However, currently, references to the memory region are held by its owner
without first incrementing the memory region object's reference count.
As a result, the automatic deallocation of the object, not taking into
account those references, results in use-after-free memory corruption.

This patch increases the reference count of an owned memory region object on each memory_region_ref() and decreases it on each memory_region_unref().

Signed-off-by: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
---

V4 -> V5:
     - ref/unref only owned memory regions (Akihiko)

  softmmu/memory.c | 5 +++++
  1 file changed, 5 insertions(+)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index 7d9494ce70..15e1699750 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1800,6 +1800,9 @@ void memory_region_ref(MemoryRegion *mr)
      /* MMIO callbacks most likely will access data that belongs
       * to the owner, hence the need to ref/unref the owner whenever
       * the memory region is in use.
+     * Likewise, the owner keeps references to the memory region,
+     * hence the need to ref/unref the memory region object to prevent
+     * its automatic deallocation while still referenced by its owner.

This comment does not make sense. Traditionally no such automatic deallocation happens so the owner has been always required to free the memory region when it gets finalized.

"[QEMU PATCH v5 09/13] virtio-gpu: Handle resource blob commands" introduces a different kind of memory region, which can be freed anytime before the device gets finalized. Even in this case, the owner removes the reference to the memory owner by doing res->region = NULL;

Hi Akihiko,

You are right, the word "owner" is not correct. The issue observed was due to the references kept in flatview ranges and the fact that flatview_destroy() is asynchronous and was called after memory region's destruction.

If I replace the word "owner" with "memory subsystem" in the commit message and drop the comment, would that be ok with you? or do want to suggest something else?

Xenia



Regards,
Akihiko Odaki



reply via email to

[Prev in Thread] Current Thread [Next in Thread]