]> Projects (at) Tadryanom (dot) Me - AdrOS.git/commitdiff
fix: 5 HIGH severity bugs from audit
authorTulio A M Mendes <[email protected]>
Tue, 10 Feb 2026 11:38:03 +0000 (08:38 -0300)
committerTulio A M Mendes <[email protected]>
Fri, 13 Feb 2026 02:20:50 +0000 (23:20 -0300)
2.3: Slab allocator now uses kmalloc(PAGE_SIZE) instead of
     pmm_alloc_page + hal_mm_phys_to_virt. The old approach could
     map physical addresses above 16MB to VAs that collide with the
     heap range (0xD0000000+), causing silent memory corruption.

3.3: execve now validates sp against stack base before each write.
     Prevents writing below the user stack page if E2BIG pre-check
     is somehow bypassed. Returns -E2BIG on underflow.

3.4: SMEP (Supervisor Mode Execution Prevention) enabled in CR4
     if CPU supports it. Prevents kernel from executing user-mapped
     pages, blocking a common exploit technique. SMAP detection added
     but not enabled yet (requires STAC/CLAC in uaccess.c first).
     CPUID leaf 7 detection added for SMEP (bit 7) and SMAP (bit 20).

4.1: Kernel heap now grows dynamically from 10MB up to 64MB max.
     When kmalloc can't find a free block, kheap_grow maps new
     physical pages at the end of the heap and creates a new free
     block. Coalesces with tail if adjacent and free.

2.4: process_waitpid circular list traversal now checks for NULL
     before comparing to start, preventing NULL deref if the list
     is broken by concurrent reaping.

include/arch/x86/cpuid.h
src/arch/x86/cpuid.c
src/hal/x86/cpu_features.c
src/kernel/scheduler.c
src/kernel/syscall.c
src/mm/heap.c
src/mm/slab.c

