2012-06-26 01:18:19

by Salman Qazi

Subject: [PATCH] sched: fix fork() error path to not crash.

In dup_task_struct, if arch_dup_task_struct fails, the clean up
code fails to clean up correctly. That's because the clean up
code depends on unininitalized ti->task pointer. We fix this
by making sure that the task and thread_info know about each other
before we attempt to take the error path.

Signed-off-by: Salman Qazi <[email protected]>
---
kernel/fork.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index ab5211b..f00e319 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -304,12 +304,17 @@ static struct task_struct *dup_task_struct(struct task_struct *orig)
}

err = arch_dup_task_struct(tsk, orig);
- if (err)
- goto out;

+ /*
+ * We defer looking at err, because we will need this setup
+ * for the clean up path to work correctly.
+ */
tsk->stack = ti;
-
setup_thread_stack(tsk, orig);
+
+ if (err)
+ goto out;
+
clear_user_return_notifier(tsk);
clear_tsk_need_resched(tsk);
stackend = end_of_stack(tsk);

2012-06-26 08:40:15

by Peter Zijlstra

Subject: Re: [PATCH] sched: fix fork() error path to not crash.

On Mon, 2012-06-25 at 18:18 -0700, Salman Qazi wrote:
> In dup_task_struct, if arch_dup_task_struct fails, the clean up
> code fails to clean up correctly. That's because the clean up
> code depends on unininitalized ti->task pointer. We fix this
> by making sure that the task and thread_info know about each other
> before we attempt to take the error path.
>
> Signed-off-by: Salman Qazi <[email protected]>

Cute, however did you find that?

2012-06-26 16:58:44

by Salman Qazi

Subject: Re: [PATCH] sched: fix fork() error path to not crash.

On Tue, Jun 26, 2012 at 1:40 AM, Peter Zijlstra <[email protected]> wrote:
> On Mon, 2012-06-25 at 18:18 -0700, Salman Qazi wrote:
>> In dup_task_struct, if arch_dup_task_struct fails, the clean up
>> code fails to clean up correctly. ?That's because the clean up
>> code depends on unininitalized ti->task pointer. ?We fix this
>> by making sure that the task and thread_info know about each other
>> before we attempt to take the error path.
>>
>> Signed-off-by: Salman Qazi <[email protected]>
>
> Cute, however did you find that?

Our test infrastructure folks have developed a fault injection
framework geared towards detecting bugs triggered by memory allocation
failures in random places.