From: Tulio A M Mendes <tadryanom@hotmail.com>
Date: Mon, 16 Feb 2026 22:32:09 +0000 (-0300)
Subject: fix: fork FD race condition + orphaned zombie memory leak
X-Git-Url: https://projects.tadryanom.me/docs/static/gitweb.css?a=commitdiff_plain;h=69b12c7adcb163b911bd7e70f7efe2a1761c128b;p=AdrOS.git

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 1ba3d79..b5a8362 100644
--- a/src/kernel/scheduler.c
+++ b/src/kernel/scheduler.c
@@ -497,6 +497,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) {
@@ -579,8 +604,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]
@@ -589,6 +629,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 5bded8d..ab4b869 100644
--- a/src/kernel/syscall.c
+++ b/src/kernel/syscall.c
@@ -902,13 +902,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;
 }