From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 608E54257C; Tue, 12 Sep 2023 13:19:11 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E73A5402E2; Tue, 12 Sep 2023 13:19:10 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id CAA12402AC for ; Tue, 12 Sep 2023 13:19:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1694517550; x=1726053550; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=vhd4+mEVeZyVMVPoLfxajEakfa5k1CfmfG+uIgoC81Y=; b=Vii0rcpIZ9TxPNglY30iwxGTZgIiGFk1+w5xXmI6vKlTtHjUttbE1Xk9 tQ6qQLuKKXF5wuum6zvqXciMSq999FJQ2LJrd2Zv1eHZ2KFsSbDZAAmjY YOsEQOftkT7kG5XmQre//3sWjOwmRr263opnnDpxWnb3U2r8+DXVTGmwn znksiUZDdWQ511QEZKQy4MEpas5W1hnjRb4Z14daRQwchQ95ffCSxP66U SIpsvrCBIElPSP7/sBsMIBR37bRl55vOry+yy23xrn8R2aMhK+gDCo99x /rZJnvPftzodmPt59NDC78bJQhwmC87d0gTxGzTgUcrs1pcM2BqEopyB6 g==; X-IronPort-AV: E=McAfee;i="6600,9927,10830"; a="381045414" X-IronPort-AV: E=Sophos;i="6.02,139,1688454000"; d="scan'208";a="381045414" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Sep 2023 04:19:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10830"; a="833877274" X-IronPort-AV: E=Sophos;i="6.02,139,1688454000"; d="scan'208";a="833877274" Received: from media-ric-kahawai-icl.sh.intel.com ([10.67.119.129]) by FMSMGA003.fm.intel.com with ESMTP; 12 Sep 2023 04:18:01 -0700 From: Ric Li To: ming3.li@intel.com Cc: dev@dpdk.org, dmitry.kozliuk@gmail.com, roretzla@linux.microsoft.com Subject: [PATCH v3] windows/virt2phys: fix block MDL not updated Date: Tue, 12 Sep 2023 19:17:59 +0800 Message-Id: <20230912111759.1502806-1-ming3.li@intel.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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. 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 larger than the hugepage size (2MB). To address this, a function to check block physical address 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 block is removed and a new one is created to store the correct mapping. To make the removal action clear, the list to store MDL blocks is chenged to a double linked list. Also fix the printing of PVOID type. Bugzilla ID: 1201 Bugzilla ID: 1213 Signed-off-by: Ric Li --- v3: * Change refresh action to block removal * Change block list to double linked list v2: * Revert wrong usage of MmGetMdlStartVa windows/virt2phys/virt2phys.c | 7 +-- windows/virt2phys/virt2phys_logic.c | 70 ++++++++++++++++++++++------- 2 files changed, 57 insertions(+), 20 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..531f08c 100644 --- a/windows/virt2phys/virt2phys_logic.c +++ b/windows/virt2phys/virt2phys_logic.c @@ -12,13 +12,13 @@ struct virt2phys_process { HANDLE id; LIST_ENTRY next; - SINGLE_LIST_ENTRY blocks; + LIST_ENTRY blocks; ULONG64 memory; }; struct virt2phys_block { PMDL mdl; - SINGLE_LIST_ENTRY next; + LIST_ENTRY next; }; static struct virt2phys_params g_params; @@ -69,24 +69,28 @@ virt2phys_process_create(HANDLE process_id) struct virt2phys_process *process; process = ExAllocatePoolZero(NonPagedPool, sizeof(*process), 'pp2v'); - if (process != NULL) + if (process != NULL) { process->id = process_id; + InitializeListHead(&process->blocks); + } + return process; } static void virt2phys_process_free(struct virt2phys_process *process, BOOLEAN unmap) { - PSINGLE_LIST_ENTRY node; + PLIST_ENTRY node, next; struct virt2phys_block *block; TraceInfo("ID = %p, unmap = %!bool!", process->id, unmap); - node = process->blocks.Next; - while (node != NULL) { + for (node = process->blocks.Flink; node != &process->blocks; node = next) { + next = node->Flink; block = CONTAINING_RECORD(node, struct virt2phys_block, next); - node = node->Next; - virt2phys_block_free(block, unmap); + RemoveEntryList(&block->next); + + virt2phys_block_free(block, TRUE); } ExFreePool(process); @@ -109,10 +113,10 @@ virt2phys_process_find(HANDLE process_id) static struct virt2phys_block * virt2phys_process_find_block(struct virt2phys_process *process, PVOID virt) { - PSINGLE_LIST_ENTRY node; + PLIST_ENTRY node; struct virt2phys_block *cur; - for (node = process->blocks.Next; node != NULL; node = node->Next) { + for (node = process->blocks.Flink; node != &process->blocks; node = node->Flink) { cur = CONTAINING_RECORD(node, struct virt2phys_block, next); if (cur->mdl->StartVa == virt) return cur; @@ -182,7 +186,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; @@ -244,13 +248,13 @@ virt2phys_add_block(struct virt2phys_process *process, return STATUS_QUOTA_EXCEEDED; } - PushEntryList(&process->blocks, &block->next); + InsertHeadList(&process->blocks, &block->next); process->memory += size; return STATUS_SUCCESS; } 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 +325,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 +350,35 @@ virt2phys_unlock_memory(PMDL mdl) IoFreeMdl(mdl); } +static BOOLEAN +virt2phys_is_valid_block(struct virt2phys_block *block, PVOID base) +{ + /* + * Check if MDL in block stores the valid physical address. + * 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 remove the block and create a new one. + */ + PHYSICAL_ADDRESS block_phys, real_phys; + + block_phys = virt2phys_block_translate(block, base); + real_phys = MmGetPhysicalAddress(base); + + if (block_phys.QuadPart == real_phys.QuadPart) + return TRUE; + + TraceWarning("VA = %p, invalid block physical address %llx, valid address %llx", + base, block_phys.QuadPart, real_phys.QuadPart); + + return FALSE; +} + 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,8 +398,17 @@ virt2phys_translate(PVOID virt, PHYSICAL_ADDRESS *phys) /* Don't lock the same memory twice. */ if (block != NULL) { - *phys = virt2phys_block_translate(block, virt); - return STATUS_SUCCESS; + if (virt2phys_is_valid_block(block, base)) { + *phys = virt2phys_block_translate(block, virt); + return STATUS_SUCCESS; + } + /* Remove the invalid block. */ + KeAcquireSpinLock(g_lock, &irql); + RemoveEntryList(&block->next); + process->memory -= MmGetMdlByteCount(block->mdl); + KeReleaseSpinLock(g_lock, irql); + + virt2phys_block_free(block, TRUE); } status = virt2phys_lock_memory(base, size, &mdl); -- 2.40.1.windows.1