From patchwork Wed May 26 21:01:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Kozlyuk X-Patchwork-Id: 93450 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 597AFA0546; Wed, 26 May 2021 23:02:04 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F1C31410EA; Wed, 26 May 2021 23:02:00 +0200 (CEST) Received: from mail-lf1-f43.google.com (mail-lf1-f43.google.com [209.85.167.43]) by mails.dpdk.org (Postfix) with ESMTP id DE4EC40143 for ; Wed, 26 May 2021 23:01:56 +0200 (CEST) Received: by mail-lf1-f43.google.com with SMTP id j10so4586634lfb.12 for ; Wed, 26 May 2021 14:01:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=dghZ5T77I3g105UKu5qiwPD0zDkg5hBU6lAUp+AvCAM=; b=ARzal04EjDFsLPjUnGsrDrlQeyZKUqKhj858Ynb7IMSYTNThzAHgH1f6rdjPMl+qZJ ZCHZN9SEIuQ3igrkqdOS2/9K5LjAx2A7wSZt44ZMVGeAz6W7wkx7vgq+VdXogEAnm0LM l8M3N5JT4TCRHhas9/Fp3Nck9uf++MT08ye8Ualvkdh5jkfQg9aMbSOeUHGPTXUURaTl 0I35wO1Ho/8h93BOz+oIJ4Cw+ckx/5hD1CoveoWL8Zr7tFUVDk+rQAb5SVEtBYupcOy2 67ri68Z6B4XayD2tuMYecLFIZqQfGGfG9petpWOup03tvyvIfsSk0fEn0K8DSX0CE2T1 fMAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=dghZ5T77I3g105UKu5qiwPD0zDkg5hBU6lAUp+AvCAM=; b=E4sXsbldq4w/Y1TyLp+oGx+/lDL1KEoLE+NQcn9e/e0zdV8aKrVchOshKWdvo8tr3S BhdAYxhg17ruD8suYaeYa+Z4hpd/iNXKRRNEv6rnWPAEP352UY9pwVWPNU22jMTyIoZW 7Nu7ztc6IgVpW5QYfdyyd6TKA+3ppkOt8tlGc85OoTbQLc3WlvaaztxsJdgM6fZG+lKG hYKxNtsuDvf38H3w7fgeK9aEYKxXriNftO1dMD1YwoRxqmzc9GqMm7sIeQ1eQLjOMiyB OIPE4JK2ECfVzHUt+wuTZPjhEBB12uMn0Lw72FKNhuOPjuIAjm+ivm8k58hmrR5pTNev nU5Q== X-Gm-Message-State: AOAM530UhgUIxvvemSgTTZpz+b6fGNTWymcYMrfbuLKxyxToxGO2fCGB Wn3RyFdoPk7zSGzXn2LNmFaCKEzuQU90DQ== X-Google-Smtp-Source: ABdhPJwLPmGohDIqMRhqXRaQPj56mvyQESJ8nCJWbeQYfK6UimfufKb3WbzPNbISsjYl5fECR/F1Gw== X-Received: by 2002:ac2:44cf:: with SMTP id d15mr15153lfm.385.1622062916222; Wed, 26 May 2021 14:01:56 -0700 (PDT) Received: from localhost.localdomain (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id u28sm13205lfk.172.2021.05.26.14.01.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 14:01:55 -0700 (PDT) From: Dmitry Kozlyuk To: dev@dpdk.org Cc: Dmitry Malloy , Narcisa Ana Maria Vasile , Pallavi Kadam , Tyler Retzlaff , Nick Connolly , Dmitry Kozlyuk Date: Thu, 27 May 2021 00:01:44 +0300 Message-Id: <20210526210147.1287-2-dmitry.kozliuk@gmail.com> X-Mailer: git-send-email 2.29.3 In-Reply-To: <20210526210147.1287-1-dmitry.kozliuk@gmail.com> References: <20210501171837.13282-1-dmitry.kozliuk@gmail.com> <20210526210147.1287-1-dmitry.kozliuk@gmail.com> MIME-Version: 1.0 Subject: [dpdk-dev] [kmods PATCH v2 1/4] windows/virt2phys: add PnpLockdown directive 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 Sender: "dev" WDK for Windows 10, version 2004 emits a warning suggesting to add PnpLockdown directive to INI [1]. Add it since virt2phys has no potential use-cases that require modifications to driver files. [1]: https://docs.microsoft.com/en-us/windows-hardware/drivers/install/inf-version-section Signed-off-by: Dmitry Kozlyuk --- windows/virt2phys/virt2phys.inf | 1 + 1 file changed, 1 insertion(+) diff --git a/windows/virt2phys/virt2phys.inf b/windows/virt2phys/virt2phys.inf index e35765e..7ca42f4 100644 --- a/windows/virt2phys/virt2phys.inf +++ b/windows/virt2phys/virt2phys.inf @@ -8,6 +8,7 @@ ClassGuid = {78A1C341-4539-11d3-B88D-00C04FAD5171} Provider = %ManufacturerName% CatalogFile = virt2phys.cat DriverVer = +PnpLockdown = 1 [DestinationDirs] DefaultDestDir = 12 From patchwork Wed May 26 21:01:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Kozlyuk X-Patchwork-Id: 93451 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6C6A8A0546; Wed, 26 May 2021 23:02:11 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2390641107; Wed, 26 May 2021 23:02:02 +0200 (CEST) Received: from mail-lj1-f180.google.com (mail-lj1-f180.google.com [209.85.208.180]) by mails.dpdk.org (Postfix) with ESMTP id 5B4E84069E for ; Wed, 26 May 2021 23:01:58 +0200 (CEST) Received: by mail-lj1-f180.google.com with SMTP id a4so3494760ljd.5 for ; Wed, 26 May 2021 14:01:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=S+gzKJxsH/zogb+dWFatX0JTiYtb1/g6aQpvSDfzvgA=; b=dDSSEbcfWFO5yyPxHbTe1SWdSkRn0tDgxdda3iJyw+j+F71kxxfOja0o30jTwEt8/1 mksqa4GgUfrl4C/MsVPMsGEA1jEWdH/Z1CYzSl1bA06PFxnTTeqfBMoSiB3JxGltGNSd 1Yrdc3jXiW5S3MqvqzaAZk5gwetnP3KZ0Dwb0UyUt9SGpFLz+FEnpIZYGgJLdLymWzRK e8MLOAuNhEdgY7cC/wFoahxh0C/Rws50V4kk1BNlE8dEpVWnrHQaNP07Q/uGQq/6O8IN TYgLkRv+4RQOBFyjVJ7dgb2Kjl3+H4t7G7PpyWVFEyrzhBoPo9Pb155uIiX+HXXNwNtK pIFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=S+gzKJxsH/zogb+dWFatX0JTiYtb1/g6aQpvSDfzvgA=; b=CK8nEKR/a1ZJgpmqodBH5UgrNhaR7syp2aWk3EucRiuJRaPTbEwGQ1qrQaW4m5B00q DCfaHW38XVGB2bX12aPB6AxfgdPVxlxNan9hrI1Yy380tfqxaRayKreOfFTWZ7C5uCxV x+cVGuNN0ji8ypTp/7yaugJ82NrmSkqk1M+pFwxXlAJajOWYy5EOoJJGAHGWjrB/EKRN 14TjxX2K/99C5XJ6rRmn5sn167xyla4ec3+Pjf0CZW9u6KdBLcF12uwy8ZwKmwqBGTuh ErqPaqmghagyF9HPraHKU3lusKOVcBpK54JQXA9L9j7OChi9Esb6jexQ+xyHVlwfoKkp nPNg== X-Gm-Message-State: AOAM533dhXj6p4nPqhtXF5frh5fPXxoxhcpNE1W3c8T82Y9SIAzmXWic OPzN0/ubmLMb0ljpsG2F/SuisX0iqe/9og== X-Google-Smtp-Source: ABdhPJzB3syByguTZpSpDSTPzs9BSWDbI8SO3kFNsVioAl0vKgvSbSDHugrbMAmctSczdzKHNoZjow== X-Received: by 2002:a2e:9dc3:: with SMTP id x3mr19390ljj.206.1622062917453; Wed, 26 May 2021 14:01:57 -0700 (PDT) Received: from localhost.localdomain (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id u28sm13205lfk.172.2021.05.26.14.01.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 14:01:56 -0700 (PDT) From: Dmitry Kozlyuk To: dev@dpdk.org Cc: Dmitry Malloy , Narcisa Ana Maria Vasile , Pallavi Kadam , Tyler Retzlaff , Nick Connolly , Dmitry Kozlyuk Date: Thu, 27 May 2021 00:01:45 +0300 Message-Id: <20210526210147.1287-3-dmitry.kozliuk@gmail.com> X-Mailer: git-send-email 2.29.3 In-Reply-To: <20210526210147.1287-1-dmitry.kozliuk@gmail.com> References: <20210501171837.13282-1-dmitry.kozliuk@gmail.com> <20210526210147.1287-1-dmitry.kozliuk@gmail.com> MIME-Version: 1.0 Subject: [dpdk-dev] [kmods PATCH v2 2/4] windows/virt2phys: do not expose pageable physical addresses 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 Sender: "dev" virt2phys relied on the user to ensure that memory for which physical address (PA) is obtained is non-pageable and remains such for the lifetime of the process. While DPDK does lock pages in memory, virt2phys can be accessed by any process with sufficient privileges. A malicious process could get PA and make memory pageable. When it is reused for another process, attacker can get access to private data or crash the system if another process is kernel. Solution is to lock all memory for which PA is requested. Locked blocks are tracked to unlock them when the process exits. If driver is unloaded while some memory is still locked, it leaves pages locked, effectively leaking RAM, trading stability for security. Factor out a module that locks memory for which physical addresses are obtained and keeps a list of locked memory blocks for each process. It exposes a thread-safe interface for use in driver callbacks. The driver reacts to a process exit by unlocking its tracked blocks. Also clean up the driver code. Remove debugging output to replace it with structured tracing in the next patch. Reported-by: Dmitry Malloy Signed-off-by: Dmitry Kozlyuk --- There's a couple of line length violations, but 80 characters is not a hard limit now and also those lines are ideomatic, splitting won't help readability. windows/virt2phys/virt2phys.c | 92 +++-- windows/virt2phys/virt2phys.vcxproj | 2 + windows/virt2phys/virt2phys.vcxproj.filters | 6 + windows/virt2phys/virt2phys_logic.c | 368 ++++++++++++++++++++ windows/virt2phys/virt2phys_logic.h | 32 ++ 5 files changed, 470 insertions(+), 30 deletions(-) create mode 100644 windows/virt2phys/virt2phys_logic.c create mode 100644 windows/virt2phys/virt2phys_logic.h diff --git a/windows/virt2phys/virt2phys.c b/windows/virt2phys/virt2phys.c index e157e9c..bf0c300 100644 --- a/windows/virt2phys/virt2phys.c +++ b/windows/virt2phys/virt2phys.c @@ -1,21 +1,25 @@ /* SPDX-License-Identifier: BSD-3-Clause - * Copyright(c) 2020 Dmitry Kozlyuk + * Copyright 2020-2021 Dmitry Kozlyuk */ #include #include -#include #include #include "virt2phys.h" +#include "virt2phys_logic.h" DRIVER_INITIALIZE DriverEntry; +EVT_WDF_DRIVER_UNLOAD virt2phys_driver_unload; EVT_WDF_DRIVER_DEVICE_ADD virt2phys_driver_EvtDeviceAdd; EVT_WDF_IO_IN_CALLER_CONTEXT virt2phys_device_EvtIoInCallerContext; +static VOID virt2phys_on_process_event( + HANDLE parent_id, HANDLE process_id, BOOLEAN create); + +_Use_decl_annotations_ NTSTATUS -DriverEntry( - IN PDRIVER_OBJECT driver_object, IN PUNICODE_STRING registry_path) +DriverEntry(PDRIVER_OBJECT driver_object, PUNICODE_STRING registry_path) { WDF_DRIVER_CONFIG config; WDF_OBJECT_ATTRIBUTES attributes; @@ -23,22 +27,51 @@ DriverEntry( PAGED_CODE(); - WDF_DRIVER_CONFIG_INIT(&config, virt2phys_driver_EvtDeviceAdd); WDF_OBJECT_ATTRIBUTES_INIT(&attributes); + WDF_DRIVER_CONFIG_INIT(&config, virt2phys_driver_EvtDeviceAdd); + config.EvtDriverUnload = virt2phys_driver_unload; status = WdfDriverCreate( - driver_object, registry_path, - &attributes, &config, WDF_NO_HANDLE); - if (!NT_SUCCESS(status)) { - KdPrint(("WdfDriverCreate() failed, status=%08x\n", status)); - } + driver_object, registry_path, + &attributes, &config, WDF_NO_HANDLE); + if (!NT_SUCCESS(status)) + return status; + + status = virt2phys_init(); + if (!NT_SUCCESS(status)) + return status; + + /* + * The goal is to ensure that no process obtains a physical address + * of pageable memory. To do this the driver locks every memory region + * for which physical address is requested. This memory must remain + * locked until process has no access to it anymore. A process can use + * the memory after it closes all handles to the interface device, + * so the driver cannot unlock memory at device cleanup callback. + * It has to track process termination instead, after which point + * a process cannot attempt any memory access. + */ + status = PsSetCreateProcessNotifyRoutine( + virt2phys_on_process_event, FALSE); + if (!NT_SUCCESS(status)) + return status; return status; } +_Use_decl_annotations_ +VOID +virt2phys_driver_unload(WDFDRIVER driver) +{ + UNREFERENCED_PARAMETER(driver); + + PsSetCreateProcessNotifyRoutine(virt2phys_on_process_event, TRUE); + + virt2phys_cleanup(); +} + _Use_decl_annotations_ NTSTATUS -virt2phys_driver_EvtDeviceAdd( - WDFDRIVER driver, PWDFDEVICE_INIT init) +virt2phys_driver_EvtDeviceAdd(WDFDRIVER driver, PWDFDEVICE_INIT init) { WDF_OBJECT_ATTRIBUTES attributes; WDFDEVICE device; @@ -46,8 +79,6 @@ virt2phys_driver_EvtDeviceAdd( UNREFERENCED_PARAMETER(driver); - PAGED_CODE(); - WdfDeviceInitSetIoType( init, WdfDeviceIoNeither); WdfDeviceInitSetIoInCallerContextCallback( @@ -57,15 +88,12 @@ virt2phys_driver_EvtDeviceAdd( status = WdfDeviceCreate(&init, &attributes, &device); if (!NT_SUCCESS(status)) { - KdPrint(("WdfDeviceCreate() failed, status=%08x\n", status)); return status; } status = WdfDeviceCreateDeviceInterface( - device, &GUID_DEVINTERFACE_VIRT2PHYS, NULL); + device, &GUID_DEVINTERFACE_VIRT2PHYS, NULL); if (!NT_SUCCESS(status)) { - KdPrint(("WdfDeviceCreateDeviceInterface() failed, " - "status=%08x\n", status)); return status; } @@ -74,8 +102,7 @@ virt2phys_driver_EvtDeviceAdd( _Use_decl_annotations_ VOID -virt2phys_device_EvtIoInCallerContext( - IN WDFDEVICE device, IN WDFREQUEST request) +virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request) { WDF_REQUEST_PARAMETERS params; ULONG code; @@ -85,21 +112,18 @@ virt2phys_device_EvtIoInCallerContext( NTSTATUS status; UNREFERENCED_PARAMETER(device); - PAGED_CODE(); WDF_REQUEST_PARAMETERS_INIT(¶ms); WdfRequestGetParameters(request, ¶ms); if (params.Type != WdfRequestTypeDeviceControl) { - KdPrint(("bogus request type=%u\n", params.Type)); WdfRequestComplete(request, STATUS_NOT_SUPPORTED); return; } code = params.Parameters.DeviceIoControl.IoControlCode; if (code != IOCTL_VIRT2PHYS_TRANSLATE) { - KdPrint(("bogus IO control code=%lu\n", code)); WdfRequestComplete(request, STATUS_NOT_SUPPORTED); return; } @@ -107,8 +131,6 @@ virt2phys_device_EvtIoInCallerContext( status = WdfRequestRetrieveInputBuffer( request, sizeof(*virt), (PVOID *)&virt, &size); if (!NT_SUCCESS(status)) { - KdPrint(("WdfRequestRetrieveInputBuffer() failed, " - "status=%08x\n", status)); WdfRequestComplete(request, status); return; } @@ -116,14 +138,24 @@ virt2phys_device_EvtIoInCallerContext( status = WdfRequestRetrieveOutputBuffer( request, sizeof(*phys), (PVOID *)&phys, &size); if (!NT_SUCCESS(status)) { - KdPrint(("WdfRequestRetrieveOutputBuffer() failed, " - "status=%08x\n", status)); WdfRequestComplete(request, status); return; } - *phys = MmGetPhysicalAddress(*virt); + status = virt2phys_translate(*virt, phys); + if (NT_SUCCESS(status)) + WdfRequestSetInformation(request, sizeof(*phys)); + WdfRequestComplete(request, status); +} + +static VOID +virt2phys_on_process_event( + HANDLE parent_id, HANDLE process_id, BOOLEAN create) +{ + UNREFERENCED_PARAMETER(parent_id); + + if (create) + return; - WdfRequestCompleteWithInformation( - request, STATUS_SUCCESS, sizeof(*phys)); + virt2phys_process_cleanup(process_id); } diff --git a/windows/virt2phys/virt2phys.vcxproj b/windows/virt2phys/virt2phys.vcxproj index c86cc9b..b462493 100644 --- a/windows/virt2phys/virt2phys.vcxproj +++ b/windows/virt2phys/virt2phys.vcxproj @@ -36,9 +36,11 @@ + + diff --git a/windows/virt2phys/virt2phys.vcxproj.filters b/windows/virt2phys/virt2phys.vcxproj.filters index 9e7e732..acd159f 100644 --- a/windows/virt2phys/virt2phys.vcxproj.filters +++ b/windows/virt2phys/virt2phys.vcxproj.filters @@ -27,10 +27,16 @@ Header Files + + Header Files + Source Files + + Source Files + diff --git a/windows/virt2phys/virt2phys_logic.c b/windows/virt2phys/virt2phys_logic.c new file mode 100644 index 0000000..a27802c --- /dev/null +++ b/windows/virt2phys/virt2phys_logic.c @@ -0,0 +1,368 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright 2021 Dmitry Kozlyuk + */ + +#include +#include + +#include "virt2phys_logic.h" + +struct virt2phys_process { + HANDLE id; + LIST_ENTRY next; + SINGLE_LIST_ENTRY blocks; +}; + +struct virt2phys_block { + PMDL mdl; + SINGLE_LIST_ENTRY next; +}; + +static LIST_ENTRY g_processes; +static PKSPIN_LOCK g_lock; + +struct virt2phys_block * +virt2phys_block_create(PMDL mdl) +{ + struct virt2phys_block *block; + + block = ExAllocatePoolZero(NonPagedPool, sizeof(*block), 'bp2v'); + if (block != NULL) + block->mdl = mdl; + return block; +} + +static void +virt2phys_block_free(struct virt2phys_block *block, BOOLEAN unmap) +{ + if (unmap) + MmUnlockPages(block->mdl); + + IoFreeMdl(block->mdl); + ExFreePool(block); +} + +static PHYSICAL_ADDRESS +virt2phys_block_translate(struct virt2phys_block *block, PVOID virt) +{ + PPFN_NUMBER pfn; + PVOID base; + PHYSICAL_ADDRESS phys; + + pfn = MmGetMdlPfnArray(block->mdl); + base = MmGetMdlVirtualAddress(block->mdl); + phys.QuadPart = pfn[0] * PAGE_SIZE + + ((uintptr_t)virt - (uintptr_t)base); + return phys; +} + +static struct virt2phys_process * +virt2phys_process_create(HANDLE process_id) +{ + struct virt2phys_process *process; + + process = ExAllocatePoolZero(NonPagedPool, sizeof(*process), 'pp2v'); + if (process != NULL) + process->id = process_id; + return process; +} + +static void +virt2phys_process_free(struct virt2phys_process *process, BOOLEAN unmap) +{ + PSINGLE_LIST_ENTRY node; + struct virt2phys_block *block; + + node = process->blocks.Next; + while (node != NULL) { + block = CONTAINING_RECORD(node, struct virt2phys_block, next); + node = node->Next; + virt2phys_block_free(block, unmap); + } + + ExFreePool(process); +} + +static struct virt2phys_process * +virt2phys_process_find(HANDLE process_id) +{ + PLIST_ENTRY node; + struct virt2phys_process *cur; + + for (node = g_processes.Flink; node != &g_processes; node = node->Flink) { + cur = CONTAINING_RECORD(node, struct virt2phys_process, next); + if (cur->id == process_id) + return cur; + } + return NULL; +} + +static struct virt2phys_block * +virt2phys_process_find_block(struct virt2phys_process *process, PVOID virt) +{ + PSINGLE_LIST_ENTRY node; + struct virt2phys_block *cur; + + for (node = process->blocks.Next; node != NULL; node = node->Next) { + cur = CONTAINING_RECORD(node, struct virt2phys_block, next); + if (cur->mdl->StartVa == virt) + return cur; + } + return NULL; +} + +NTSTATUS +virt2phys_init(void) +{ + g_lock = ExAllocatePoolZero(NonPagedPool, sizeof(*g_lock), 'gp2v'); + if (g_lock == NULL) + return STATUS_INSUFFICIENT_RESOURCES; + + InitializeListHead(&g_processes); + + return STATUS_SUCCESS; +} + +void +virt2phys_cleanup(void) +{ + PLIST_ENTRY node, next; + struct virt2phys_process *process; + KIRQL irql; + + KeAcquireSpinLock(g_lock, &irql); + for (node = g_processes.Flink; node != &g_processes; node = next) { + next = node->Flink; + process = CONTAINING_RECORD(node, struct virt2phys_process, next); + RemoveEntryList(&process->next); + KeReleaseSpinLock(g_lock, irql); + + virt2phys_process_free(process, FALSE); + + KeAcquireSpinLock(g_lock, &irql); + } + KeReleaseSpinLock(g_lock, irql); +} + +static struct virt2phys_process * +virt2phys_process_detach(HANDLE process_id) +{ + struct virt2phys_process *process; + + process = virt2phys_process_find(process_id); + if (process != NULL) + RemoveEntryList(&process->next); + return process; +} + +void +virt2phys_process_cleanup(HANDLE process_id) +{ + struct virt2phys_process *process; + KIRQL irql; + + KeAcquireSpinLock(g_lock, &irql); + process = virt2phys_process_detach(process_id); + KeReleaseSpinLock(g_lock, irql); + + if (process != NULL) + virt2phys_process_free(process, TRUE); +} + +static struct virt2phys_block * +virt2phys_find_block(HANDLE process_id, void *virt, + struct virt2phys_process **process) +{ + PLIST_ENTRY node; + struct virt2phys_process *cur; + + for (node = g_processes.Flink; node != &g_processes; + node = node->Flink) { + cur = CONTAINING_RECORD(node, struct virt2phys_process, next); + if (cur->id == process_id) { + *process = cur; + return virt2phys_process_find_block(cur, virt); + } + } + + *process = NULL; + return NULL; +} + +static BOOLEAN +virt2phys_exceeeds(LONG64 count, ULONG64 limit) +{ + return limit > 0 && count > (LONG64)limit; +} + +static BOOLEAN +virt2phys_add_block(struct virt2phys_process *process, + struct virt2phys_block *block) +{ + struct virt2phys_process *existing; + + existing = virt2phys_process_find(process->id); + if (existing == NULL) + InsertHeadList(&g_processes, &process->next); + else + process = existing; + + PushEntryList(&process->blocks, &block->next); + + return existing != NULL; +} + +static NTSTATUS +virt2phys_query_memory(void *virt, void **base, size_t *size) +{ + MEMORY_BASIC_INFORMATION info; + SIZE_T info_size; + NTSTATUS status; + + status = ZwQueryVirtualMemory( + ZwCurrentProcess(), virt, MemoryBasicInformation, + &info, sizeof(info), &info_size); + if (NT_SUCCESS(status)) { + *base = info.AllocationBase; + *size = info.RegionSize; + } + return status; +} + +static BOOLEAN +virt2phys_is_contiguous(PMDL mdl) +{ + PPFN_NUMBER pfn; + size_t i, pfn_count; + + pfn = MmGetMdlPfnArray(mdl); + pfn_count = ADDRESS_AND_SIZE_TO_SPAN_PAGES( + MmGetMdlVirtualAddress(mdl), MmGetMdlByteCount(mdl)); + for (i = 1; i < pfn_count; i++) { + if (pfn[i] != pfn[i - 1] + 1) + return FALSE; + } + return TRUE; +} + +static NTSTATUS +virt2phys_check_memory(PMDL mdl) +{ + MEMORY_BASIC_INFORMATION info; + SIZE_T info_size; + PVOID virt; + size_t size; + NTSTATUS status; + + if (!virt2phys_is_contiguous(mdl)) + return STATUS_UNSUCCESSFUL; + + virt = MmGetMdlVirtualAddress(mdl); + size = MmGetMdlByteCount(mdl); + status = ZwQueryVirtualMemory( + ZwCurrentProcess(), virt, MemoryBasicInformation, + &info, sizeof(info), &info_size); + if (!NT_SUCCESS(status)) + return status; + + if (info.AllocationBase != virt || info.RegionSize != size) + return STATUS_UNSUCCESSFUL; + if (info.State != MEM_COMMIT) + return STATUS_UNSUCCESSFUL; + if (info.Type != MEM_PRIVATE) + return STATUS_UNSUCCESSFUL; + return status; +} + +static NTSTATUS +virt2phys_lock_memory(void *virt, size_t size, PMDL *mdl) +{ + *mdl = IoAllocateMdl(virt, (ULONG)size, FALSE, FALSE, NULL); + if (*mdl == NULL) + return STATUS_INSUFFICIENT_RESOURCES; + + __try { + /* Future memory usage is unknown, declare RW access. */ + MmProbeAndLockPages(*mdl, UserMode, IoModifyAccess); + } + __except (EXCEPTION_EXECUTE_HANDLER) { + IoFreeMdl(*mdl); + *mdl = NULL; + return STATUS_UNSUCCESSFUL; + } + return STATUS_SUCCESS; +} + +static VOID +virt2phys_unlock_memory(PMDL mdl) +{ + MmUnlockPages(mdl); + IoFreeMdl(mdl); +} + +NTSTATUS +virt2phys_translate(PVOID virt, PHYSICAL_ADDRESS *phys) +{ + PMDL mdl; + HANDLE process_id; + void *base; + size_t size; + struct virt2phys_process *process; + struct virt2phys_block *block; + BOOLEAN created, tracked; + KIRQL irql; + NTSTATUS status; + + process_id = PsGetCurrentProcessId(); + + status = virt2phys_query_memory(virt, &base, &size); + if (!NT_SUCCESS(status)) + return status; + + KeAcquireSpinLock(g_lock, &irql); + block = virt2phys_find_block(process_id, base, &process); + KeReleaseSpinLock(g_lock, irql); + + /* Don't lock the same memory twice. */ + if (block != NULL) { + *phys = virt2phys_block_translate(block, virt); + return STATUS_SUCCESS; + } + + 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 = virt2phys_block_create(mdl); + if (block == NULL) { + virt2phys_unlock_memory(mdl); + return STATUS_INSUFFICIENT_RESOURCES; + } + + created = FALSE; + if (process == NULL) { + process = virt2phys_process_create(process_id); + if (process == NULL) { + virt2phys_block_free(block, TRUE); + return STATUS_INSUFFICIENT_RESOURCES; + } + created = TRUE; + } + + KeAcquireSpinLock(g_lock, &irql); + tracked = virt2phys_add_block(process, block); + KeReleaseSpinLock(g_lock, irql); + + /* Same process has been added concurrently, block attached to it. */ + if (tracked && created) + virt2phys_process_free(process, FALSE); + + *phys = virt2phys_block_translate(block, virt); + return STATUS_SUCCESS; +} diff --git a/windows/virt2phys/virt2phys_logic.h b/windows/virt2phys/virt2phys_logic.h new file mode 100644 index 0000000..1582206 --- /dev/null +++ b/windows/virt2phys/virt2phys_logic.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright 2021 Dmitry Kozlyuk + */ + +#ifndef VIRT2PHYS_LOGIC_H +#define VIRT2PHYS_LOGIC_H + +/** + * Initialize internal data structures. + */ +NTSTATUS virt2phys_init(void); + +/** + * Free memory allocated for internal data structures. + * Do not unlock memory so that it's not paged even if driver is unloaded + * when an application still uses this memory. + */ +void virt2phys_cleanup(void); + +/** + * Unlock all tracked memory blocks of a process. + * Free memory allocated for tracking of the process. + */ +void virt2phys_process_cleanup(HANDLE process_id); + +/** + * Lock current process memory region containing @p virt + * and get physical address corresponding to @p virt. + */ +NTSTATUS virt2phys_translate(PVOID virt, PHYSICAL_ADDRESS *phys); + +#endif /* VIRT2PHYS_LOGIC_H */ From patchwork Wed May 26 21:01:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Kozlyuk X-Patchwork-Id: 93452 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7F695A0546; Wed, 26 May 2021 23:02:18 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 638144110E; Wed, 26 May 2021 23:02:03 +0200 (CEST) Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) by mails.dpdk.org (Postfix) with ESMTP id 38B9D410E6 for ; Wed, 26 May 2021 23:01:59 +0200 (CEST) Received: by mail-lf1-f52.google.com with SMTP id j6so4597207lfr.11 for ; Wed, 26 May 2021 14:01:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=mBqvnwph9Yp2mLQXehDOCd70c64arm+VzlOlgzSx+3s=; b=DspMuVWeRTQ6ApYuoaTlp1qBUGehaKnoJoRUbvd4yf36wnizFkxilSZjtYdgRVbGL4 A0RR3x6TgjPbvkOixUW90XKetXfC/6LBJepB3i9k52BxUhj7Gjdlo0G+59/oQ0/Ad7BW GfVRaEF9KWUgz02ow0Kcv55IxAxvshLUX2VPmQNGbfWETByPPkgPfP5R7vjztDCK2Ad4 Y3XN4guQ4Tekynz4hCuQUueQnTy2qXsZDD53wmIxHELtrVCxv8q/27rLeiEMwe5P4alw cq6nNgAxkHDu1Ls7QFGaPXwFORycyLgfbvKI+cKLDOHdzB/0J1wUM7FKyjdmaaM8/6Vg p5Dg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=mBqvnwph9Yp2mLQXehDOCd70c64arm+VzlOlgzSx+3s=; b=VZHu8gN6aAlK1hbUliylFPItKTNxOx742Ez8zrthnjtvkpeg0JHegt7UnVfGLOEOOp aazI4Mxnv7yM6fH44FNdCw8QsVIHPB1bZHyav+mL6CzvpFSR9mdqKXvW3VssH4/epc4w YZi0YJZQU4oG3GZUOwusHx89qSzEHAqNXxBPZbjrsvg2l1uzTEJV44Vn5C2Al26KUob8 DZtD1XOsKGQmR81yATAPm4oENdt+wnZecWTIssgU5Oc88o4Y5Pa5+lBgHhZ1M5mkrdpG daSAtJkCopacm4ZoWg3ds7NhrTYzPsCbGABrsxKwlJZgOuh4cH/qUUepkJutg47SIwt4 LG8g== X-Gm-Message-State: AOAM533mBoXtd8nSgQX6ikjSMQE2SZLrGB1YZwLErmegdI7B7MB1C7mA VO+tpysEvzUPxHN10gJrnnnfEe86bSGhgw== X-Google-Smtp-Source: ABdhPJwlyPBJvELGAx/ysP6Z0Gn0tbCyUiRPVAucF1Jcuqvd/qF8vEAU5ORKJQOnoyi9CYI41FxxUQ== X-Received: by 2002:ac2:4281:: with SMTP id m1mr36595lfh.164.1622062918338; Wed, 26 May 2021 14:01:58 -0700 (PDT) Received: from localhost.localdomain (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id u28sm13205lfk.172.2021.05.26.14.01.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 14:01:57 -0700 (PDT) From: Dmitry Kozlyuk To: dev@dpdk.org Cc: Dmitry Malloy , Narcisa Ana Maria Vasile , Pallavi Kadam , Tyler Retzlaff , Nick Connolly , Dmitry Kozlyuk Date: Thu, 27 May 2021 00:01:46 +0300 Message-Id: <20210526210147.1287-4-dmitry.kozliuk@gmail.com> X-Mailer: git-send-email 2.29.3 In-Reply-To: <20210526210147.1287-1-dmitry.kozliuk@gmail.com> References: <20210501171837.13282-1-dmitry.kozliuk@gmail.com> <20210526210147.1287-1-dmitry.kozliuk@gmail.com> MIME-Version: 1.0 Subject: [dpdk-dev] [kmods PATCH v2 3/4] windows/virt2phys: add limits against resource exhaustion 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 Sender: "dev" Tracked processes are enumerated in process creation/termination callback, making it O(N = number of tracked processes). A malicious user could cause system-wide slowdown of process termination by making just one call to virt2phys from many processes. Limit the number of tracked processes by `ProcessCountLimit` items. Any process with access to virt2phys could exhaust RAM for all the system by locking it. It can also exhaust RAM needed for other processes with access to virt2phys. Limit amount of memory locked by each tracked process with `ProcessMemoryLimitMB`. All limits are read from registry at driver load. Each limit can be turned off by setting it to 0. Document limits and their non-zero defaults for administrators. Signed-off-by: Dmitry Kozlyuk --- windows/virt2phys/README.md | 38 +++++++++++++++ windows/virt2phys/virt2phys.c | 73 ++++++++++++++++++++++++++++- windows/virt2phys/virt2phys_logic.c | 46 +++++++++++++----- windows/virt2phys/virt2phys_logic.h | 9 +++- 4 files changed, 152 insertions(+), 14 deletions(-) create mode 100644 windows/virt2phys/README.md diff --git a/windows/virt2phys/README.md b/windows/virt2phys/README.md new file mode 100644 index 0000000..04012ff --- /dev/null +++ b/windows/virt2phys/README.md @@ -0,0 +1,38 @@ +Virtual to Physical Address Translator +====================================== + +Purpose and Operation +--------------------- + +``virt2phys`` driver allows user-mode processes to obtain physical address +of a given virtual address in their address space. +Virtual addresses must belong to regions from process private working set. +These regions must be physically contiguous. +The driver ensures that memory regions with translated addresses +are not swapped out as long as the process has access to this memory. + +It is not safe to administratively unload the driver +while there are processes that have used virt2phys to translate addresses. +Doing so will permanently leak RAM occupied by all memory regions +that contain translated addresses. +Terminate all such processes before unloading the driver. + +Configuration +------------- + +``virt2phys`` is configured at loading time via registry key +``HKLM\SYSTEM\ControlSet001\Services\virt2phys\Parameters``. + +* ``ProcessCountLimit`` (default 16) + + Maximum number of processes that can have access to memory regions + with translated addresses. When this limit is reached, no more processes + can translate addresses using ``virt2phys``. Large number of tracked + processes may slow down system operation. Set limit to 0 to disable it. + +* ``ProcessMemoryLimitMB`` (default 16384, i.e. 16 GB) + + Maximum amount of memory in all regions that contain translated addresses, + total per process. When this limit is reached, the process can not translate + addresses from new regions. Large values can cause RAM exhaustion. + Set limit to 0 to disable it. \ No newline at end of file diff --git a/windows/virt2phys/virt2phys.c b/windows/virt2phys/virt2phys.c index bf0c300..44204c9 100644 --- a/windows/virt2phys/virt2phys.c +++ b/windows/virt2phys/virt2phys.c @@ -14,15 +14,22 @@ EVT_WDF_DRIVER_UNLOAD virt2phys_driver_unload; EVT_WDF_DRIVER_DEVICE_ADD virt2phys_driver_EvtDeviceAdd; EVT_WDF_IO_IN_CALLER_CONTEXT virt2phys_device_EvtIoInCallerContext; +static NTSTATUS virt2phys_load_params( + WDFDRIVER driver, struct virt2phys_params *params); static VOID virt2phys_on_process_event( HANDLE parent_id, HANDLE process_id, BOOLEAN create); +static const ULONG PROCESS_COUNT_LIMIT_DEF = 1 << 4; +static const ULONG PROCESS_MEMORY_LIMIT_DEF = 16 * (1 << 10); /* MB */ + _Use_decl_annotations_ NTSTATUS DriverEntry(PDRIVER_OBJECT driver_object, PUNICODE_STRING registry_path) { WDF_DRIVER_CONFIG config; WDF_OBJECT_ATTRIBUTES attributes; + WDFDRIVER driver; + struct virt2phys_params params; NTSTATUS status; PAGED_CODE(); @@ -32,11 +39,15 @@ DriverEntry(PDRIVER_OBJECT driver_object, PUNICODE_STRING registry_path) config.EvtDriverUnload = virt2phys_driver_unload; status = WdfDriverCreate( driver_object, registry_path, - &attributes, &config, WDF_NO_HANDLE); + &attributes, &config, &driver); + if (!NT_SUCCESS(status)) + return status; + + status = virt2phys_load_params(driver, ¶ms); if (!NT_SUCCESS(status)) return status; - status = virt2phys_init(); + status = virt2phys_init(¶ms); if (!NT_SUCCESS(status)) return status; @@ -58,6 +69,64 @@ DriverEntry(PDRIVER_OBJECT driver_object, PUNICODE_STRING registry_path) return status; } +static NTSTATUS +virt2phys_read_param(WDFKEY key, PCUNICODE_STRING name, ULONG *value, + ULONG def) +{ + NTSTATUS status; + + status = WdfRegistryQueryULong(key, name, value); + if (status == STATUS_OBJECT_NAME_NOT_FOUND) { + *value = def; + status = STATUS_SUCCESS; + } + return status; +} + +static NTSTATUS +virt2phys_read_mb(WDFKEY key, PCUNICODE_STRING name, ULONG64 *bytes, + ULONG def_mb) +{ + ULONG mb; + NTSTATUS status; + + status = virt2phys_read_param(key, name, &mb, def_mb); + if (NT_SUCCESS(status)) + *bytes = (ULONG64)mb * (1ULL << 20); + return status; +} + +static NTSTATUS +virt2phys_load_params(WDFDRIVER driver, struct virt2phys_params *params) +{ + static DECLARE_CONST_UNICODE_STRING( + process_count_limit, L"ProcessCountLimit"); + static DECLARE_CONST_UNICODE_STRING( + process_memory_limit, L"ProcessMemoryLimitMB"); + + WDFKEY key; + NTSTATUS status; + + status = WdfDriverOpenParametersRegistryKey( + driver, KEY_READ, WDF_NO_OBJECT_ATTRIBUTES, &key); + if (!NT_SUCCESS(status)) + return status; + + status = virt2phys_read_param(key, &process_count_limit, + ¶ms->process_count_limit, PROCESS_COUNT_LIMIT_DEF); + if (!NT_SUCCESS(status)) + goto cleanup; + + status = virt2phys_read_mb(key, &process_memory_limit, + ¶ms->process_memory_limit, PROCESS_MEMORY_LIMIT_DEF); + if (!NT_SUCCESS(status)) + goto cleanup; + +cleanup: + WdfRegistryClose(key); + return status; +} + _Use_decl_annotations_ VOID virt2phys_driver_unload(WDFDRIVER driver) diff --git a/windows/virt2phys/virt2phys_logic.c b/windows/virt2phys/virt2phys_logic.c index a27802c..37b4dd4 100644 --- a/windows/virt2phys/virt2phys_logic.c +++ b/windows/virt2phys/virt2phys_logic.c @@ -11,6 +11,7 @@ struct virt2phys_process { HANDLE id; LIST_ENTRY next; SINGLE_LIST_ENTRY blocks; + ULONG64 memory; }; struct virt2phys_block { @@ -18,7 +19,9 @@ struct virt2phys_block { SINGLE_LIST_ENTRY next; }; +static struct virt2phys_params g_params; static LIST_ENTRY g_processes; +static LONG g_process_count; static PKSPIN_LOCK g_lock; struct virt2phys_block * @@ -112,7 +115,7 @@ virt2phys_process_find_block(struct virt2phys_process *process, PVOID virt) } NTSTATUS -virt2phys_init(void) +virt2phys_init(const struct virt2phys_params *params) { g_lock = ExAllocatePoolZero(NonPagedPool, sizeof(*g_lock), 'gp2v'); if (g_lock == NULL) @@ -120,6 +123,7 @@ virt2phys_init(void) InitializeListHead(&g_processes); + g_params = *params; return STATUS_SUCCESS; } @@ -165,8 +169,10 @@ virt2phys_process_cleanup(HANDLE process_id) process = virt2phys_process_detach(process_id); KeReleaseSpinLock(g_lock, irql); - if (process != NULL) + if (process != NULL) { virt2phys_process_free(process, TRUE); + InterlockedDecrement(&g_process_count); + } } static struct virt2phys_block * @@ -195,21 +201,38 @@ virt2phys_exceeeds(LONG64 count, ULONG64 limit) return limit > 0 && count > (LONG64)limit; } -static BOOLEAN +static NTSTATUS virt2phys_add_block(struct virt2phys_process *process, - struct virt2phys_block *block) + struct virt2phys_block *block, BOOLEAN *process_exists) { struct virt2phys_process *existing; + size_t size; existing = virt2phys_process_find(process->id); - if (existing == NULL) + *process_exists = existing != NULL; + if (existing == NULL) { + /* + * This check is done with the lock held so that's no race. + * Increment below must be atomic however, + * because decrement is done without holding the lock. + */ + if (virt2phys_exceeeds(g_process_count + 1, + g_params.process_count_limit)) + return STATUS_QUOTA_EXCEEDED; + InsertHeadList(&g_processes, &process->next); - else + InterlockedIncrement(&g_process_count); + } else process = existing; - PushEntryList(&process->blocks, &block->next); + size = MmGetMdlByteCount(block->mdl); + if (virt2phys_exceeeds(process->memory + size, + g_params.process_memory_limit)) + return STATUS_QUOTA_EXCEEDED; - return existing != NULL; + PushEntryList(&process->blocks, &block->next); + process->memory += size; + return STATUS_SUCCESS; } static NTSTATUS @@ -356,13 +379,14 @@ virt2phys_translate(PVOID virt, PHYSICAL_ADDRESS *phys) } KeAcquireSpinLock(g_lock, &irql); - tracked = virt2phys_add_block(process, block); + status = virt2phys_add_block(process, block, &tracked); KeReleaseSpinLock(g_lock, irql); /* Same process has been added concurrently, block attached to it. */ if (tracked && created) virt2phys_process_free(process, FALSE); - *phys = virt2phys_block_translate(block, virt); - return STATUS_SUCCESS; + if (NT_SUCCESS(status)) + *phys = virt2phys_block_translate(block, virt); + return status; } diff --git a/windows/virt2phys/virt2phys_logic.h b/windows/virt2phys/virt2phys_logic.h index 1582206..c8255f9 100644 --- a/windows/virt2phys/virt2phys_logic.h +++ b/windows/virt2phys/virt2phys_logic.h @@ -5,10 +5,17 @@ #ifndef VIRT2PHYS_LOGIC_H #define VIRT2PHYS_LOGIC_H +struct virt2phys_params { + /** Maximum number of tracked processes (0 = unlimited). */ + ULONG process_count_limit; + /** Maximum amount of memory locked by a process (0 = unlimited). */ + ULONG64 process_memory_limit; +}; + /** * Initialize internal data structures. */ -NTSTATUS virt2phys_init(void); +NTSTATUS virt2phys_init(const struct virt2phys_params *params); /** * Free memory allocated for internal data structures. From patchwork Wed May 26 21:01:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Kozlyuk X-Patchwork-Id: 93453 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D6D18A0546; Wed, 26 May 2021 23:02:24 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8AC5841115; Wed, 26 May 2021 23:02:04 +0200 (CEST) Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) by mails.dpdk.org (Postfix) with ESMTP id 0BFFE410E6 for ; Wed, 26 May 2021 23:02:00 +0200 (CEST) Received: by mail-lf1-f44.google.com with SMTP id j6so4597253lfr.11 for ; Wed, 26 May 2021 14:01:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=mHIWlvJvsetZOZSHZbPP3aO2UJoWOWIrAtPW0s2kb2U=; b=BmXOg8NIk+dppkESnfQYyGwBOMvb7aZFF807eIjuRIc5ktDljPLMKD9dIohfr1+AmB JM3mcY/DdLESxkErfOubDwin4bCUb4JGIX+BlczOoLuRN3vWqKGm+I+SXQaTXrfzgjI6 szlAy9DWyBmctX5TDdTkup3t73l4wloGuANPvyDZk8yJo6fbcvsVOmXojkTRYc1PMsAn NRtUpf1Iux/2pb5Yp/Bo/9/s4TpC8hE6mCNhLtnZVPO8qdo07FmfLAsU7d2h1hobAbJj 2YaxQepwVbn/VzeV9gtCYSbU5lF3cZbyOxSYkh3ajcovVj3v6vR/BsqD272M/RRkGOIO 5XbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=mHIWlvJvsetZOZSHZbPP3aO2UJoWOWIrAtPW0s2kb2U=; b=Khi21Yh5bEw8mHbwO0+trevNYO/97KvjoOVksvaiyBGtdpyquHhCLzw4yON5emV3Ol DuSttu1FGM4TW69s67TP/KOhcEEWfCSkFN77psfb1M2Mr1MWPkpOySFNIqyto8KjeVbT QKoRG8huvg65noAYQBe8TUIHguh/gOH5ScZ6i6pY7MYJglrd59QEqZDhfyS7UIGhinya xcBNyJsw57y5fNtONbOrFa/HRQbvDswy3MaRZrldI0YiCDZvOP7Wh7DZFq7IxZHThfOP gZtX7cqb0ugRlkb4Spy2YdE3a7EkvhbgdGPIqoOayusBZHzPb8ZLvrO3z8vqbjcrjubD GDpQ== X-Gm-Message-State: AOAM533S+o+h056h3F8p4HC2xelI7HeFBbnQpLz26TLXPoxxJItROyY3 eQKtxTOcKKAdDFYd/DRHsqNccu9pQjngVg== X-Google-Smtp-Source: ABdhPJwHi+DsGgqIOjBxByRFEP+awWEo533wHpWT+PuW2oklFThKn+c3nn5CQnEXfj0kyulhN0OS1Q== X-Received: by 2002:a05:6512:1328:: with SMTP id x40mr7973lfu.589.1622062919250; Wed, 26 May 2021 14:01:59 -0700 (PDT) Received: from localhost.localdomain (broadband-37-110-65-23.ip.moscow.rt.ru. [37.110.65.23]) by smtp.gmail.com with ESMTPSA id u28sm13205lfk.172.2021.05.26.14.01.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 14:01:58 -0700 (PDT) From: Dmitry Kozlyuk To: dev@dpdk.org Cc: Dmitry Malloy , Narcisa Ana Maria Vasile , Pallavi Kadam , Tyler Retzlaff , Nick Connolly , Dmitry Kozlyuk Date: Thu, 27 May 2021 00:01:47 +0300 Message-Id: <20210526210147.1287-5-dmitry.kozliuk@gmail.com> X-Mailer: git-send-email 2.29.3 In-Reply-To: <20210526210147.1287-1-dmitry.kozliuk@gmail.com> References: <20210501171837.13282-1-dmitry.kozliuk@gmail.com> <20210526210147.1287-1-dmitry.kozliuk@gmail.com> MIME-Version: 1.0 Subject: [dpdk-dev] [kmods PATCH v2 4/4] windows/virt2phys: add tracing 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 Sender: "dev" WPP tracing [1] allows kernel drivers to print logs that can be viewed without attaching a debugger to the running system. Traces are colelcted only when enabled. Instrument virt2phys with traces: * ERROR: failures that prevent the driver from working. * WARNING: incorrect calls to the driver. * INFO: starting or completing operations with memory. [1]: https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/wpp-software-tracing Signed-off-by: Dmitry Kozlyuk --- Comment at the bottom of virt2phys_trace.h is consumed by WPP code generator, which is the reason it has no leading asterisks. windows/virt2phys/virt2phys.c | 18 +++++++- windows/virt2phys/virt2phys.vcxproj | 5 ++- windows/virt2phys/virt2phys.vcxproj.filters | 3 ++ windows/virt2phys/virt2phys_logic.c | 35 ++++++++++++--- windows/virt2phys/virt2phys_trace.h | 50 +++++++++++++++++++++ 5 files changed, 101 insertions(+), 10 deletions(-) create mode 100644 windows/virt2phys/virt2phys_trace.h diff --git a/windows/virt2phys/virt2phys.c b/windows/virt2phys/virt2phys.c index 44204c9..f4d5298 100644 --- a/windows/virt2phys/virt2phys.c +++ b/windows/virt2phys/virt2phys.c @@ -8,6 +8,8 @@ #include "virt2phys.h" #include "virt2phys_logic.h" +#include "virt2phys_trace.h" +#include "virt2phys.tmh" DRIVER_INITIALIZE DriverEntry; EVT_WDF_DRIVER_UNLOAD virt2phys_driver_unload; @@ -66,6 +68,8 @@ DriverEntry(PDRIVER_OBJECT driver_object, PUNICODE_STRING registry_path) if (!NT_SUCCESS(status)) return status; + WPP_INIT_TRACING(driver_object, registry_path); + return status; } @@ -131,11 +135,11 @@ _Use_decl_annotations_ VOID virt2phys_driver_unload(WDFDRIVER driver) { - UNREFERENCED_PARAMETER(driver); - PsSetCreateProcessNotifyRoutine(virt2phys_on_process_event, TRUE); virt2phys_cleanup(); + + WPP_CLEANUP(WdfDriverWdmGetDriverObject(driver)); } _Use_decl_annotations_ @@ -157,12 +161,15 @@ virt2phys_driver_EvtDeviceAdd(WDFDRIVER driver, PWDFDEVICE_INIT init) status = WdfDeviceCreate(&init, &attributes, &device); if (!NT_SUCCESS(status)) { + TraceError("WdfDriverCreate() = %!STATUS!", status); return status; } status = WdfDeviceCreateDeviceInterface( device, &GUID_DEVINTERFACE_VIRT2PHYS, NULL); if (!NT_SUCCESS(status)) { + TraceError("WdfDeviceCreateDeviceInterface() = %!STATUS!", + status); return status; } @@ -187,12 +194,14 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request) WdfRequestGetParameters(request, ¶ms); if (params.Type != WdfRequestTypeDeviceControl) { + TraceWarning("Bogus IO request type %lu", params.Type); WdfRequestComplete(request, STATUS_NOT_SUPPORTED); return; } code = params.Parameters.DeviceIoControl.IoControlCode; if (code != IOCTL_VIRT2PHYS_TRANSLATE) { + TraceWarning("Bogus IO control code %lx", code); WdfRequestComplete(request, STATUS_NOT_SUPPORTED); return; } @@ -200,6 +209,7 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request) status = WdfRequestRetrieveInputBuffer( request, sizeof(*virt), (PVOID *)&virt, &size); if (!NT_SUCCESS(status)) { + TraceWarning("Retrieving input buffer: %!STATUS!", status); WdfRequestComplete(request, status); return; } @@ -207,6 +217,7 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request) status = WdfRequestRetrieveOutputBuffer( request, sizeof(*phys), (PVOID *)&phys, &size); if (!NT_SUCCESS(status)) { + TraceWarning("Retrieving output buffer: %!STATUS!", status); WdfRequestComplete(request, status); return; } @@ -214,6 +225,9 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request) status = virt2phys_translate(*virt, phys); if (NT_SUCCESS(status)) WdfRequestSetInformation(request, sizeof(*phys)); + + TraceInfo("Translate %p to %llx: %!STATUS!", + virt, phys->QuadPart, status); WdfRequestComplete(request, status); } diff --git a/windows/virt2phys/virt2phys.vcxproj b/windows/virt2phys/virt2phys.vcxproj index b462493..c9f884a 100644 --- a/windows/virt2phys/virt2phys.vcxproj +++ b/windows/virt2phys/virt2phys.vcxproj @@ -41,6 +41,7 @@ + @@ -169,9 +170,9 @@ - false + true true - trace.h + virt2phys_trace.h true diff --git a/windows/virt2phys/virt2phys.vcxproj.filters b/windows/virt2phys/virt2phys.vcxproj.filters index acd159f..6afff72 100644 --- a/windows/virt2phys/virt2phys.vcxproj.filters +++ b/windows/virt2phys/virt2phys.vcxproj.filters @@ -30,6 +30,9 @@ Header Files + + Header Files + diff --git a/windows/virt2phys/virt2phys_logic.c b/windows/virt2phys/virt2phys_logic.c index 37b4dd4..e3ff293 100644 --- a/windows/virt2phys/virt2phys_logic.c +++ b/windows/virt2phys/virt2phys_logic.c @@ -6,6 +6,8 @@ #include #include "virt2phys_logic.h" +#include "virt2phys_trace.h" +#include "virt2phys_logic.tmh" struct virt2phys_process { HANDLE id; @@ -38,6 +40,8 @@ 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); + if (unmap) MmUnlockPages(block->mdl); @@ -76,6 +80,8 @@ virt2phys_process_free(struct virt2phys_process *process, BOOLEAN unmap) PSINGLE_LIST_ENTRY node; struct virt2phys_block *block; + TraceInfo("ID = %p, unmap = %!bool!", process->id, unmap); + node = process->blocks.Next; while (node != NULL) { block = CONTAINING_RECORD(node, struct virt2phys_block, next); @@ -208,6 +214,8 @@ virt2phys_add_block(struct virt2phys_process *process, struct virt2phys_process *existing; size_t size; + TraceInfo("ID = %p, VA = %p", process->id, block->mdl->StartVa); + existing = virt2phys_process_find(process->id); *process_exists = existing != NULL; if (existing == NULL) { @@ -217,8 +225,11 @@ virt2phys_add_block(struct virt2phys_process *process, * because decrement is done without holding the lock. */ if (virt2phys_exceeeds(g_process_count + 1, - g_params.process_count_limit)) + g_params.process_count_limit)) { + TraceWarning("Process count limit reached (%lu)", + g_params.process_count_limit); return STATUS_QUOTA_EXCEEDED; + } InsertHeadList(&g_processes, &process->next); InterlockedIncrement(&g_process_count); @@ -227,8 +238,11 @@ virt2phys_add_block(struct virt2phys_process *process, size = MmGetMdlByteCount(block->mdl); if (virt2phys_exceeeds(process->memory + size, - g_params.process_memory_limit)) + g_params.process_memory_limit)) { + TraceWarning("Process %p memory limit reached (%llu bytes)", + process->id, g_params.process_memory_limit); return STATUS_QUOTA_EXCEEDED; + } PushEntryList(&process->blocks, &block->next); process->memory += size; @@ -277,8 +291,10 @@ virt2phys_check_memory(PMDL mdl) size_t size; NTSTATUS status; - if (!virt2phys_is_contiguous(mdl)) + if (!virt2phys_is_contiguous(mdl)) { + TraceWarning("Locked region is not physycally contiguous"); return STATUS_UNSUCCESSFUL; + } virt = MmGetMdlVirtualAddress(mdl); size = MmGetMdlByteCount(mdl); @@ -288,12 +304,19 @@ virt2phys_check_memory(PMDL mdl) if (!NT_SUCCESS(status)) return status; - if (info.AllocationBase != virt || info.RegionSize != size) + if (info.AllocationBase != virt || info.RegionSize != size) { + TraceWarning("Race for the region: supplied %p (%llu bytes), locked %p (%llu bytes)", + virt, size, info.AllocationBase, info.RegionSize); return STATUS_UNSUCCESSFUL; - if (info.State != MEM_COMMIT) + } + if (info.State != MEM_COMMIT) { + TraceWarning("Attempt to lock uncommitted memory"); return STATUS_UNSUCCESSFUL; - if (info.Type != MEM_PRIVATE) + } + if (info.Type != MEM_PRIVATE) { + TraceWarning("Attempt to lock shared memory"); return STATUS_UNSUCCESSFUL; + } return status; } diff --git a/windows/virt2phys/virt2phys_trace.h b/windows/virt2phys/virt2phys_trace.h new file mode 100644 index 0000000..df863cb --- /dev/null +++ b/windows/virt2phys/virt2phys_trace.h @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright 2021 Dmitry Kozlyuk + */ + +/* Tracing GUID: C5C835BB-5CFB-4757-B1D4-9DD74662E212 */ +#define WPP_CONTROL_GUIDS \ + WPP_DEFINE_CONTROL_GUID( \ + VIRT2PHYS_TRACE_GUID, \ + (C5C835BB, 5CFB, 4757, B1D4, 9DD74662E212), \ + WPP_DEFINE_BIT(TRACE_GENERAL)) + +#define WPP_FLAG_LEVEL_LOGGER(flag, level) \ + WPP_LEVEL_LOGGER(flag) + +#define WPP_FLAG_LEVEL_ENABLED(flag, level) \ + (WPP_LEVEL_ENABLED(flag) && \ + WPP_CONTROL(WPP_BIT_ ## flag).Level >= level) + +#define WPP_LEVEL_FLAGS_LOGGER(lvl, flags) \ + WPP_LEVEL_LOGGER(flags) + +#define WPP_LEVEL_FLAGS_ENABLED(lvl, flags) \ + (WPP_LEVEL_ENABLED(flags) && \ + WPP_CONTROL(WPP_BIT_ ## flags).Level >= lvl) + +/* + * WPP orders static parameters before dynamic parameters. + * To support trace functions defined below which sets FLAGS and LEVEL, + * a custom macro must be defined to reorder the arguments + * to what the .tpl configuration file expects. + */ +#define WPP_RECORDER_FLAGS_LEVEL_ARGS(flags, lvl) \ + WPP_RECORDER_LEVEL_FLAGS_ARGS(lvl, flags) +#define WPP_RECORDER_FLAGS_LEVEL_FILTER(flags, lvl) \ + WPP_RECORDER_LEVEL_FLAGS_FILTER(lvl, flags) + +/* +begin_wpp config + +USEPREFIX(TraceError, "[%!FUNC!] "); +FUNC TraceError{FLAGS=TRACE_GENERAL, LEVEL=TRACE_LEVEL_ERROR}(MSG, ...); + +USEPREFIX(TraceWarning, "[%!FUNC!] "); +FUNC TraceWarning{FLAGS=TRACE_GENERAL, LEVEL=TRACE_LEVEL_WARNING}(MSG, ...); + +USEPREFIX(TraceInfo, "[%!FUNC!] "); +FUNC TraceInfo{FLAGS=TRACE_GENERAL, LEVEL=TRACE_LEVEL_INFORMATION}(MSG, ...); + +end_wpp +*/