[v1] fork: initialize function graph state before copy_exec_state()

[PATCH] fork: initialize function graph state before copy_exec_state()

Posted by Jérémy Jean 1 month, 1 week ago

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.

Initialize the child function graph state immediately after
dup_task_struct(), before the first fallible operation.

Fixes: 6b1c66c9cca9 ("exec_state: relocate dumpable information")
Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 kernel/fork.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 1e68404bd773..cec82a64a07c 100644
--- 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);
 	retval = copy_exec_state(clone_flags, p);
 	if (retval)
 		goto bad_fork_free;
@@ -2165,8 +2166,6 @@ __latent_entropy struct task_struct *copy_process(
 	 */
 	p->clear_child_tid = (clone_flags & CLONE_CHILD_CLEARTID) ? args->child_tid : NULL;
 
-	ftrace_graph_init_task(p);
-
 	rt_mutex_init_task(p);
 	raw_spin_lock_init(&p->blocked_lock);
 
-- 
2.47.3

Re: [PATCH] fork: initialize function graph state before copy_exec_state()

Posted by Bradley Morgan 1 month, 1 week ago

On 21 August 2026 10:22:08 BST, Jérémy Jean
<Jeremy.Jean@oss.cyber.gouv.fr> wrote:
>From: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>
>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 <include@grrlz.net>

Thanks!

Re: [PATCH] fork: initialize function graph state before copy_exec_state()

Posted by Jérémy Jean 1 month, 1 week ago

On 2026-08-21 23:58, Bradley Morgan wrote:
> On 21 August 2026 10:22:08 BST, Jérémy Jean
> <Jeremy.Jean@oss.cyber.gouv.fr> wrote:
>> From: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>> 
>> 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 <include@grrlz.net>
> 
> Thanks!

Hello Bradley,

Many thanks for your feedback and advices.
I will send a v2 shortly.

Regards,
Jérémy