linux-kernel - Re: [PATCH 1/2] function-graph: move initialization of new tasks up in fork

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090602222443.GC6041@nowhere>
Date:	Wed, 3 Jun 2009 00:24:45 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>, stable@...nel.org,
	Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCH 1/2] function-graph: move initialization of new tasks
	up in fork

On Tue, Jun 02, 2009 at 06:02:13PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@...hat.com>
> 
> When the function graph tracer is enabled, all new tasks must allocate
> a ret_stack to place the return address of functions. This is because
> the function graph tracer will replace the real return address with a
> call to the tracing of the exit function.
> 
> This initialization happens in fork, but it happens too late. If fork
> fails, then it will call free_task and that calls the freeing of this
> ret_stack. But before initialization happens, the new (failed) task
> points to its parents ret_stack. If a fork failure happens during
> the function trace, it would be catastrophic for the parent.
> 
> Also, there's no need to call ftrace_graph_exit_task from fork, since
> it is called by free_task which fork calls on failure.
> 
> [ Impact: prevent crash during failed fork running function graph tracer ]
> 
> Signed-off-by: Steven Rostedt <rostedt@...dmis.org>


Ah, thanks a lot. It was on my TODO list.
Oleg Nesterov reported me that problem...

Frederic.


> ---
>  kernel/fork.c |   10 ++++------
>  1 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index b9e2edd..c4b1e35 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -982,6 +982,8 @@ static struct task_struct *copy_process(unsigned long clone_flags,
>  	if (!p)
>  		goto fork_out;
>  
> +	ftrace_graph_init_task(p);
> +
>  	rt_mutex_init_task(p);
>  
>  #ifdef CONFIG_PROVE_LOCKING
> @@ -1131,8 +1133,6 @@ static struct task_struct *copy_process(unsigned long clone_flags,
>  		}
>  	}
>  
> -	ftrace_graph_init_task(p);
> -
>  	p->pid = pid_nr(pid);
>  	p->tgid = p->pid;
>  	if (clone_flags & CLONE_THREAD)
> @@ -1141,7 +1141,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
>  	if (current->nsproxy != p->nsproxy) {
>  		retval = ns_cgroup_clone(p, pid);
>  		if (retval)
> -			goto bad_fork_free_graph;
> +			goto bad_fork_free_pid;
>  	}
>  
>  	p->set_child_tid = (clone_flags & CLONE_CHILD_SETTID) ? child_tidptr : NULL;
> @@ -1233,7 +1233,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
>  		spin_unlock(&current->sighand->siglock);
>  		write_unlock_irq(&tasklist_lock);
>  		retval = -ERESTARTNOINTR;
> -		goto bad_fork_free_graph;
> +		goto bad_fork_free_pid;
>  	}
>  
>  	if (clone_flags & CLONE_THREAD) {
> @@ -1268,8 +1268,6 @@ static struct task_struct *copy_process(unsigned long clone_flags,
>  	cgroup_post_fork(p);
>  	return p;
>  
> -bad_fork_free_graph:
> -	ftrace_graph_exit_task(p);
>  bad_fork_free_pid:
>  	if (pid != &init_struct_pid)
>  		free_pid(pid);
> -- 
> 1.6.3.1
> 
> -- 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/