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 9e709f6130f888cb8c68a2fae635ab89eac391d5..aab9f1436e7f460bf88fb67d3a8413e68a545fe8 100644 (file)
--- a/src/arch/x86/cpuid.c
+++ b/src/arch/x86/cpuid.c
@@ -12,6 +12,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) {
@@ -31,10 +32,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;
@@ -136,7 +137,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 b3b0020286e83d2d40cb087b3ae333050b1ad522..7fcf2d541f049cf3f22d9fccc81ad9db182cfda1 100644 (file)
--- a/src/arch/x86/sysenter.S
+++ b/src/arch/x86/sysenter.S
@@ -60,6 +60,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 1e2422e60efd1a5d589cbb7eb550150c2150c6e9..ff2fb42898b58ea392704dd14b8445c78670d328 100644 (file)
--- a/user/ulibc/src/stdio.c
+++ b/user/ulibc/src/stdio.c
@@ -29,12 +29,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 361d4495e077d1f06b353b4fe4e0998cd6e1b2a8..2234dff98713e931a8cff23605cfeff9f9d99208 100644 (file)
--- a/user/ulibc/src/stdlib.c
+++ b/user/ulibc/src/stdlib.c
@@ -34,7 +34,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 15fe07dd0912a3973fb6ad4885e775e5abede374..077c621bf86534d2dfb7c324d76125f1b4a0dad2 100644 (file)
--- a/user/ulibc/src/unistd.c
+++ b/user/ulibc/src/unistd.c
@@ -97,5 +97,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");
 }