From patchwork Sat May 1 17:18:36 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Kozlyuk X-Patchwork-Id: 92588 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 A4AFFA0546; Sat, 1 May 2021 19:19:01 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C66FD410EF; Sat, 1 May 2021 19:18:50 +0200 (CEST) Received: from mail-lj1-f179.google.com (mail-lj1-f179.google.com [209.85.208.179]) by mails.dpdk.org (Postfix) with ESMTP id 3F6354013F for ; Sat, 1 May 2021 19:18:48 +0200 (CEST) Received: by mail-lj1-f179.google.com with SMTP id l22so1875306ljc.9 for ; Sat, 01 May 2021 10:18:48 -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=LTb3qoB9gTGD4db/phPTifUAGCf5evbTSe4YB9bkDEc=; b=VEUROqCDOfM4FpZZ2iTRJFCfp1oEh2mQtZrzd2tX9W3uTUVoOberFWFw71CC7JaV6J /xVxoywbkgQ59bZhgv0AcsCk3j+ch3JAvIjxcLelhx9z6kieas7BQILqwWKW8ywe9QgH hBUndyNiF6UwelqsP1LRZQ/wNxi0MayP0Qfz0achfsHypKzH8Y4QeIJtKjJ0WWelLQe9 uM34S906U0sK70tiMMexeATpfBcCpbyrbdYGEwTxS58EB7zsB0PzaAtKxd/ABCz60kKy QRbnDSe11+iqoVqUb8UAo5CHzIi2GgTGCWYCAVb1gdLObD9gXFUBxbZzKSEnmXnVMjs6 NZ0Q== 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=LTb3qoB9gTGD4db/phPTifUAGCf5evbTSe4YB9bkDEc=; b=dPARALMZvtRLhgEI8xwy4XkNH9NxvZ4hZDbFBX8t6FQO1YMImqCU9IzQOpv70pc0oH HhBoqFjhMTj0WH2gTEa8vmGokLQJvv1h4PgNLFhBnKBBOkB4mLQh+H5Y6cwPXU851iFm c8MIfA4KEAeHf0S75bOo0jLL6Uuo/PESU6ur/Zwh6DAlukDB9HuLR77MguLGIwZjbryb aD49j46FmCQpuNZTdUqyw3rt7RIrvdUSnW2SoozsMqa9RXRz0b/tLWfXQRxmiTCnKkYI DILrYNDvN/TLFP9W2XTUAK8V7salP8MF6iHtqw+9LmSB/RMKtq8AG49+tP6TNq+LqP4P DZNA== X-Gm-Message-State: AOAM531RghpElZxkgiDX5M9BSgPmetVxS52XM4pZdEBLFTYosAfEbPXo BmTtPjAWng6wQAa20IquQG5/oaHOZHGRrA== X-Google-Smtp-Source: ABdhPJzyXmkrRexKLrlGJGaZOr3XjQKTIZYGX/6zruwL+DvWMQp0a5m7QjB8gztJ5pBvVFSoNEgnxg== X-Received: by 2002:a2e:6e03:: with SMTP id j3mr7414162ljc.218.1619889527462; Sat, 01 May 2021 10:18:47 -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 i1sm616703lfe.53.2021.05.01.10.18.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 01 May 2021 10:18:46 -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: Sat, 1 May 2021 20:18:36 +0300 Message-Id: <20210501171837.13282-3-dmitry.kozliuk@gmail.com> X-Mailer: git-send-email 2.29.3 In-Reply-To: <20210501171837.13282-1-dmitry.kozliuk@gmail.com> References: <20210501171837.13282-1-dmitry.kozliuk@gmail.com> MIME-Version: 1.0 Subject: [dpdk-dev] [kmods PATCH 2/3] 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 --- windows/virt2phys/virt2phys.c | 75 +++-- windows/virt2phys/virt2phys.vcxproj | 2 + windows/virt2phys/virt2phys.vcxproj.filters | 8 +- windows/virt2phys/virt2phys_logic.c | 304 ++++++++++++++++++++ windows/virt2phys/virt2phys_logic.h | 32 +++ 5 files changed, 394 insertions(+), 27 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..0c05fe3 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,42 @@ 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)); + return status; } + status = virt2phys_init(); + if (!NT_SUCCESS(status)) + return status; + + 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 +70,6 @@ virt2phys_driver_EvtDeviceAdd( UNREFERENCED_PARAMETER(driver); - PAGED_CODE(); - WdfDeviceInitSetIoType( init, WdfDeviceIoNeither); WdfDeviceInitSetIoInCallerContextCallback( @@ -57,15 +79,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 +93,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 +103,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 +122,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 +129,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 e5ce5fe..294f086 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..6b65d71 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 + - + \ No newline at end of file diff --git a/windows/virt2phys/virt2phys_logic.c b/windows/virt2phys/virt2phys_logic.c new file mode 100644 index 0000000..155d34f --- /dev/null +++ b/windows/virt2phys/virt2phys_logic.c @@ -0,0 +1,304 @@ +/* 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 = ExAllocatePool(NonPagedPool, sizeof(*block)); + if (block != NULL) { + RtlZeroMemory(block, sizeof(*block)); + 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 struct virt2phys_process * +virt2phys_process_create(HANDLE process_id) +{ + struct virt2phys_process *process; + + process = ExAllocatePool(NonPagedPool, sizeof(*process)); + if (process != NULL) { + RtlZeroMemory(process, sizeof(*process)); + 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 BOOLEAN +virt2phys_process_has_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 TRUE; + } + return FALSE; +} + +NTSTATUS +virt2phys_init(void) +{ + g_lock = ExAllocatePool(NonPagedPool, sizeof(*g_lock)); + 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 BOOLEAN +virt2phys_has_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_has_block(cur, virt); + } + } + + *process = NULL; + return FALSE; +} + +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)) + return status; + if (info.State == MEM_FREE) + return STATUS_MEMORY_NOT_ALLOCATED; + + *base = info.AllocationBase; + *size = info.RegionSize; + 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 locked, 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); + locked = virt2phys_has_block(process_id, base, &process); + KeReleaseSpinLock(g_lock, irql); + + /* Don't lock the same memory twice. */ + if (locked) { + *phys = MmGetPhysicalAddress(virt); + return STATUS_SUCCESS; + } + + status = virt2phys_lock_memory(base, size, &mdl); + if (!NT_SUCCESS(status)) + 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 = MmGetPhysicalAddress(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 */