diff --git a/src/sys/syscall.c b/src/sys/syscall.c index 538dbdf..7ad9ad2 100644 --- a/src/sys/syscall.c +++ b/src/sys/syscall.c @@ -325,6 +325,9 @@ static uint64_t gui_cmd_window_create(const syscall_args_t *args) { win->handle_resize = user_window_resize; win->resizable = false; // Default to false, can be enabled via syscall + // Store owner PID to allow safe detachment during window destruction. + // This prevents Use-After-Free when a process continues drawing after its window is closed. + win->owner_pid = proc->pid; proc->ui_window = win; wm_add_window(win); wm_mark_dirty(0, 0, get_screen_width(), 30); @@ -336,20 +339,18 @@ static uint64_t gui_cmd_draw_rect(const syscall_args_t *args) { Window *win = (Window *)args->arg2; uint64_t *u_params = (uint64_t *)args->arg3; uint32_t color = (uint32_t)args->arg4; - if (win && u_params) { + process_t *proc = process_get_current(); + + if (win && u_params && proc && proc->ui_window == win) { uint64_t params[4]; for (int i = 0; i < 4; i++) params[i] = u_params[i]; extern void draw_rect(int x, int y, int w, int h, uint32_t color); extern void graphics_set_render_target(uint32_t *buffer, int w, int h); - uint64_t rflags; - bool use_wm_lock = (win->pixels == NULL); - if (use_wm_lock) rflags = wm_lock_acquire(); - else rflags = spinlock_acquire_irqsave(&win->lock); + uint64_t rflags = spinlock_acquire_irqsave(&win->lock); if (win->pixels) { - // Strict user-to-window relative clamping int rx = (int)params[0]; int ry = (int)params[1]; int rw = (int)params[2]; int rh = (int)params[3]; if (rx < 0) { rw += rx; rx = 0; } @@ -363,11 +364,12 @@ static uint64_t gui_cmd_draw_rect(const syscall_args_t *args) { graphics_set_render_target(NULL, 0, 0); } } else { - draw_rect(win->x + params[0], win->y + params[1], params[2], params[3], color); + uint64_t wflags = wm_lock_acquire(); + draw_rect(win->x + (int)params[0], win->y + (int)params[1], (int)params[2], (int)params[3], color); + wm_lock_release(wflags); } - if (use_wm_lock) wm_lock_release(rflags); - else spinlock_release_irqrestore(&win->lock, rflags); + spinlock_release_irqrestore(&win->lock, rflags); } return 0; } @@ -376,17 +378,16 @@ static uint64_t gui_cmd_draw_rounded_rect_filled(const syscall_args_t *args) { Window *win = (Window *)args->arg2; uint64_t *u_params = (uint64_t *)args->arg3; uint32_t color = (uint32_t)args->arg4; - if (win && u_params) { + process_t *proc = process_get_current(); + + if (win && u_params && proc && proc->ui_window == win) { uint64_t params[5]; for (int i = 0; i < 5; i++) params[i] = u_params[i]; extern void draw_rounded_rect_filled(int x, int y, int w, int h, int radius, uint32_t color); extern void graphics_set_render_target(uint32_t *buffer, int w, int h); - uint64_t rflags; - bool use_wm_lock = (win->pixels == NULL); - if (use_wm_lock) rflags = wm_lock_acquire(); - else rflags = spinlock_acquire_irqsave(&win->lock); + uint64_t rflags = spinlock_acquire_irqsave(&win->lock); if (win->pixels) { int rx = (int)params[0]; int ry = (int)params[1]; @@ -404,8 +405,7 @@ static uint64_t gui_cmd_draw_rounded_rect_filled(const syscall_args_t *args) { } } - if (use_wm_lock) wm_lock_release(rflags); - else spinlock_release_irqrestore(&win->lock, rflags); + spinlock_release_irqrestore(&win->lock, rflags); } return 0; } @@ -417,7 +417,9 @@ static uint64_t gui_cmd_draw_string(const syscall_args_t *args) { int uy = coords >> 32; const char *user_str = (const char *)args->arg4; uint32_t color = (uint32_t)args->arg5; - if (win && user_str) { + process_t *proc = process_get_current(); + + if (win && user_str && proc && proc->ui_window == win) { extern void draw_string(int x, int y, const char *str, uint32_t color); extern void graphics_set_render_target(uint32_t *buffer, int w, int h); @@ -430,10 +432,7 @@ static uint64_t gui_cmd_draw_string(const syscall_args_t *args) { } kernel_str[i] = 0; - uint64_t rflags; - bool use_wm_lock = (win->pixels == NULL); - if (use_wm_lock) rflags = wm_lock_acquire(); - else rflags = spinlock_acquire_irqsave(&win->lock); + uint64_t rflags = spinlock_acquire_irqsave(&win->lock); ttf_font_t *font = win->font ? (ttf_font_t*)win->font : graphics_get_current_ttf(); @@ -463,6 +462,7 @@ static uint64_t gui_cmd_draw_string(const syscall_args_t *args) { graphics_set_render_target(NULL, 0, 0); } } else { + uint64_t wflags = wm_lock_acquire(); if (font) { int baseline = win->y + uy + font_manager_get_font_ascent_scaled(font, font->pixel_height) - 2; int cur_x = win->x + ux; @@ -483,10 +483,10 @@ static uint64_t gui_cmd_draw_string(const syscall_args_t *args) { } else { draw_string(win->x + ux, win->y + uy, kernel_str, color); } + wm_lock_release(wflags); } - if (use_wm_lock) wm_lock_release(rflags); - else spinlock_release_irqrestore(&win->lock, rflags); + spinlock_release_irqrestore(&win->lock, rflags); } return 0; } @@ -714,14 +714,13 @@ static uint64_t gui_cmd_draw_image(const syscall_args_t *args) { Window *win = (Window *)args->arg2; uint64_t *u_params = (uint64_t *)args->arg3; uint32_t *image_data = (uint32_t *)args->arg4; - if (win && u_params && image_data) { + process_t *proc = process_get_current(); + + if (win && u_params && image_data && proc && proc->ui_window == win) { uint64_t params[4]; for (int i = 0; i < 4; i++) params[i] = u_params[i]; - uint64_t rflags; - bool use_wm_lock = (win->pixels == NULL); - if (use_wm_lock) rflags = wm_lock_acquire(); - else rflags = spinlock_acquire_irqsave(&win->lock); + uint64_t rflags = spinlock_acquire_irqsave(&win->lock); if (win->pixels) { int rx = (int)params[0]; int ry = (int)params[1]; @@ -755,17 +754,17 @@ static uint64_t gui_cmd_draw_image(const syscall_args_t *args) { } } - if (use_wm_lock) wm_lock_release(rflags); - else spinlock_release_irqrestore(&win->lock, rflags); + spinlock_release_irqrestore(&win->lock, rflags); } return 0; } static uint64_t gui_cmd_mark_dirty(const syscall_args_t *args) { - uint64_t rflags = wm_lock_acquire(); Window *win = (Window *)args->arg2; uint64_t *u_params = (uint64_t *)args->arg3; - if (win && u_params) { + process_t *proc = process_get_current(); + + if (win && u_params && proc && proc->ui_window == win) { uint64_t params[4]; for (int i = 0; i < 4; i++) params[i] = u_params[i]; @@ -776,9 +775,11 @@ static uint64_t gui_cmd_mark_dirty(const syscall_args_t *args) { mem_memcpy(win->comp_pixels, win->pixels, (size_t)win->w * (win->h - 20) * 4); spinlock_release_irqrestore(&win->lock, win_rflags); } + + uint64_t rflags = wm_lock_acquire(); wm_mark_dirty(win->x + (int)params[0], win->y + (int)params[1], (int)params[2], (int)params[3]); + wm_lock_release(rflags); } - wm_lock_release(rflags); return 0; } @@ -862,11 +863,15 @@ static uint64_t gui_cmd_get_font_height_scaled(const syscall_args_t *args) { static uint64_t gui_cmd_window_set_resizable(const syscall_args_t *args) { Window *win = (Window *)args->arg2; - if (win) { + process_t *proc = process_get_current(); + if (win && proc && proc->ui_window == win) { + uint64_t flags = spinlock_acquire_irqsave(&win->lock); + win->resizable = (args->arg3 != 0); + spinlock_release_irqrestore(&win->lock, flags); + extern void serial_write(const char *str); serial_write("[WM] Resizable: "); serial_write(args->arg3 ? "true\n" : "false\n"); - win->resizable = (args->arg3 != 0); } return 0; } @@ -874,7 +879,9 @@ static uint64_t gui_cmd_window_set_resizable(const syscall_args_t *args) { static uint64_t gui_cmd_window_set_title(const syscall_args_t *args) { Window *win = (Window *)args->arg2; const char *user_title = (const char *)args->arg3; - if (win && user_title) { + process_t *proc = process_get_current(); + + if (win && user_title && proc && proc->ui_window == win) { int title_len = 0; while (user_title[title_len] && title_len < 255) title_len++; @@ -885,10 +892,15 @@ static uint64_t gui_cmd_window_set_title(const syscall_args_t *args) { } kernel_title[title_len] = '\0'; - if (win->title && win->title != (char*)"Unknown") { - kfree(win->title); - } + uint64_t flags = spinlock_acquire_irqsave(&win->lock); + char *old_title = win->title; win->title = kernel_title; + spinlock_release_irqrestore(&win->lock, flags); + + if (old_title && old_title != (char*)"Unknown") { + kfree(old_title); + } + wm_mark_dirty(win->x, win->y - 20, win->w, 20); // Mark title bar dirty wm_refresh(); } diff --git a/src/wm/font_manager.c b/src/wm/font_manager.c index c873c65..2072acc 100644 --- a/src/wm/font_manager.c +++ b/src/wm/font_manager.c @@ -3,6 +3,7 @@ #include "font_manager.h" #include "stb_truetype.h" #include "fat32.h" +#include "spinlock.h" #include // Simple math implementations for stb_truetype @@ -78,6 +79,9 @@ typedef struct { unsigned char *bitmap; } font_cache_entry_t; static font_cache_entry_t font_cache[FONT_CACHE_SIZE] = {0}; +// Global lock for the glyph cache. Prevents multi-core race conditions where +// bitmaps were being freed while in use by other cores. +static spinlock_t font_cache_lock = SPINLOCK_INIT; bool font_manager_init(void) { return true; @@ -202,6 +206,8 @@ void font_manager_render_char_scaled(ttf_font_t *font, int x, int y, uint32_t co if (!font) return; uint32_t hash = (codepoint * 31 + (uint32_t)scale * 73) % FONT_CACHE_SIZE; + + uint64_t fflags = spinlock_acquire_irqsave(&font_cache_lock); font_cache_entry_t *entry = &font_cache[hash]; unsigned char *bitmap = NULL; @@ -219,16 +225,27 @@ void font_manager_render_char_scaled(ttf_font_t *font, int x, int y, uint32_t co real_scale = stbtt_ScaleForPixelHeight(info, scale); } + // Drop lock during slow decompression to avoid contention + spinlock_release_irqrestore(&font_cache_lock, fflags); bitmap = stbtt_GetCodepointBitmap(info, 0, real_scale, codepoint, &w, &h, &xoff, &yoff); + fflags = spinlock_acquire_irqsave(&font_cache_lock); - if (entry->bitmap) stbtt_FreeBitmap(entry->bitmap, NULL); - entry->codepoint = codepoint; - entry->scale = scale; - entry->font = font; - entry->w = w; entry->h = h; entry->xoff = xoff; entry->yoff = yoff; - entry->bitmap = bitmap; + // Check if someone else filled it while we were away + if (entry->bitmap && entry->codepoint == codepoint && entry->scale == scale && entry->font == font) { + if (bitmap) stbtt_FreeBitmap(bitmap, NULL); + bitmap = entry->bitmap; + w = entry->w; h = entry->h; xoff = entry->xoff; yoff = entry->yoff; + } else { + if (entry->bitmap) stbtt_FreeBitmap(entry->bitmap, NULL); + entry->codepoint = codepoint; + entry->scale = scale; + entry->font = font; + entry->w = w; entry->h = h; entry->xoff = xoff; entry->yoff = yoff; + entry->bitmap = bitmap; + } } - + + // Hold lock while using the bitmap to prevent eviction if (bitmap) { for (int row = 0; row < h; row++) { for (int col = 0; col < w; col++) { @@ -242,6 +259,7 @@ void font_manager_render_char_scaled(ttf_font_t *font, int x, int y, uint32_t co } } } + spinlock_release_irqrestore(&font_cache_lock, fflags); } void font_manager_render_char_sloped(ttf_font_t *font, int x, int y, uint32_t codepoint, uint32_t color, float scale, float slope, void (*put_pixel_fn)(int, int, uint32_t)) { @@ -249,6 +267,7 @@ void font_manager_render_char_sloped(ttf_font_t *font, int x, int y, uint32_t co if (!font) return; uint32_t hash = (codepoint * 31 + (uint32_t)scale * 73) % FONT_CACHE_SIZE; + uint64_t fflags = spinlock_acquire_irqsave(&font_cache_lock); font_cache_entry_t *entry = &font_cache[hash]; unsigned char *bitmap = NULL; @@ -266,14 +285,22 @@ void font_manager_render_char_sloped(ttf_font_t *font, int x, int y, uint32_t co real_scale = stbtt_ScaleForPixelHeight(info, scale); } + spinlock_release_irqrestore(&font_cache_lock, fflags); bitmap = stbtt_GetCodepointBitmap(info, 0, real_scale, codepoint, &w, &h, &xoff, &yoff); - - if (entry->bitmap) stbtt_FreeBitmap(entry->bitmap, NULL); - entry->codepoint = codepoint; - entry->scale = scale; - entry->font = font; - entry->w = w; entry->h = h; entry->xoff = xoff; entry->yoff = yoff; - entry->bitmap = bitmap; + fflags = spinlock_acquire_irqsave(&font_cache_lock); + + if (entry->bitmap && entry->codepoint == codepoint && entry->scale == scale && entry->font == font) { + if (bitmap) stbtt_FreeBitmap(bitmap, NULL); + bitmap = entry->bitmap; + w = entry->w; h = entry->h; xoff = entry->xoff; yoff = entry->yoff; + } else { + if (entry->bitmap) stbtt_FreeBitmap(entry->bitmap, NULL); + entry->codepoint = codepoint; + entry->scale = scale; + entry->font = font; + entry->w = w; entry->h = h; entry->xoff = xoff; entry->yoff = yoff; + entry->bitmap = bitmap; + } } if (bitmap) { @@ -291,6 +318,7 @@ void font_manager_render_char_sloped(ttf_font_t *font, int x, int y, uint32_t co } } } + spinlock_release_irqrestore(&font_cache_lock, fflags); } int font_manager_get_string_width(ttf_font_t *font, const char *s) { diff --git a/src/wm/wm.c b/src/wm/wm.c index 9206a40..288692f 100644 --- a/src/wm/wm.c +++ b/src/wm/wm.c @@ -2850,6 +2850,17 @@ Window* wm_find_window_by_title(const char *title) { void wm_remove_window(Window *win) { if (!win) return; + // Safety: Detach from owner first to prevent UAF in GUI syscalls. + // By clearing the owner's pointer here, any concurrent or future syscalls for this + // window handle will fail validation rather than accessing freed memory. + if (win->owner_pid != 0) { + extern process_t* process_get_by_pid(uint32_t pid); + process_t *proc = process_get_by_pid(win->owner_pid); + if (proc && proc->ui_window == (void*)win) { + proc->ui_window = NULL; + } + } + serial_write("WM: Removing window '"); if (win->title) serial_write(win->title); else serial_write("unknown"); diff --git a/src/wm/wm.h b/src/wm/wm.h index 53f367c..6d2027a 100644 --- a/src/wm/wm.h +++ b/src/wm/wm.h @@ -69,6 +69,9 @@ struct Window { void (*handle_close)(Window *win); void (*handle_resize)(Window *win, int w, int h); bool resizable; + // The PID of the process that created/owns this window. + // Used to safely detach the window from the owner's process_t on destruction. + uint32_t owner_pid; }; #define LUMOS_MAX_RESULTS 6