[dpdk-dev] [kmods PATCH v2 3/4] windows/virt2phys: add limits against resource exhaustion

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

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 <dmitry.kozliuk@gmail.com>
---
 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, &params);
 	if (!NT_SUCCESS(status))
 		return status;
 
-	status = virt2phys_init();
+	status = virt2phys_init(&params);
 	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,
+		&params->process_count_limit, PROCESS_COUNT_LIMIT_DEF);
+	if (!NT_SUCCESS(status))
+		goto cleanup;
+
+	status = virt2phys_read_mb(key, &process_memory_limit,
+		&params->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.
-- 
2.29.3