The virt2phys_translate function previously scanned existing blocks,
returning the physical address from the stored MDL info if present.
This method was problematic when a virtual address pointed to a freed
and reallocated memory segment, potentially changing the physical
address mapping. Yet, virt2phys_translate would consistently return
the originally stored physical address, which could be invalid.
This issue surfaced when allocating a memory region larger than 2MB
using rte_malloc. This action would allocate a new memory segment
and use virt2phy to set the iova. The driver would store the MDL
and lock the pages initially. When this region was freed, the memory
segment used as a whole page could be freed, invalidating the virtual
to physical mapping. It then needed to be deleted from the driver's
block MDL info. Before this fix, the driver would only return the
initial physical address, leading to illegal iova for some pages when
allocating a new memory region of the same size.
To address this, a refresh function has been added. If a block with the
same base address is detected in the driver's context, the MDL's
physical address is compared with the real physical address.
If they don't match, the MDL within the block is released and rebuilt
to store the correct mapping.
Also refine the access of MDL StartVa field and fix the printing
of PVOID type.
Signed-off-by: Ric Li <ming3.li@intel.com>
---
windows/virt2phys/virt2phys.c | 7 ++--
windows/virt2phys/virt2phys_logic.c | 60 +++++++++++++++++++++++++----
2 files changed, 57 insertions(+), 10 deletions(-)
@@ -182,7 +182,7 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request)
{
WDF_REQUEST_PARAMETERS params;
ULONG code;
- PVOID *virt;
+ PVOID *pvirt, virt;
PHYSICAL_ADDRESS *phys;
size_t size;
NTSTATUS status;
@@ -207,12 +207,13 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request)
}
status = WdfRequestRetrieveInputBuffer(
- request, sizeof(*virt), (PVOID *)&virt, &size);
+ request, sizeof(*pvirt), (PVOID *)&pvirt, &size);
if (!NT_SUCCESS(status)) {
TraceWarning("Retrieving input buffer: %!STATUS!", status);
WdfRequestComplete(request, status);
return;
}
+ virt = *pvirt;
status = WdfRequestRetrieveOutputBuffer(
request, sizeof(*phys), (PVOID *)&phys, &size);
@@ -222,7 +223,7 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request)
return;
}
- status = virt2phys_translate(*virt, phys);
+ status = virt2phys_translate(virt, phys);
if (NT_SUCCESS(status))
WdfRequestSetInformation(request, sizeof(*phys));
@@ -40,7 +40,7 @@ virt2phys_block_create(PMDL mdl)
static void
virt2phys_block_free(struct virt2phys_block *block, BOOLEAN unmap)
{
- TraceInfo("VA = %p, unmap = %!bool!", block->mdl->StartVa, unmap);
+ TraceInfo("VA = %p, unmap = %!bool!", MmGetMdlBaseVa(block->mdl), unmap);
if (unmap)
MmUnlockPages(block->mdl);
@@ -114,7 +114,7 @@ virt2phys_process_find_block(struct virt2phys_process *process, PVOID virt)
for (node = process->blocks.Next; node != NULL; node = node->Next) {
cur = CONTAINING_RECORD(node, struct virt2phys_block, next);
- if (cur->mdl->StartVa == virt)
+ if (MmGetMdlBaseVa(cur->mdl) == virt)
return cur;
}
return NULL;
@@ -182,7 +182,7 @@ virt2phys_process_cleanup(HANDLE process_id)
}
static struct virt2phys_block *
-virt2phys_find_block(HANDLE process_id, void *virt,
+virt2phys_find_block(HANDLE process_id, PVOID virt,
struct virt2phys_process **process)
{
PLIST_ENTRY node;
@@ -214,7 +214,7 @@ virt2phys_add_block(struct virt2phys_process *process,
struct virt2phys_process *existing;
size_t size;
- TraceInfo("ID = %p, VA = %p", process->id, block->mdl->StartVa);
+ TraceInfo("ID = %p, VA = %p", process->id, MmGetMdlBaseVa(block->mdl));
existing = virt2phys_process_find(process->id);
*process_exists = existing != NULL;
@@ -250,7 +250,7 @@ virt2phys_add_block(struct virt2phys_process *process,
}
static NTSTATUS
-virt2phys_query_memory(void *virt, void **base, size_t *size)
+virt2phys_query_memory(PVOID virt, PVOID *base, size_t *size)
{
MEMORY_BASIC_INFORMATION info;
SIZE_T info_size;
@@ -321,7 +321,7 @@ virt2phys_check_memory(PMDL mdl)
}
static NTSTATUS
-virt2phys_lock_memory(void *virt, size_t size, PMDL *mdl)
+virt2phys_lock_memory(PVOID virt, size_t size, PMDL *mdl)
{
*mdl = IoAllocateMdl(virt, (ULONG)size, FALSE, FALSE, NULL);
if (*mdl == NULL)
@@ -346,12 +346,53 @@ virt2phys_unlock_memory(PMDL mdl)
IoFreeMdl(mdl);
}
+static NTSTATUS
+virt2phys_block_refresh(struct virt2phys_block *block, PVOID base, size_t size)
+{
+ NTSTATUS status;
+ PMDL mdl = block->mdl;
+
+ /*
+ * Check if we need to refresh MDL in block.
+ * The virtual to physical memory mapping may be changed when the
+ * virtual memory region is freed by the user process and malloc again,
+ * then we need to unlocked the physical memory and lock again to
+ * refresh the MDL information stored in block.
+ */
+ PHYSICAL_ADDRESS block_phys, real_phys;
+
+ block_phys = virt2phys_block_translate(block, base);
+ real_phys = MmGetPhysicalAddress(base);
+
+ if (block_phys.QuadPart == real_phys.QuadPart)
+ /* No need to refresh block. */
+ return STATUS_SUCCESS;
+
+ virt2phys_unlock_memory(mdl);
+ mdl = NULL;
+
+ status = virt2phys_lock_memory(base, size, &mdl);
+ if (!NT_SUCCESS(status))
+ return status;
+
+ status = virt2phys_check_memory(mdl);
+ if (!NT_SUCCESS(status)) {
+ virt2phys_unlock_memory(mdl);
+ return status;
+ }
+ block->mdl = mdl;
+
+ TraceInfo("Block refreshed from %llx to %llx", block_phys.QuadPart, real_phys.QuadPart);
+
+ return STATUS_SUCCESS;
+}
+
NTSTATUS
virt2phys_translate(PVOID virt, PHYSICAL_ADDRESS *phys)
{
PMDL mdl;
HANDLE process_id;
- void *base;
+ PVOID base;
size_t size;
struct virt2phys_process *process;
struct virt2phys_block *block;
@@ -371,6 +412,11 @@ virt2phys_translate(PVOID virt, PHYSICAL_ADDRESS *phys)
/* Don't lock the same memory twice. */
if (block != NULL) {
+ KeAcquireSpinLock(g_lock, &irql);
+ status = virt2phys_block_refresh(block, base, size);
+ KeReleaseSpinLock(g_lock, irql);
+ if (!NT_SUCCESS(status))
+ return status;
*phys = virt2phys_block_translate(block, virt);
return STATUS_SUCCESS;
}