Re: [PATCH] fork: initialize function graph state before copy_exec_state()
Bradley Morgan <[email protected]>
| Newsgroups | org.kvack.linux-mm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 21 August 2026 10:22:08 BST, Jérémy Jean <[email protected]> wrote: >From: Jérémy Jean <[email protected]> > >dup_task_struct() copies the parent's task_struct, including ret_stack. >ftrace_graph_init_task() clears the copied function graph state, but it >currently runs after copy_exec_state(). > >For non-CLONE_VM forks, copy_exec_state() allocates a new task_exec_state. >If that allocation fails, copy_process() reaches bad_fork_free and >free_task() calls ftrace_graph_exit_task(). Since the child still carries >the parent's ret_stack pointer, the unwind frees the parent's active >function graph return stack. The parent subsequently accesses freed memory >from function_graph_enter_regs(). KASAN reports a UAF. Ugh, real one. dup_task_struct() does *tsk = *orig so the child inherits the parent's ret_stack, task_exec_state_copy() can return -ENOMEM, and free_task() calls ftrace_graph_exit_task() unconditionally, so the child happily frees the parent's return stack. Exactly as described. >Initialize the child function graph state immediately after >dup_task_struct(), before the first fallible operation. Good! The alloc inside ftrace_graph_init_task() swallows its own failure, so moving it earlier adds no new error path. >Fixes: 6b1c66c9cca9 ("exec_state: relocate dumpable information") Tag is right, before that commit nothing between dup_task_struct() and the init call could fail. >Assisted-by: Codex:gpt-5 Love you declared AI If you still have the KASAN report, pasting it in would help the folks applying this see it is real and not theoretical. >diff --git a/kernel/fork.c b/kernel/fork.c >--- a/kernel/fork.c >+++ b/kernel/fork.c >@@ -2139,6 +2139,7 @@ __latent_entropy struct task_struct *copy_process( > p = dup_task_struct(current, node); > if (!p) > goto fork_out; >+ ftrace_graph_init_task(p); One small ask, feel free to bikeshed: nobody is going to remember why this call has to sit before copy_exec_state(), and the next reorder breaks it again. Something like: /* * Must run before the first fallible op, so error paths never * free the parent's ret_stack. */ ftrace_graph_init_task(p); With or without that, idc .. Reviewed-by: Bradley Morgan <[email protected]> Thanks!