index f033db942053171d403f846eb23480b1c4d0eb6d..c847a2f8d2518b188264044add04caa7484587bf 100644 (file)
@@ -41,6 +41,10 @@ struct x86_cpu_features {
     uint8_t lm        : 1;    /* Long Mode (64-bit) */
     uint8_t syscall   : 1;    /* SYSCALL/SYSRET */
 
+    /* CPUID leaf 7 — EBX (structured extended features) */
+    uint8_t smep      : 1;    /* Supervisor Mode Execution Prevention */
+    uint8_t smap      : 1;    /* Supervisor Mode Access Prevention */
+
     /* Extended info */
     uint32_t max_ext_leaf;
     char brand[49];           /* CPU brand string (leaves 0x80000002-4) */
index 1f590bd7619b6b9f718497a3cf54f97955d34dfc..9e3d7f4d8708ef52b17f30893d2dcbafaaefb9b9 100644 (file)
@@ -66,6 +66,13 @@ void x86_cpuid_detect(struct x86_cpu_features* out) {
     out->initial_apic_id = (uint8_t)(ebx >> 24);
     out->logical_cpus    = (uint8_t)(ebx >> 16);
 
+    /* Leaf 7: structured extended features (SMEP, SMAP, etc.) */
+    if (out->max_leaf >= 7) {
+        cpuid(7, &eax, &ebx, &ecx, &edx);
+        out->smep = (ebx >> 7)  & 1;
+        out->smap = (ebx >> 20) & 1;
+    }
+
     /* Extended leaves */
     cpuid(0x80000000, &eax, &ebx, &ecx, &edx);
     out->max_ext_leaf = eax;
@@ -125,6 +132,8 @@ void x86_cpuid_print(const struct x86_cpu_features* f) {
     if (f->x2apic) uart_print(" x2APIC");
     if (f->hypervisor) uart_print(" HYPERVISOR");
     if (f->syscall) uart_print(" SYSCALL");
+    if (f->smep) uart_print(" SMEP");
+    if (f->smap) uart_print(" SMAP");
     uart_print("\n");
 
     uart_print("[CPUID] APIC ID: ");
index 3a561f585eae42e14e44e7094819443ce5b997e8..662df62aa25bb6f0834e4c28b8e037b19283a99e 100644 (file)
@@ -1,8 +1,22 @@
 #include "hal/cpu_features.h"
 #include "arch/x86/cpuid.h"
+#include "uart_console.h"
 
 #include <stddef.h>
 
+#define CR4_SMEP (1U << 20)
+#define CR4_SMAP (1U << 21)
+
+static inline uint32_t read_cr4(void) {
+    uint32_t val;
+    __asm__ volatile("mov %%cr4, %0" : "=r"(val));
+    return val;
+}
+
+static inline void write_cr4(uint32_t val) {
+    __asm__ volatile("mov %0, %%cr4" :: "r"(val) : "memory");
+}
+
 static struct cpu_features g_features;
 static struct x86_cpu_features g_x86_features;
 
@@ -34,6 +48,21 @@ void hal_cpu_detect_features(void) {
 
     g_features.logical_cpus  = g_x86_features.logical_cpus;
     g_features.initial_cpu_id = g_x86_features.initial_apic_id;
+
+    /* Enable SMEP if supported: prevents kernel from executing user-mapped pages.
+     * This blocks a common exploit technique where an attacker maps shellcode in
+     * userspace and tricks the kernel into jumping to it. */
+    if (g_x86_features.smep) {
+        uint32_t cr4 = read_cr4();
+        cr4 |= CR4_SMEP;
+        write_cr4(cr4);
+        uart_print("[CPU] SMEP enabled.\n");
+    }
+
+    /* SMAP (Supervisor Mode Access Prevention) is NOT enabled yet because
+     * copy_from_user/copy_to_user do not bracket accesses with STAC/CLAC.
+     * Enabling SMAP without STAC/CLAC would fault on every user memory access.
+     * TODO: Add STAC/CLAC to x86 uaccess.c, then enable SMAP here. */
 }
 
 const struct cpu_features* hal_cpu_get_features(void) {
index f3a02fceb49da86230c162e7f426b8a6de1af94a..45ff1c818123f874a41d9304a940b014f91218b0 100644 (file)
@@ -261,7 +261,7 @@ int process_waitpid(int pid, int* status_out, uint32_t options) {
                     }
                 }
                 it = it->next;
-            } while (it != start);
+            } while (it && it != start);
         }
 
         if (!found_child) {
index 7395e206ed01f21221e5bbb6831517718a997aa6..486dbf30b68346e375be76ad2c777577adb29722 100644 (file)
@@ -684,12 +684,14 @@ static int syscall_execve_impl(struct registers* regs, const char* user_path, co
     // The loader returns a fresh stack top (user_sp). We'll pack strings below it.
     uintptr_t sp = user_sp;
     sp &= ~(uintptr_t)0xF;
+    const uintptr_t sp_base = user_sp - user_stack_size;
 
     uintptr_t argv_ptrs_va[EXECVE_MAX_ARGC + 1];
     uintptr_t envp_ptrs_va[EXECVE_MAX_ENVC + 1];
 
     for (int i = envc - 1; i >= 0; i--) {
         size_t len = strlen(kenvp[i]) + 1;
+        if (sp - len < sp_base) { vmm_as_activate(old_as); current_process->addr_space = old_as; vmm_as_destroy(new_as); ret = -E2BIG; goto out; }
         sp -= len;
         memcpy((void*)sp, kenvp[i], len);
         envp_ptrs_va[i] = sp;
@@ -698,6 +700,7 @@ static int syscall_execve_impl(struct registers* regs, const char* user_path, co
 
     for (int i = argc - 1; i >= 0; i--) {
         size_t len = strlen(kargv[i]) + 1;
+        if (sp - len < sp_base) { vmm_as_activate(old_as); current_process->addr_space = old_as; vmm_as_destroy(new_as); ret = -E2BIG; goto out; }
         sp -= len;
         memcpy((void*)sp, kargv[i], len);
         argv_ptrs_va[i] = sp;
index e5c7315280f0c914fb55e03f5499384a997aa540..fcf53bcc6c70ca077acc981bd8b9f8cee7d30283 100644 (file)
@@ -11,6 +11,7 @@
 // Heap starts at 3GB + 256MB
 #define KHEAP_START 0xD0000000
 #define KHEAP_INITIAL_SIZE (10 * 1024 * 1024) // 10MB
+#define KHEAP_MAX_SIZE    (64 * 1024 * 1024) // 64MB max growth
 #define PAGE_SIZE 4096
 
 #define HEAP_MAGIC 0xCAFEBABE
@@ -26,6 +27,7 @@ typedef struct heap_header {
 
 static heap_header_t* head = NULL;
 static heap_header_t* tail = NULL;
+static uintptr_t heap_end = 0; // Current end of mapped heap region
 
 static spinlock_t heap_lock = {0};
 
@@ -74,6 +76,7 @@ void kheap_init(void) {
     head->prev = NULL;
     
     tail = head;
+    heap_end = KHEAP_START + KHEAP_INITIAL_SIZE;
     spin_unlock_irqrestore(&heap_lock, flags);
 
     uart_print("[HEAP] 10MB Heap Ready.\n");
@@ -136,8 +139,90 @@ void* kmalloc(size_t size) {
         current = current->next;
     }
     
+    // No free block found — try to grow the heap.
+    size_t grow_size = aligned_size + sizeof(heap_header_t);
+    // Round up to page boundary, minimum 64KB growth.
+    if (grow_size < 64 * 1024) grow_size = 64 * 1024;
+    grow_size = (grow_size + PAGE_SIZE - 1) & ~(size_t)(PAGE_SIZE - 1);
+
+    if (heap_end + grow_size > KHEAP_START + KHEAP_MAX_SIZE) {
+        spin_unlock_irqrestore(&heap_lock, flags);
+        uart_print("[HEAP] OOM: max heap size reached.\n");
+        return NULL;
+    }
+
+    // Map new pages.
+    uintptr_t map_addr = heap_end;
+    for (size_t off = 0; off < grow_size; off += PAGE_SIZE) {
+        void* phys_frame = pmm_alloc_page();
+        if (!phys_frame) {
+            // Partial growth: use whatever we mapped so far.
+            grow_size = off;
+            break;
+        }
+        vmm_map_page((uint64_t)(uintptr_t)phys_frame, (uint64_t)(map_addr + off),
+                     VMM_FLAG_PRESENT | VMM_FLAG_RW);
+    }
+
+    if (grow_size == 0) {
+        spin_unlock_irqrestore(&heap_lock, flags);
+        uart_print("[HEAP] OOM: kmalloc failed (no phys pages).\n");
+        return NULL;
+    }
+
+    // Create a new free block in the grown region.
+    heap_header_t* new_block = (heap_header_t*)heap_end;
+    new_block->magic = HEAP_MAGIC;
+    new_block->size = grow_size - sizeof(heap_header_t);
+    new_block->is_free = 1;
+    new_block->next = NULL;
+    new_block->prev = tail;
+    if (tail) tail->next = new_block;
+    tail = new_block;
+    heap_end += grow_size;
+
+    // Coalesce with previous block if it's free and adjacent.
+    if (new_block->prev && new_block->prev->is_free) {
+        heap_header_t* prev = new_block->prev;
+        heap_header_t* expected = (heap_header_t*)((uint8_t*)prev + sizeof(heap_header_t) + prev->size);
+        if (expected == new_block) {
+            prev->size += sizeof(heap_header_t) + new_block->size;
+            prev->next = new_block->next;
+            if (new_block->next) {
+                new_block->next->prev = prev;
+            } else {
+                tail = prev;
+            }
+        }
+    }
+
+    // Retry allocation from the start (the new block should satisfy it).
+    current = head;
+    while (current) {
+        if (current->magic != HEAP_MAGIC) break;
+        if (current->is_free && current->size >= aligned_size) {
+            if (current->size > aligned_size + sizeof(heap_header_t) + 16) {
+                heap_header_t* split = (heap_header_t*)((uint8_t*)current + sizeof(heap_header_t) + aligned_size);
+                split->magic = HEAP_MAGIC;
+                split->size = current->size - aligned_size - sizeof(heap_header_t);
+                split->is_free = 1;
+                split->next = current->next;
+                split->prev = current;
+                if (current->next) current->next->prev = split;
+                current->next = split;
+                current->size = aligned_size;
+                if (current == tail) tail = split;
+            }
+            current->is_free = 0;
+            void* ret = (void*)((uint8_t*)current + sizeof(heap_header_t));
+            spin_unlock_irqrestore(&heap_lock, flags);
+            return ret;
+        }
+        current = current->next;
+    }
+
     spin_unlock_irqrestore(&heap_lock, flags);
-    uart_print("[HEAP] OOM: kmalloc failed.\n");
+    uart_print("[HEAP] OOM: kmalloc failed after grow.\n");
     return NULL;
 }
 
index b103620e5f3308225ed7920b388e16498a785442..911ceaee4ea12ad109ecad5ce3f95a0dc347fe32 100644 (file)
@@ -1,5 +1,6 @@
 #include "slab.h"
 #include "pmm.h"
+#include "heap.h"
 #include "hal/mm.h"
 #include "uart_console.h"
 
@@ -24,19 +25,12 @@ void slab_cache_init(slab_cache_t* cache, const char* name, uint32_t obj_size) {
 }
 
 static int slab_grow(slab_cache_t* cache) {
-    void* page = pmm_alloc_page();
-    if (!page) return -1;
-
-    uint8_t* base = (uint8_t*)(uintptr_t)page;
-
-    /* In higher-half kernel the physical page needs to be accessible.
-     * For simplicity we assume the kernel heap region or identity-mapped
-     * low memory is used. We map via the kernel virtual address. */
-    /* TODO: For pages above 4MB, a proper kernel mapping is needed.
-     * For now, slab pages come from pmm_alloc_page which returns
-     * physical addresses. We need to convert to virtual. */
-
-    uint8_t* vbase = (uint8_t*)hal_mm_phys_to_virt((uintptr_t)base);
+    /* Allocate from the kernel heap instead of raw pmm_alloc_page.
+     * The heap is already VMM-mapped at valid kernel VAs, so we avoid
+     * the hal_mm_phys_to_virt bug where phys addresses above 16MB
+     * translate to VAs that collide with the heap range (0xD0000000+). */
+    uint8_t* vbase = (uint8_t*)kmalloc(PAGE_SIZE);
+    if (!vbase) return -1;
 
     for (uint32_t i = 0; i < cache->objs_per_slab; i++) {
         struct slab_free_node* node = (struct slab_free_node*)(vbase + i * cache->obj_size);