From c12ab945fc4ed6650874668cc89e0a21518690bf Mon Sep 17 00:00:00 2001 From: Luke Craig Date: Thu, 2 Jul 2026 14:35:22 -0400 Subject: [PATCH] portal: pin target mm with get_task_mm() before access_remote_vm handle_op_read_procargs() and handle_op_read_procenv() looked up the target task with the unreferenced get_target_task_by_id() and then used task->mm directly, calling access_remote_vm() without pinning the mm. That faults when the target has no valid user mm (kernel thread), is exiting (mm being torn down), or when its mm is reaped concurrently: access_remote_vm() -> get_user_pages_remote() -> down_read() on a stale mmap lock -> kernel Oops. On the signal/kill path the read runs in the caller's syscall context, so when the caller is init the Oops panics the whole system ("Attempted to kill init"). Mirror fs/proc/base.c: add get_target_task_mm(), which does the pid lookup and get_task_mm() inside a single rcu read-side section. get_task_mm() takes task_lock (no sleep, safe under rcu), returns NULL for kernel-thread/already-exited tasks, and pins the mm with an mm_users reference so it stays live until mmput(). Both readers now acquire the mm this way and mmput() on every exit path. Include linux/sched/mm.h (>=4.11) for get_task_mm/mmput. Generic fix: any firmware whose userspace resolves a target process's args/env on the signal-send path (e.g. lifeguard intercepting kill for blocked signals) could trigger the fault. --- src/portal/portal_internal.h | 6 ++++++ src/portal/portal_mem.c | 36 ++++++++++++++++++++++++++++++++ src/portal/portal_osi.c | 40 ++++++++++++++++++++---------------- 3 files changed, 64 insertions(+), 18 deletions(-) diff --git a/src/portal/portal_internal.h b/src/portal/portal_internal.h index 568a6d0..0f550ed 100644 --- a/src/portal/portal_internal.h +++ b/src/portal/portal_internal.h @@ -44,6 +44,12 @@ typedef void (*portal_op_handler)(portal_region *mem_region); // Helper function to get task based on pid from mem_region struct task_struct *get_target_task_by_id(portal_region *mem_region); +// Safely acquire the target task's mm for remote memory access. Looks up the +// task and pins its mm with get_task_mm() under rcu, so the returned mm stays +// live until mmput(). Returns NULL for no task / kernel thread / exiting task. +// When non-NULL and is_current is provided, *is_current reflects task==current. +struct mm_struct *get_target_task_mm(portal_region *mem_region, bool *is_current); + // Generate handler prototypes #define X(lower, upper) void handle_op_##lower(portal_region *mem_region); PORTAL_OP_LIST diff --git a/src/portal/portal_mem.c b/src/portal/portal_mem.c index f99821b..4245d35 100644 --- a/src/portal/portal_mem.c +++ b/src/portal/portal_mem.c @@ -5,6 +5,10 @@ #include /* For CONFIG_COMPAT */ #include #include /* For init_pid_ns / find_pid_ns */ +#include /* task_struct; get_task_mm/mmput (<4.11) */ +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,11,0) +#include /* get_task_mm / mmput (>=4.11) */ +#endif /* Helper function to determine if an address is in kernel space */ bool igloo_is_kernel_addr(unsigned long addr) @@ -105,6 +109,38 @@ struct task_struct *get_target_task_by_id(portal_region* mem_region) return task; } +struct mm_struct *get_target_task_mm(portal_region *mem_region, bool *is_current) +{ + pid_t target_pid = (pid_t)(mem_region->header.pid); + struct task_struct *task; + struct mm_struct *mm = NULL; + + /* + * The bare get_target_task_by_id() returns an *unreferenced* task; callers + * that then touched task->mm directly and called access_remote_vm() could + * fault -- the mm may be a kernel-thread/exiting mm, or get reaped mid + * access (observed: access_remote_vm -> down_read on a stale mmap lock -> + * kernel Oops that panics init on the kill/signal path). Mirror fs/proc: + * hold rcu across both the pid lookup and get_task_mm(), which pins the mm + * (mm_users ref) and returns NULL for kernel threads / already-exited tasks. + * get_task_mm() only takes task_lock (a spinlock) and does not sleep, so it + * is safe inside the rcu read-side critical section. The returned mm stays + * live until the caller's mmput(). + */ + rcu_read_lock(); + if (target_pid == CURRENT_PID_NUM) + task = current; + else + task = pid_task(find_pid_ns(target_pid, &init_pid_ns), PIDTYPE_PID); + if (task) { + if (is_current) + *is_current = (task == current); + mm = get_task_mm(task); + } + rcu_read_unlock(); + return mm; +} + void handle_op_read(portal_region *mem_region) { int resp; diff --git a/src/portal/portal_osi.c b/src/portal/portal_osi.c index 092729b..1f68b30 100644 --- a/src/portal/portal_osi.c +++ b/src/portal/portal_osi.c @@ -560,22 +560,18 @@ void handle_op_osi_proc_mem(portal_region *mem_region) void handle_op_read_procargs(portal_region *mem_region) { - struct task_struct *task = get_target_task_by_id(mem_region); - struct mm_struct *mm = task ? task->mm : NULL; + bool is_current = false; + /* Pin the target's mm (get_task_mm) so access_remote_vm never runs against + * a kernel-thread/exiting/reaped mm -- see get_target_task_mm(). */ + struct mm_struct *mm = get_target_task_mm(mem_region, &is_current); char *buf = PORTAL_DATA(mem_region); unsigned long arg_start, arg_end; size_t len = 0; int i; int ret; - igloo_debug_osi("igloo: Handling HYPER_OP_READ_PROCARGS (pid=%d, comm='%s')\n", - task ? task->pid : -1, task ? task->comm : "NULL"); - - if (!task) { - igloo_debug_osi("igloo: No task found for pid %llu\n", - (unsigned long long)(mem_region->header.addr)); - goto fail; - } + igloo_debug_osi("igloo: Handling HYPER_OP_READ_PROCARGS (pid=%llu, is_current=%d)\n", + (unsigned long long)(mem_region->header.pid), is_current); if (!mm || !mm->arg_end || !mm->arg_start || mm->arg_end <= mm->arg_start) { igloo_debug_osi("igloo: Invalid memory area for procargs\n"); @@ -595,9 +591,9 @@ void handle_op_read_procargs(portal_region *mem_region) } /* Read the arguments data - use different methods based on whether it's current task */ - if (task != current) { + if (!is_current) { /* For other processes, use access_remote_vm */ - igloo_debug_osi("igloo: Using access_remote_vm for process %d\n", task->pid); + igloo_debug_osi("igloo: Using access_remote_vm for target process\n"); if (access_remote_vm(mm, arg_start, buf, len, FOLL_FORCE) != len) { igloo_debug_osi("igloo: Failed to read arguments area\n"); goto fail; @@ -641,9 +637,12 @@ void handle_op_read_procargs(portal_region *mem_region) mem_region->header.size = (len); mem_region->header.op = (HYPER_RESP_READ_OK); igloo_debug_osi("igloo: Read procargs: len=%zu\n", len); + mmput(mm); return; fail: + if (mm) + mmput(mm); snprintf(PORTAL_DATA(mem_region), CHUNK_SIZE, "UNKNOWN_PROCARGS"); mem_region->header.size = (strlen(PORTAL_DATA(mem_region))); mem_region->header.op = (HYPER_RESP_READ_FAIL); @@ -652,14 +651,16 @@ void handle_op_read_procargs(portal_region *mem_region) void handle_op_read_procenv(portal_region *mem_region) { - struct task_struct *task = get_target_task_by_id(mem_region); - struct mm_struct *mm = task ? task->mm : NULL; + bool is_current = false; + /* Pin the target's mm (get_task_mm) so access_remote_vm never runs against + * a kernel-thread/exiting/reaped mm -- see get_target_task_mm(). */ + struct mm_struct *mm = get_target_task_mm(mem_region, &is_current); unsigned long env_start, env_end, len; char *buf = PORTAL_DATA(mem_region); int ret; - igloo_debug_osi("igloo: Handling HYPER_OP_READ_PROCENV (pid=%d, comm='%s')\n", - task ? task->pid : -1, task ? task->comm : "NULL"); + igloo_debug_osi("igloo: Handling HYPER_OP_READ_PROCENV (pid=%llu, is_current=%d)\n", + (unsigned long long)(mem_region->header.pid), is_current); if (!mm || !mm->env_end || !mm->env_start || mm->env_end <= mm->env_start) { goto fail; @@ -676,9 +677,9 @@ void handle_op_read_procenv(portal_region *mem_region) } // Use different methods based on whether we're accessing current task or another task - if (task != current) { + if (!is_current) { // For other processes, use access_remote_vm - igloo_debug_osi("igloo: Using access_remote_vm for process %d\n", task->pid); + igloo_debug_osi("igloo: Using access_remote_vm for target process\n"); ret = 0; if (access_remote_vm(mm, env_start, buf, len, FOLL_FORCE) != len) { igloo_debug_osi("igloo: access_remote_vm failed for procenv at %#lx (len %lu)\n", @@ -716,9 +717,12 @@ void handle_op_read_procenv(portal_region *mem_region) mem_region->header.size = (len); mem_region->header.op = (HYPER_RESP_READ_OK); igloo_debug_osi("igloo: Read procenv from stack: '%s' (len=%lu)\n", buf, len); + mmput(mm); return; fail: + if (mm) + mmput(mm); snprintf(PORTAL_DATA(mem_region), CHUNK_SIZE, "UNKNOWN_PROCENV"); mem_region->header.size = (strlen(PORTAL_DATA(mem_region))); mem_region->header.op = (HYPER_RESP_READ_FAIL);