From 2d6baf19a002fd53aea727adc591be3f4843e60f Mon Sep 17 00:00:00 2001
From: nakst <>
Date: Sun, 5 Dec 2021 17:38:03 +0000
Subject: [PATCH] validate timer adjust address

---
 arch/x86_pc.cpp    |  5 ++---
 desktop/api.cpp    |  6 +++---
 kernel/syscall.cpp | 20 +++++++++++++++++---
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/arch/x86_pc.cpp b/arch/x86_pc.cpp
index 3b62ba6..c41b2fb 100644
--- a/arch/x86_pc.cpp
+++ b/arch/x86_pc.cpp
@@ -826,9 +826,8 @@ extern "C" bool PostContextSwitch(InterruptContext *context, MMSpace *oldAddress
 	currentThread->timerAdjustTicks += ProcessorReadTimeStamp() - local->currentThread->lastInterruptTimeStamp;
 
 	if (currentThread->timerAdjustAddress && MMArchIsBufferInUserRange(currentThread->timerAdjustAddress, sizeof(uint64_t))) {
-		// TODO If the MMArchSafeCopy fails, then the kernel will panic because interrupts are disabled here.
-		// 	We probably need a special version of MMArchSafeCopy that doesn't try to resolve page faults and fails faster.
-		// TODO Instead of timerAdjustAddress, maybe copy it onto a fixed location at the base of thread's stack?
+		// ES_SYSCALL_THREAD_SET_TIMER_ADJUST_ADDRESS ensures that this address is on the thread's user stack,
+		// which is managed by the kernel.
 		MMArchSafeCopy(currentThread->timerAdjustAddress, (uintptr_t) &local->currentThread->timerAdjustTicks, sizeof(uint64_t));
 	}
 
diff --git a/desktop/api.cpp b/desktop/api.cpp
index 4bd6bf8..a9ea22e 100644
--- a/desktop/api.cpp
+++ b/desktop/api.cpp
@@ -151,8 +151,6 @@ struct {
 	double performanceTimerStack[PERFORMANCE_TIMER_STACK_SIZE];
 	uintptr_t performanceTimerStackCount;
 
-	ThreadLocalStorage firstThreadLocalStorage;
-
 	EsHandle workAvailable;
 	EsMutex workMutex;
 	Array<Work> workQueue;
@@ -1481,6 +1479,8 @@ void ThreadInitialise(ThreadLocalStorage *local) {
 #include "desktop.cpp"
 
 extern "C" void _start(EsProcessStartupInformation *_startupInformation) {
+	ThreadLocalStorage threadLocalStorage;
+
 	api.startupInformation = _startupInformation;
 	bool desktop = api.startupInformation->isDesktop;
 	
@@ -1498,7 +1498,7 @@ extern "C" void _start(EsProcessStartupInformation *_startupInformation) {
 
 		_init();
 		EsRandomSeed(ProcessorReadTimeStamp());
-		ThreadInitialise(&api.firstThreadLocalStorage);
+		ThreadInitialise(&threadLocalStorage);
 		EsMessageMutexAcquire();
 
 		api.global = (GlobalData *) EsMemoryMapObject(api.startupInformation->globalDataRegion, 
diff --git a/kernel/syscall.cpp b/kernel/syscall.cpp
index 97541e8..3000636 100644
--- a/kernel/syscall.cpp
+++ b/kernel/syscall.cpp
@@ -1139,7 +1139,11 @@ SYSCALL_IMPLEMENT(ES_SYSCALL_THREAD_STACK_SIZE) {
 		MMRegion *region = MMFindAndPinRegion(currentVMM, thread->userStackBase, thread->userStackReserve);
 		KMutexAcquire(&currentVMM->reserveMutex);
 
-		if (thread->userStackCommit <= argument3 && argument3 <= thread->userStackReserve && !(argument3 & (K_PAGE_BITS - 1)) && region) {
+		if (argument3 >= K_PAGE_SIZE /* see ES_SYSCALL_THREAD_SET_TIMER_ADJUST_ADDRESS */
+				&& thread->userStackCommit <= argument3 
+				&& argument3 <= thread->userStackReserve 
+				&& !(argument3 & (K_PAGE_BITS - 1)) 
+				&& region) {
 #ifdef K_ARCH_STACK_GROWS_DOWN
 			success = MMCommitRange(currentVMM, region, (thread->userStackReserve - argument3) / K_PAGE_SIZE, argument3 / K_PAGE_SIZE); 
 #else
@@ -1320,8 +1324,18 @@ SYSCALL_IMPLEMENT(ES_SYSCALL_THREAD_SET_TLS) {
 }
 
 SYSCALL_IMPLEMENT(ES_SYSCALL_THREAD_SET_TIMER_ADJUST_ADDRESS) {
-	currentThread->timerAdjustAddress = argument0;
-	SYSCALL_RETURN(ES_SUCCESS, false);
+#ifdef K_ARCH_STACK_GROWS_DOWN
+	uintptr_t page = currentThread->userStackBase + currentThread->userStackReserve - K_PAGE_SIZE;
+#else
+	uintptr_t page = currentThread->userStackBase;
+#endif
+
+	if (argument0 >= page && argument0 <= page + K_PAGE_SIZE - sizeof(uint64_t)) {
+		currentThread->timerAdjustAddress = argument0;
+		SYSCALL_RETURN(ES_SUCCESS, false);
+	} else {
+		SYSCALL_RETURN(ES_FATAL_ERROR_INVALID_MEMORY_REGION, true);
+	}
 }
 
 SYSCALL_IMPLEMENT(ES_SYSCALL_PROCESS_GET_TLS) {