fix: fork FD race condition + orphaned zombie memory leak

Bug 1 — Fork FD race (HIGH severity):
  process_fork_create() enqueued the child to the runqueue under
  sched_lock, but syscall_fork_impl() copied file descriptors AFTER
  the function returned — with sched_lock released. On SMP, the child
  could be scheduled on another CPU and reach userspace before FDs
  were populated, seeing NULL file descriptors.

  Fix: move FD copying (with refcount bumps) into process_fork_create()
  itself, under sched_lock, before the child is enqueued. Added proper
  rollback of refcount bumps if kstack_alloc fails.

Bug 2 — Orphaned zombie leak (MEDIUM severity):
  When a process exited, its children were not reparented to PID 1
  (init). Zombie children of exited parents could never be reaped via
  waitpid, leaking process structs and kernel stacks forever.

  Fix: in process_exit_notify(), iterate the process list and reparent
  all children to PID 1. If any reparented child is already a zombie
  and init is blocked in waitpid(-1), wake init immediately.

Also verified (no bugs found):
- EOI handling correct (sent before handlers, spurious skips EOI)
- Lock ordering safe (all locks use irqsave, no cross-CPU ABBA)
- Heap has double-free and corruption detection
- User stack has guard pages

83/83 smoke tests pass, cppcheck clean.

diff --git a/src/kernel/scheduler.c b/src/kernel/scheduler.c
index 8afd25b5a61f4fc133af1ff064f15b296ac06f82..25a82cf4ed372a80d469fbbde30dbe6db91c6659 100644 (file)
--- a/src/kernel/scheduler.c
+++ b/src/kernel/scheduler.c
@@ -506,6 +506,31 @@ void process_exit_notify(int status) {
     current_process->state = PROCESS_ZOMBIE;
     alarm_queue_remove(current_process);
 
+    /* Reparent children to PID 1 (init) so orphaned zombies can be reaped.
+     * If init is waiting on pid==-1, wake it to collect newly-adopted zombies. */
+    {
+        struct process* init_proc = process_find_locked(1);
+        struct process* it = ready_queue_head;
+        if (it && init_proc) {
+            const struct process* const start = it;
+            do {
+                if (it->parent_pid == current_process->pid && it != current_process) {
+                    it->parent_pid = 1;
+                    /* If the child is already a zombie and init is waiting, wake init */
+                    if (it->state == PROCESS_ZOMBIE &&
+                        init_proc->state == PROCESS_BLOCKED && init_proc->waiting &&
+                        init_proc->wait_pid == -1) {
+                        init_proc->wait_result_pid = (int)it->pid;
+                        init_proc->wait_result_status = it->exit_status;
+                        init_proc->state = PROCESS_READY;
+                        rq_enqueue(pcpu_rq[init_proc->cpu_id].active, init_proc);
+                    }
+                }
+                it = it->next;
+            } while (it && it != start);
+        }
+    }
+
     if (current_process->pid != 0) {
         struct process* parent = process_find_locked(current_process->parent_pid);
         if (parent && parent->state == PROCESS_BLOCKED && parent->waiting) {
@@ -588,8 +613,23 @@ struct process* process_fork_create(uintptr_t child_as, const void* child_regs)
         arch_fpu_init_state(proc->fpu_state);
     }
 
-    for (int i = 0; i < PROCESS_MAX_FILES; i++) {
-        proc->files[i] = NULL;
+    /* Copy parent's file descriptors under sched_lock so the child
+     * cannot be scheduled before its FD table is fully populated.
+     * Refcounts are bumped atomically for each shared struct file. */
+    if (current_process) {
+        for (int i = 0; i < PROCESS_MAX_FILES; i++) {
+            struct file* f = current_process->files[i];
+            if (f) {
+                __sync_fetch_and_add(&f->refcount, 1);
+                proc->files[i] = f;
+                proc->fd_flags[i] = current_process->fd_flags[i];
+            } else {
+                proc->files[i] = NULL;
+            }
+        }
+    } else {
+        for (int i = 0; i < PROCESS_MAX_FILES; i++)
+            proc->files[i] = NULL;
     }
     for (int i = 0; i < PROCESS_MAX_MMAPS; i++) {
         proc->mmaps[i] = current_process ? current_process->mmaps[i]
@@ -598,6 +638,13 @@ struct process* process_fork_create(uintptr_t child_as, const void* child_regs)
 
     void* stack = kstack_alloc();
     if (!stack) {
+        /* Undo FD refcount bumps on failure */
+        if (current_process) {
+            for (int i = 0; i < PROCESS_MAX_FILES; i++) {
+                if (proc->files[i])
+                    __sync_sub_and_fetch(&proc->files[i]->refcount, 1);
+            }
+        }
         kfree(proc);
         spin_unlock_irqrestore(&sched_lock, flags);
         return NULL;

diff --git a/src/kernel/syscall.c b/src/kernel/syscall.c
index eed7a29456f27950a96a64066ff7fc6fbdef944e..0dbbae172ed8bce13cf81d73aedff2a905a61035 100644 (file)
--- a/src/kernel/syscall.c
+++ b/src/kernel/syscall.c
@@ -911,13 +911,8 @@ static int syscall_fork_impl(struct registers* regs) {
     child->heap_start = current_process->heap_start;
     child->heap_break = current_process->heap_break;
 
-    for (int fd = 0; fd < PROCESS_MAX_FILES; fd++) {
-        struct file* f = current_process->files[fd];
-        if (!f) continue;
-        __sync_fetch_and_add(&f->refcount, 1);
-        child->files[fd] = f;
-        child->fd_flags[fd] = current_process->fd_flags[fd];
-    }
+    /* FDs are already copied inside process_fork_create under sched_lock
+     * to prevent race where child runs before FDs are set up. */
 
     return (int)child->pid;
 }