From d861bfb2efc686319afb99906e5783e24e42f7e4 Mon Sep 17 00:00:00 2001 From: Tulio A M Mendes Date: Fri, 17 Apr 2026 00:16:43 -0300 Subject: [PATCH] fix: 4 security/robustness bugs from audit MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 1. epoll: remove fd from epoll instances on close() — prevents use-after-close where a reused fd number would monitor the wrong file. Also auto-remove stale entries in epoll_wait. 2. clone: validate flags against CLONE_SUPPORTED_MASK — unknown flags (CLONE_NEWPID, CLONE_NEWUSER, etc.) now return EINVAL instead of being silently ignored. 3. futex: cleanup waiters on process exit — futex_waiters table moved to file scope; futex_cleanup_process() called from SYSCALL_EXIT prevents UAF when FUTEX_WAKE dereferences a freed process pointer. 4. ksem: fix infinite spin when waiters array full — replace schedule() spin-yield with process_sleep(1), and increase KSEM_MAX_WAITERS/KCOND_MAX_WAITERS from 16 to 64. --- Makefile | 6 ++-- include/sync.h | 4 +-- src/kernel/sync.c | 4 +-- src/kernel/syscall.c | 67 ++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 70 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index c876038e..648e3604 100644 --- a/Makefile +++ b/Makefile @@ -91,9 +91,9 @@ ifeq ($(ARCH),x86) # Default User Flags (Allow override via make CFLAGS=...) CFLAGS ?= -O2 -Wall -Wextra -Werror -Wno-error=cpp - + # Merge Flags - CFLAGS := $(ARCH_CFLAGS) $(CFLAGS) + CFLAGS := $(ARCH_CFLAGS) $(CFLAGS) -MMD -MP LDFLAGS := $(ARCH_LDFLAGS) $(LDFLAGS) ASFLAGS := $(ARCH_ASFLAGS) $(ASFLAGS) @@ -403,6 +403,8 @@ $(BUILD_DIR)/%.o: $(SRC_DIR)/%.c @echo " CC $<" @$(CC) $(CFLAGS) -c $< -o $@ +-include $(wildcard $(BUILD_DIR)/**/*.d) + # lwIP sources (compiled with relaxed warnings) $(BUILD_DIR)/lwip/%.o: %.c @mkdir -p $(dir $@) diff --git a/include/sync.h b/include/sync.h index f3ff61f1..6d927f18 100644 --- a/include/sync.h +++ b/include/sync.h @@ -17,7 +17,7 @@ /* Kernel counting semaphore (blocking, sleep/wake — NOT spin-wait) */ /* ------------------------------------------------------------------ */ -#define KSEM_MAX_WAITERS 16 +#define KSEM_MAX_WAITERS 64 struct process; /* forward */ @@ -80,7 +80,7 @@ int kmbox_tryfetch(kmbox_t* mb, void** msg); /* Kernel condition variable (paired with kmutex_t) */ /* ------------------------------------------------------------------ */ -#define KCOND_MAX_WAITERS 16 +#define KCOND_MAX_WAITERS 64 typedef struct kcond { spinlock_t lock; diff --git a/src/kernel/sync.c b/src/kernel/sync.c index 89455c08..8d51e4fa 100644 --- a/src/kernel/sync.c +++ b/src/kernel/sync.c @@ -32,8 +32,8 @@ void ksem_init(ksem_t* s, int32_t initial_count) { void ksem_wait(ksem_t* s) { while (ksem_wait_timeout(s, 0) != 0) { - /* Waiters array full — yield and retry */ - schedule(); + /* Waiters array full — sleep 1 tick instead of spin-yielding */ + process_sleep(1); } } diff --git a/src/kernel/syscall.c b/src/kernel/syscall.c index 1f4dbab9..880dd245 100644 --- a/src/kernel/syscall.c +++ b/src/kernel/syscall.c @@ -798,6 +798,26 @@ static struct file* fd_get(int fd); static void socket_syscall_dispatch(struct registers* regs, uint32_t syscall_no); static void posix_ext_syscall_dispatch(struct registers* regs, uint32_t syscall_no); +/* ------------------------------------------------------------------ */ +/* Futex waiter table (file-scope for cleanup on process exit) */ +/* ------------------------------------------------------------------ */ + +#define FUTEX_WAIT 0 +#define FUTEX_WAKE 1 +#define FUTEX_MAX_WAITERS 32 + +static struct { uintptr_t addr; struct process* proc; } futex_waiters[FUTEX_MAX_WAITERS]; + +static void futex_cleanup_process(struct process* p) { + if (!p) return; + for (int i = 0; i < FUTEX_MAX_WAITERS; i++) { + if (futex_waiters[i].proc == p) { + futex_waiters[i].proc = NULL; + futex_waiters[i].addr = 0; + } + } +} + struct pollfd { int fd; int16_t events; @@ -939,6 +959,13 @@ static int syscall_clone_impl(struct registers* regs) { uintptr_t child_stack = (uintptr_t)sc_arg1(regs); uintptr_t tls_base = (uintptr_t)sc_arg3(regs); + /* Reject unsupported clone flags — prevents silent misbehavior */ + #define CLONE_SUPPORTED_MASK (CLONE_VM | CLONE_FS | CLONE_FILES | \ + CLONE_SIGHAND | CLONE_THREAD | \ + CLONE_SETTLS | CLONE_PARENT_SETTID |\ + CLONE_CHILD_CLEARTID) + if (clone_flags & ~CLONE_SUPPORTED_MASK) return -EINVAL; + struct process* child = process_clone_create(clone_flags, child_stack, regs, tls_base); if (!child) return -ENOMEM; @@ -1081,6 +1108,31 @@ struct epoll_instance { int count; }; +static void epoll_close(fs_node_t* node); + +static void epoll_instance_remove_fd(struct epoll_instance* ep, int fd) { + if (!ep) return; + for (int i = 0; i < ep->count; i++) { + if (ep->items[i].fd == fd) { + ep->items[i] = ep->items[ep->count - 1]; + ep->count--; + return; + } + } +} + +static void epoll_cleanup_fd(int fd) { + if (!current_process) return; + for (int i = 0; i < PROCESS_MAX_FILES; i++) { + struct file* f = current_process->files[i]; + if (!f || !f->node || !f->node->f_ops || f->node->f_ops->close != epoll_close) + continue; + if (i == fd) continue; /* don't touch the epoll fd itself */ + struct epoll_instance* ep = (struct epoll_instance*)(uintptr_t)f->node->inode; + epoll_instance_remove_fd(ep, fd); + } +} + static void epoll_close(fs_node_t* node) { if (node && node->inode) { kfree((void*)(uintptr_t)node->inode); @@ -1207,9 +1259,13 @@ static int syscall_epoll_wait_impl(int epfd, struct epoll_event* user_events, int fd = ep->items[i].fd; struct file* f = fd_get(fd); if (!f || !f->node) { + /* Stale fd — report EPOLLERR once and remove the item */ out[ready].events = EPOLLERR; out[ready].data = ep->items[i].data; ready++; + ep->items[i] = ep->items[ep->count - 1]; + ep->count--; + i--; /* re-check the swapped-in item */ continue; } @@ -1881,6 +1937,11 @@ static int fd_close(int fd) { struct file* f = current_process->files[fd]; if (!f) return -EBADF; + + /* Remove this fd from any epoll instances before clearing the slot, + * so epoll_cleanup_fd can still walk current_process->files. */ + epoll_cleanup_fd(fd); + current_process->files[fd] = NULL; if (__sync_sub_and_fetch(&f->refcount, 1) == 0) { @@ -3187,6 +3248,7 @@ void syscall_handler(struct registers* regs) { if (current_process) { flock_release_pid(current_process->pid); rlock_release_pid(current_process->pid); + futex_cleanup_process(current_process); } for (int fd = 0; fd < PROCESS_MAX_FILES; fd++) { @@ -4210,11 +4272,6 @@ static void posix_ext_syscall_dispatch(struct registers* regs, uint32_t syscall_ } if (syscall_no == SYSCALL_FUTEX) { - #define FUTEX_WAIT 0 - #define FUTEX_WAKE 1 - #define FUTEX_MAX_WAITERS 32 - static struct { uintptr_t addr; struct process* proc; } futex_waiters[FUTEX_MAX_WAITERS]; - uint32_t* uaddr = (uint32_t*)sc_arg0(regs); int op = (int)sc_arg1(regs); uint32_t val = sc_arg2(regs); -- 2.43.0