Hello,
On 4/13/24 14:57, Akihiko Odaki wrote:
...
+static void
+virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
+ struct
virtio_gpu_simple_resource *res)
+{
+ VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+
+ if (!res->mr) {
+ return;
+ }
+
+ memory_region_set_enabled(res->mr, false);
+ memory_region_del_subregion(&b->hostmem, res->mr);
+
+ /* memory region owns res->mr object and frees it when mr is
released */
+ res->mr = NULL;
+
+ virgl_renderer_resource_unmap(res->resource_id);
Hi,
First, thanks for keeping working on this.
This patch has some changes since the previous version, but it is still
vulnerable to the race condition pointed out. The memory region is
asynchronously unmapped from the guest address space, but the backing
memory on the host address space is unmapped synchronously before that.
This results in use-after-free. The whole unmapping operation needs to
be implemented in an asynchronous manner.
Thanks for the clarification! I missed this point from the previous
discussion.
Could you please clarify what do you mean by the "asynchronous manner"?
Virglrenderer API works only in the virtio-gpu-gl context, it can't be
accessed from other places.
The memory_region_del_subregion() should remove the region as long as
nobody references it, isn't it? On Linux guest nobody should reference
hostmem regions besides virtio-gpu device on the unmap, don't know about
other guests.
We can claim it a guest's fault if MR lives after the deletion and in
that case exit Qemu with a noisy error msg or leak resource. WDYT?