From 7aba5c2105bfc8a10948d15b5b7e3cbbc68d6a5c Mon Sep 17 00:00:00 2001 From: Tulio A M Mendes Date: Tue, 9 Jun 2026 04:12:43 -0300 Subject: [PATCH] scheduler: harden SIGKILL SMP and clone semantics Fix the remote SIGKILL path so it no longer tears down a task on behalf of another CPU before that task reaches a safe exit point. SIGKILL is now queued as a fatal pending signal, forcibly unblocked, blocked/sleeping tasks are woken, and reschedule IPIs are sent so the target exits on its own CPU. Fix CLONE_VM address-space accounting by using the global address-space refcount table to detect the first shared-mm clone and to decide teardown during reap. This closes the leak/counter skew when a non-leader thread creates another CLONE_VM thread. Align clone semantics with the implementation by rejecting unsupported shared-state flags from the syscall interface, while still inheriting cwd, file references, descriptor flags, signal handlers, and signal mask by copy. Update pthread/fulltest users accordingly. Also prevent userspace from changing SIGKILL/SIGSTOP disposition or blocking them, make default SIGKILL delivery fatal in usermode signal dispatch, and extend the kill regression test so the child tries to block SIGKILL before spinning. --- include/process.h | 2 +- src/arch/x86/idt.c | 2 + src/kernel/scheduler.c | 98 ++++++++++++++++------------------- src/kernel/syscall.c | 6 ++- user/cmds/fulltest/fulltest.c | 4 +- user/ulibc/src/pthread.c | 4 +- 6 files changed, 57 insertions(+), 59 deletions(-) diff --git a/include/process.h b/include/process.h index 1b187df2..a56003dc 100644 --- a/include/process.h +++ b/include/process.h @@ -28,7 +28,7 @@ #define CLONE_CHILD_CLEARTID 0x00200000 /* Clear child tid on exit */ /* Convenience: flags for a typical pthread_create */ -#define CLONE_THREAD_FLAGS (CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND | CLONE_THREAD | CLONE_SETTLS) +#define CLONE_THREAD_FLAGS (CLONE_VM | CLONE_THREAD | CLONE_SETTLS) #define PROCESS_FLAG_THREAD 0x01 /* This process is a thread (not group leader) */ diff --git a/src/arch/x86/idt.c b/src/arch/x86/idt.c index 8ba90321..9ffeaa46 100644 --- a/src/arch/x86/idt.c +++ b/src/arch/x86/idt.c @@ -110,6 +110,8 @@ static void deliver_signals_to_usermode(struct registers* regs) { current_process ? (int)current_process->pid : -1, (unsigned)current_process->last_fault_addr, (unsigned)regs->eip); + } + if (sig == 9 || sig == 11) { process_exit_notify(128 + sig); __asm__ volatile("sti"); schedule(); diff --git a/src/kernel/scheduler.c b/src/kernel/scheduler.c index 58648bff..6fbbce18 100644 --- a/src/kernel/scheduler.c +++ b/src/kernel/scheduler.c @@ -23,6 +23,16 @@ static void as_refcount_inc(uintptr_t as) { } } +static uint32_t as_refcount_get(uintptr_t as) { + if (!as) return 0; + for (int i = 0; i < AS_REFCOUNT_MAX; i++) { + if (g_as_refcnt[i].as == as) { + return g_as_refcnt[i].refcnt; + } + } + return 0; +} + static uint32_t as_refcount_dec(uintptr_t as) { if (!as) return 0; for (int i = 0; i < AS_REFCOUNT_MAX; i++) { @@ -400,7 +410,7 @@ static void process_reap_locked(struct process* p) { if (as_refcount_dec(p->addr_space) == 0) { vmm_as_destroy(p->addr_space); } - } else if (p->as_refcount > 0) { + } else if (as_refcount_get(p->addr_space) > 0) { /* Leader with live threads: decrement refcount; last one destroys */ if (as_refcount_dec(p->addr_space) == 0) { vmm_as_destroy(p->addr_space); @@ -449,6 +459,7 @@ int process_kill(uint32_t pid, int sig) { } uintptr_t flags = spin_lock_irqsave(&sched_lock); + uint32_t wake_cpu = (uint32_t)-1; struct process* p = process_find_locked(pid); if (!p || p->pid == 0) { spin_unlock_irqrestore(&sched_lock, flags); @@ -468,39 +479,20 @@ int process_kill(uint32_t pid, int sig) { } } + wake_cpu = p->cpu_id < SCHED_MAX_CPUS ? p->cpu_id : 0; + p->sig_pending_mask |= (1U << (uint32_t)sig); if (sig == SIG_KILL) { - /* Remove from runqueue/sleep queue BEFORE marking ZOMBIE */ - if (p->state == PROCESS_READY) { - rq_remove_if_queued(p); - } + p->sig_blocked_mask &= ~(1U << (uint32_t)sig); + } + if (p->state == PROCESS_BLOCKED || p->state == PROCESS_SLEEPING) { sleep_queue_remove(p); - alarm_queue_remove(p); - process_close_all_files_locked(p); - process_mount_unref_cwd(p); - p->exit_status = 128 + sig; - p->state = PROCESS_ZOMBIE; - - if (p->pid != 0) { - struct process* parent = process_find_locked(p->parent_pid); - if (parent && parent->state == PROCESS_BLOCKED && parent->waiting) { - if (parent->wait_pid == -1 || parent->wait_pid == (int)p->pid) { - parent->wait_result_pid = (int)p->pid; - parent->wait_result_status = p->exit_status; - parent->state = PROCESS_READY; - rq_enqueue(pcpu_rq[parent->cpu_id].active, parent); - } - } - } - } else { - p->sig_pending_mask |= (1U << (uint32_t)sig); - if (p->state == PROCESS_BLOCKED || p->state == PROCESS_SLEEPING) { - sleep_queue_remove(p); - p->state = PROCESS_READY; - rq_enqueue(pcpu_rq[p->cpu_id].active, p); - } + p->state = PROCESS_READY; + rq_enqueue(pcpu_rq[wake_cpu].active, p); + sched_pcpu_inc_load(wake_cpu); } spin_unlock_irqrestore(&sched_lock, flags); + sched_ipi_resched(wake_cpu); return 0; } @@ -510,6 +502,7 @@ int process_kill_pgrp(uint32_t pgrp, int sig) { uintptr_t flags = spin_lock_irqsave(&sched_lock); int found = 0; + uint32_t ipi_mask = 0; struct process* it = ready_queue_head; if (it) { @@ -521,12 +514,18 @@ int process_kill_pgrp(uint32_t pgrp, int sig) { if (current_process->euid != it->uid && current_process->uid != it->uid) continue; } + uint32_t pcpu = it->cpu_id < SCHED_MAX_CPUS ? it->cpu_id : 0; it->sig_pending_mask |= (1U << (uint32_t)sig); + if (sig == 9) { + it->sig_blocked_mask &= ~(1U << (uint32_t)sig); + } if (it->state == PROCESS_BLOCKED || it->state == PROCESS_SLEEPING) { sleep_queue_remove(it); it->state = PROCESS_READY; - rq_enqueue(pcpu_rq[it->cpu_id].active, it); + rq_enqueue(pcpu_rq[pcpu].active, it); + sched_pcpu_inc_load(pcpu); } + if (pcpu < 32) ipi_mask |= (1U << pcpu); found = 1; } it = it->next; @@ -534,6 +533,13 @@ int process_kill_pgrp(uint32_t pgrp, int sig) { } spin_unlock_irqrestore(&sched_lock, flags); + uint32_t my_cpu = percpu_cpu_index(); + ipi_mask &= ~(1U << my_cpu); + while (ipi_mask) { + uint32_t c = (uint32_t)__builtin_ctz(ipi_mask); + sched_ipi_resched(c); + ipi_mask &= ~(1U << c); + } return found ? 0 : -ESRCH; } @@ -864,12 +870,11 @@ struct process* process_clone_create(uint32_t clone_flags, proc->addr_space = current_process->addr_space; proc->flags |= PROCESS_FLAG_THREAD; /* If this is the first thread, add the parent's own ref to the table */ - if (current_process->as_refcount == 0) { + if (as_refcount_get(proc->addr_space) == 0) { as_refcount_inc(proc->addr_space); /* parent's ref */ added_parent_as_ref = 1; } as_refcount_inc(proc->addr_space); /* child's ref */ - current_process->as_refcount++; } else { proc->addr_space = vmm_as_clone_user_cow(current_process->addr_space); if (!proc->addr_space) { @@ -886,27 +891,21 @@ struct process* process_clone_create(uint32_t clone_flags, proc->tgid = proc->pid; } - /* CLONE_FS: share cwd */ strcpy(proc->cwd, current_process->cwd); process_mount_ref_cwd(proc); - /* CLONE_FILES: share file descriptor table */ - if (clone_flags & CLONE_FILES) { - for (int i = 0; i < PROCESS_MAX_FILES; i++) { - proc->files[i] = current_process->files[i]; - if (proc->files[i]) { - __sync_fetch_and_add(&proc->files[i]->refcount, 1); - } - proc->fd_flags[i] = current_process->fd_flags[i]; + for (int i = 0; i < PROCESS_MAX_FILES; i++) { + proc->files[i] = current_process->files[i]; + if (proc->files[i]) { + __sync_fetch_and_add(&proc->files[i]->refcount, 1); } + proc->fd_flags[i] = current_process->fd_flags[i]; } - /* CLONE_SIGHAND: share signal handlers */ - if (clone_flags & CLONE_SIGHAND) { - for (int i = 0; i < PROCESS_MAX_SIG; i++) { - proc->sigactions[i] = current_process->sigactions[i]; - } + for (int i = 0; i < PROCESS_MAX_SIG; i++) { + proc->sigactions[i] = current_process->sigactions[i]; } + proc->sig_blocked_mask = current_process->sig_blocked_mask; /* CLONE_SETTLS: set TLS base */ if (clone_flags & CLONE_SETTLS) { @@ -941,15 +940,10 @@ struct process* process_clone_create(uint32_t clone_flags, /* Allocate kernel stack */ void* kstack = kstack_alloc(); if (!kstack) { - if (clone_flags & CLONE_FILES) { - process_release_file_refs(proc); - } + process_release_file_refs(proc); process_mount_unref_cwd(proc); if (clone_flags & CLONE_VM) { (void)as_refcount_dec(proc->addr_space); - if (current_process->as_refcount > 0) { - current_process->as_refcount--; - } if (added_parent_as_ref) { (void)as_refcount_dec(proc->addr_space); } diff --git a/src/kernel/syscall.c b/src/kernel/syscall.c index 0e28441d..e23a3772 100644 --- a/src/kernel/syscall.c +++ b/src/kernel/syscall.c @@ -993,8 +993,7 @@ static int syscall_clone_impl(struct registers* 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 | \ + #define CLONE_SUPPORTED_MASK (CLONE_VM | CLONE_THREAD | \ CLONE_SETTLS | CLONE_PARENT_SETTID |\ CLONE_CHILD_CLEARTID) if (clone_flags & ~CLONE_SUPPORTED_MASK) return -EINVAL; @@ -3031,6 +3030,7 @@ static int syscall_getpgrp_impl(void) { static int syscall_sigaction_impl(int sig, const struct sigaction* user_act, struct sigaction* user_oldact) { if (!current_process) return -EINVAL; if (sig <= 0 || sig >= PROCESS_MAX_SIG) return -EINVAL; + if (sig == 9 || sig == 19) return -EINVAL; if (user_oldact) { if (user_range_ok(user_oldact, sizeof(*user_oldact)) == 0) return -EFAULT; @@ -3058,6 +3058,8 @@ static int syscall_sigprocmask_impl(uint32_t how, uint32_t mask, uint32_t* old_o if (copy_to_user(old_out, &old, sizeof(old)) < 0) return -EFAULT; } + mask &= ~((1U << 9) | (1U << 19)); + /* POSIX: SIG_BLOCK=0 (OR), SIG_UNBLOCK=1 (AND-NOT), SIG_SETMASK=2 (set) */ if (how == 0U) { current_process->sig_blocked_mask |= mask; diff --git a/user/cmds/fulltest/fulltest.c b/user/cmds/fulltest/fulltest.c index fe74183d..ee539588 100644 --- a/user/cmds/fulltest/fulltest.c +++ b/user/cmds/fulltest/fulltest.c @@ -2017,6 +2017,7 @@ void _start(void) { } if (pid == 0) { + (void)sys_sigprocmask(SIG_BLOCK, (1U << SIGKILL), 0); for (;;) { __asm__ volatile("nop"); } @@ -5406,8 +5407,7 @@ void _start(void) { /* Stack grows downward on x86 */ void* sp = child_stack + 8192; - int tid = sys_clone(CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND - | CLONE_PARENT_SETTID, + int tid = sys_clone(CLONE_VM | CLONE_PARENT_SETTID, sp, &parent_tid, 0, 0); if (tid < 0) { sys_write(1, "[test] clone failed\n", diff --git a/user/ulibc/src/pthread.c b/user/ulibc/src/pthread.c index 046e2d5b..84fe5039 100644 --- a/user/ulibc/src/pthread.c +++ b/user/ulibc/src/pthread.c @@ -24,7 +24,7 @@ #define CLONE_PARENT_SETTID 0x00100000 #define CLONE_CHILD_CLEARTID 0x00200000 -#define CLONE_THREAD_FLAGS (CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND | CLONE_THREAD | CLONE_SETTLS) +#define CLONE_THREAD_FLAGS (CLONE_VM | CLONE_THREAD | CLONE_SETTLS) #define THREAD_STACK_SIZE 8192 @@ -86,7 +86,7 @@ int pthread_create(pthread_t* thread, const pthread_attr_t* attr, * * We use _syscall5 but note: parent_tidptr and child_tidptr are 0 for now, * tls is 0 (no TLS for basic threads). */ - uint32_t flags = CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND | CLONE_THREAD; + uint32_t flags = CLONE_VM | CLONE_THREAD; int ret = _syscall5(SYS_CLONE, (int)flags, (int)(uintptr_t)sp, 0, 0, 0); if (ret < 0) { -- 2.43.0