* [PATCH 0/2] [GIT PULL][urgent] function-graph: allocation and freeing of ret_stack
@ 2009-06-02 22:02 Steven Rostedt
2009-06-02 22:02 ` [PATCH 1/2] function-graph: move initialization of new tasks up in fork Steven Rostedt
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Steven Rostedt @ 2009-06-02 22:02 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker, stable
Ingo,
Here's a couple of more fixes.
Please pull the latest tip/tracing/urgent-1 tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/tracing/urgent-1
Steven Rostedt (2):
function-graph: move initialization of new tasks up in fork
function-graph: always initialize task ret_stack
----
kernel/fork.c | 10 ++++------
kernel/trace/ftrace.c | 6 ++++--
2 files changed, 8 insertions(+), 8 deletions(-)
--
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] function-graph: move initialization of new tasks up in fork
2009-06-02 22:02 [PATCH 0/2] [GIT PULL][urgent] function-graph: allocation and freeing of ret_stack Steven Rostedt
@ 2009-06-02 22:02 ` Steven Rostedt
2009-06-02 22:24 ` Frederic Weisbecker
2009-06-02 22:02 ` [PATCH 2/2] function-graph: always initialize task ret_stack Steven Rostedt
2009-06-07 9:51 ` [PATCH 0/2] [GIT PULL][urgent] function-graph: allocation and freeing of ret_stack Ingo Molnar
2 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2009-06-02 22:02 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker, stable
[-- Attachment #1: 0001-function-graph-move-initialization-of-new-tasks-up-i.patch --]
[-- Type: text/plain, Size: 2386 bytes --]
From: Steven Rostedt <srostedt@redhat.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@goodmis.org>
---
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(¤t->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
--
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] function-graph: always initialize task ret_stack
2009-06-02 22:02 [PATCH 0/2] [GIT PULL][urgent] function-graph: allocation and freeing of ret_stack Steven Rostedt
2009-06-02 22:02 ` [PATCH 1/2] function-graph: move initialization of new tasks up in fork Steven Rostedt
@ 2009-06-02 22:02 ` Steven Rostedt
2009-06-07 9:51 ` [PATCH 0/2] [GIT PULL][urgent] function-graph: allocation and freeing of ret_stack Ingo Molnar
2 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2009-06-02 22:02 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker, stable
[-- Attachment #1: 0002-function-graph-always-initialize-task-ret_stack.patch --]
[-- Type: text/plain, Size: 1341 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
On creating a new task while running the function graph tracer, if
we fail to allocate the ret_stack, and then fail the fork, the
code will free the parent ret_stack. This is because the child
duplicated the parent and currently points to the parent's ret_stack.
This patch always initializes the task's ret_stack to NULL.
[ Impact: prevent crash of parent on low memory during fork ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/ftrace.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 1664d3f..bb081f3 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2738,6 +2738,9 @@ void unregister_ftrace_graph(void)
/* Allocate a return stack for newly created task */
void ftrace_graph_init_task(struct task_struct *t)
{
+ /* Make sure we do not use the parent ret_stack */
+ t->ret_stack = NULL;
+
if (atomic_read(&ftrace_graph_active)) {
struct ftrace_ret_stack *ret_stack;
@@ -2753,8 +2756,7 @@ void ftrace_graph_init_task(struct task_struct *t)
/* make curr_ret_stack visable before we add the ret_stack */
smp_wmb();
t->ret_stack = ret_stack;
- } else
- t->ret_stack = NULL;
+ }
}
void ftrace_graph_exit_task(struct task_struct *t)
--
1.6.3.1
--
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] function-graph: move initialization of new tasks up in fork
2009-06-02 22:02 ` [PATCH 1/2] function-graph: move initialization of new tasks up in fork Steven Rostedt
@ 2009-06-02 22:24 ` Frederic Weisbecker
0 siblings, 0 replies; 5+ messages in thread
From: Frederic Weisbecker @ 2009-06-02 22:24 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, stable, Oleg Nesterov
On Tue, Jun 02, 2009 at 06:02:13PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.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@goodmis.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(¤t->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
>
> --
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] [GIT PULL][urgent] function-graph: allocation and freeing of ret_stack
2009-06-02 22:02 [PATCH 0/2] [GIT PULL][urgent] function-graph: allocation and freeing of ret_stack Steven Rostedt
2009-06-02 22:02 ` [PATCH 1/2] function-graph: move initialization of new tasks up in fork Steven Rostedt
2009-06-02 22:02 ` [PATCH 2/2] function-graph: always initialize task ret_stack Steven Rostedt
@ 2009-06-07 9:51 ` Ingo Molnar
2 siblings, 0 replies; 5+ messages in thread
From: Ingo Molnar @ 2009-06-07 9:51 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-kernel, Andrew Morton, Frederic Weisbecker, stable
* Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Ingo,
>
> Here's a couple of more fixes.
>
> Please pull the latest tip/tracing/urgent-1 tree, which can be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> tip/tracing/urgent-1
>
>
> Steven Rostedt (2):
> function-graph: move initialization of new tasks up in fork
> function-graph: always initialize task ret_stack
>
> ----
> kernel/fork.c | 10 ++++------
> kernel/trace/ftrace.c | 6 ++++--
> 2 files changed, 8 insertions(+), 8 deletions(-)
Pulled, thanks Steve!
( Note, we need to keep these two commits in mind and forward them
to stable@kernel.org in a few days - as they have no Cc:
stable@kernel.org tags. )
Ingo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-06-07 9:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-02 22:02 [PATCH 0/2] [GIT PULL][urgent] function-graph: allocation and freeing of ret_stack Steven Rostedt
2009-06-02 22:02 ` [PATCH 1/2] function-graph: move initialization of new tasks up in fork Steven Rostedt
2009-06-02 22:24 ` Frederic Weisbecker
2009-06-02 22:02 ` [PATCH 2/2] function-graph: always initialize task ret_stack Steven Rostedt
2009-06-07 9:51 ` [PATCH 0/2] [GIT PULL][urgent] function-graph: allocation and freeing of ret_stack Ingo Molnar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.