From: Ric Li <ming3.li@intel.com>
To: ming3.li@intel.com
Cc: dev@dpdk.org, dmitry.kozliuk@gmail.com
Subject: [PATCH v2] windows/virt2phys: fix block MDL not updated
Date: Mon, 11 Sep 2023 21:09:36 +0800 [thread overview]
Message-ID: <20230911130936.1485584-1-ming3.li@intel.com> (raw)
In-Reply-To: <20230911082229.1479773-1-ming3.li@intel.com>
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 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 | 54 ++++++++++++++++++++++++++---
2 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/windows/virt2phys/virt2phys.c b/windows/virt2phys/virt2phys.c
index f4d5298..b64a13d 100644
--- a/windows/virt2phys/virt2phys.c
+++ b/windows/virt2phys/virt2phys.c
@@ -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));
diff --git a/windows/virt2phys/virt2phys_logic.c b/windows/virt2phys/virt2phys_logic.c
index e3ff293..8529a2b 100644
--- a/windows/virt2phys/virt2phys_logic.c
+++ b/windows/virt2phys/virt2phys_logic.c
@@ -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;
@@ -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 unlock 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;
}
--
2.40.1.windows.1
next prev parent reply other threads:[~2023-09-11 13:09 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-11 8:22 [PATCH] " Ric Li
2023-09-11 13:09 ` Ric Li [this message]
2023-09-11 21:50 ` [PATCH v2] " Dmitry Kozlyuk
2023-09-12 11:13 ` Li, Ming3
2023-09-12 11:17 ` [PATCH v3] " Ric Li
2023-11-27 1:31 ` Li, Ming3
2023-11-30 4:10 ` Dmitry Kozlyuk
2023-12-04 10:22 ` [PATCH v4] " Ric Li
2023-12-04 10:32 ` Ric Li
2024-10-15 23:04 ` Stephen Hemminger
2024-10-16 6:32 ` Dmitry Kozlyuk
2024-11-20 12:03 ` Thomas Monjalon
2023-09-12 12:18 ` [PATCH v2] " Dmitry Kozlyuk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230911130936.1485584-1-ming3.li@intel.com \
--to=ming3.li@intel.com \
--cc=dev@dpdk.org \
--cc=dmitry.kozliuk@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).