fix: audit and correct bugs in Fases 1-4

Fase 1 (CPUID):
- cpuid.c: Replace strict-aliasing-violating uint32_t* casts for
  vendor string with memcpy() to avoid UB
- cpuid.c: Increase itoa tmp buffer from 4 to 12 bytes to prevent
  potential overflow with larger APIC IDs

Fase 2 (Spinlock): No bugs found — TTAS + cpu_relax + barriers OK

Fase 3 (SYSENTER):
- sysenter.S: Add 'cld' at entry to clear direction flag. Userspace
  could leave DF=1 and SYSENTER doesn't reset EFLAGS, which would
  corrupt any kernel string/memory operations using rep movsb/stosb

Fase 4 (ulibc):
- stdio.c: Fix PUTC macro underflow — use 'pos+1 < size' instead
  of 'pos < size-1' which underflows to SIZE_MAX when size==0
- stdio.c: Fix vsnprintf(buf, 0, fmt) — was counting raw format
  chars instead of returning 0; now returns 0 immediately
- stdlib.c: Use uintptr_t instead of unsigned int for brk() pointer
  comparison to be correct on all architectures
- unistd.c: Replace 'hlt' with 'nop' in _exit() fallback loop —
  hlt is privileged and causes #GP in ring 3

Passes: make, ulibc build, cppcheck, QEMU smoke test.

diff --git a/src/arch/x86/cpuid.c b/src/arch/x86/cpuid.c
index fa192260253118f9d5bab00862c6112b061fe784..1f590bd7619b6b9f718497a3cf54f97955d34dfc 100644 (file)
--- a/src/arch/x86/cpuid.c
+++ b/src/arch/x86/cpuid.c
@@ -3,6 +3,7 @@
 #include "utils.h"
 
 #include <stddef.h>
+#include <stdint.h>
 
 static inline void cpuid(uint32_t leaf, uint32_t* eax, uint32_t* ebx,
                           uint32_t* ecx, uint32_t* edx) {
@@ -22,10 +23,10 @@ void x86_cpuid_detect(struct x86_cpu_features* out) {
     cpuid(0, &eax, &ebx, &ecx, &edx);
     out->max_leaf = eax;
 
-    /* Vendor: EBX-EDX-ECX */
-    *(uint32_t*)&out->vendor[0] = ebx;
-    *(uint32_t*)&out->vendor[4] = edx;
-    *(uint32_t*)&out->vendor[8] = ecx;
+    /* Vendor: EBX-EDX-ECX (use memcpy to avoid strict-aliasing UB) */
+    memcpy(&out->vendor[0], &ebx, 4);
+    memcpy(&out->vendor[4], &edx, 4);
+    memcpy(&out->vendor[8], &ecx, 4);
     out->vendor[12] = '\0';
 
     if (out->max_leaf < 1) return;
@@ -127,7 +128,7 @@ void x86_cpuid_print(const struct x86_cpu_features* f) {
     uart_print("\n");
 
     uart_print("[CPUID] APIC ID: ");
-    char tmp[4];
+    char tmp[12];
     itoa(f->initial_apic_id, tmp, 10);
     uart_print(tmp);
     uart_print(", Logical CPUs: ");

diff --git a/src/arch/x86/sysenter.S b/src/arch/x86/sysenter.S
index c0f186e99cc1a5816a2f54794504978cd40ff791..8a4a704fbf93557eb1fb29ee8e105f6d5b3b7ac0 100644 (file)
--- a/src/arch/x86/sysenter.S
+++ b/src/arch/x86/sysenter.S
@@ -51,6 +51,9 @@ sysenter_entry:
      *   User stack: [return_eip, saved_edx(arg3), saved_ecx(arg2)]
      */
 
+    /* Ensure direction flag is clear — userspace may have set DF=1 */
+    cld
+
     /* Build iret-style frame: ss, useresp, eflags, cs, eip */
     push $0x23          /* ss  = user data segment (GDT entry 4 | RPL 3) */
     push %ecx           /* useresp = user ESP */

diff --git a/user/ulibc/src/stdio.c b/user/ulibc/src/stdio.c
index 87089a160c83ba0b9692478f27d7cefa02d97e9c..a01bc017a608a43e80ad5e6a29463b68c97cc936 100644 (file)
--- a/user/ulibc/src/stdio.c
+++ b/user/ulibc/src/stdio.c
@@ -20,12 +20,11 @@ int puts(const char* s) {
 int vsnprintf(char* buf, size_t size, const char* fmt, va_list ap) {
     size_t pos = 0;
 
-#define PUTC(c) do { if (pos < size - 1) buf[pos] = (c); pos++; } while(0)
+#define PUTC(c) do { if (pos + 1 < size) buf[pos] = (c); pos++; } while(0)
 
     if (size == 0) {
-        /* Just count characters */
-        while (*fmt) { pos++; fmt++; }
-        return (int)pos;
+        /* Cannot write anything; return 0 */
+        return 0;
     }
 
     while (*fmt) {

diff --git a/user/ulibc/src/stdlib.c b/user/ulibc/src/stdlib.c
index 7afd02dfea6ce1239c227348c752125c2a294aa4..072ff02f0f033d92e8ac3c05b17d508eac74c8db 100644 (file)
--- a/user/ulibc/src/stdlib.c
+++ b/user/ulibc/src/stdlib.c
@@ -25,7 +25,7 @@ void* malloc(size_t size) {
     void* new_end = (void*)((char*)heap_end + size);
     void* result = brk(new_end);
 
-    if ((unsigned int)result < (unsigned int)new_end) {
+    if ((uintptr_t)result < (uintptr_t)new_end) {
         return (void*)0;  /* OOM */
     }
 

diff --git a/user/ulibc/src/unistd.c b/user/ulibc/src/unistd.c
index 5114a7574ac45b7ef90f21d8ffebf9ab087335d4..3036f5c15265a52250929b667fb9d79628eb3288 100644 (file)
--- a/user/ulibc/src/unistd.c
+++ b/user/ulibc/src/unistd.c
@@ -88,5 +88,7 @@ void* brk(void* addr) {
 
 void _exit(int status) {
     _syscall1(SYS_EXIT, status);
-    for (;;) __asm__ volatile("hlt");
+    /* If exit syscall somehow returns, loop forever.
+     * Cannot use hlt — it's privileged and causes #GP in ring 3. */
+    for (;;) __asm__ volatile("nop");
 }