| Message ID | 514B742E.3010304@canonical.com |
|---|---|
| State | New |
| Headers | show |
On 21/03/13 20:57, John Johansen wrote: > From d6a1da349c76ac2ebe4774d1da9fb7e660df01d3 Mon Sep 17 00:00:00 2001 > From: John Johansen <john.johansen@canonical.com> > Date: Thu, 21 Mar 2013 05:04:13 -0700 > Subject: [PATCH] UBUNTU: SAUCE: Fix ptrace when task is in task_is_stopped() > state > > This patch fixes a regression in ptrace, introduced by commit 9e74eb39 > (backport of 9899d11f) which makes assumptions about ptrace behavior > which are not true in the 2.6.32 kernel. > > BugLink: http://bugs.launchpad.net/bugs/1145234 > > 9899d11f makes the assumption that task_is_stopped() is not a valid state > in ptrace because it is built on top of a series of patches which change > how the TASK_STOPPED state is tracked (321fb561 which requires d79fdd6d > and several other patches). > > Because Lucid does not have the set of patches that make task_is_stopped() > an invalid state in ptrace_check_attach, partially revert 9e74eb39 so > that ptrace_check_attach() correctly handles task_is_stopped(). However > we must replace the assignment of TASK_TRACED with __TASK_TRACED to > ensure TASK_WAKEKILL is cleared. > > Signed-off-by: John Johansen <john.johansen@canonical.com> > --- > kernel/ptrace.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/kernel/ptrace.c b/kernel/ptrace.c > index d0036f0..d9c8c47 100644 > --- a/kernel/ptrace.c > +++ b/kernel/ptrace.c > @@ -81,14 +81,18 @@ void __ptrace_unlink(struct task_struct *child) > } > > /* Ensure that nothing can wake it up, even SIGKILL */ > -static bool ptrace_freeze_traced(struct task_struct *task) > +static bool ptrace_freeze_traced(struct task_struct *task, int kill) > { > - bool ret = false; > + bool ret = true; > > spin_lock_irq(&task->sighand->siglock); > - if (task_is_traced(task) && !__fatal_signal_pending(task)) { > + if (task_is_stopped(task) && !__fatal_signal_pending(task)) > task->state = __TASK_TRACED; > - ret = true; > + else if (!kill) { > + if (task_is_traced(task) && !__fatal_signal_pending(task)) > + task->state = __TASK_TRACED; > + else > + ret = false; > } > spin_unlock_irq(&task->sighand->siglock); > > @@ -131,7 +135,7 @@ int ptrace_check_attach(struct task_struct *child, int kill) > * child->sighand can't be NULL, release_task() > * does ptrace_unlink() before __exit_signal(). > */ > - if (kill || ptrace_freeze_traced(child)) > + if (ptrace_freeze_traced(child, kill)) > ret = 0; > } > read_unlock(&tasklist_lock); > I believe this fixes the original semantics as intended. I had to study this a bit harder now that kill is passed into ptrace_freeze_traced() but from what I can see this does the trick. Acked-by: Colin Ian King <colin.king@canonical.com>
On Thu, Mar 21, 2013 at 01:57:18PM -0700, John Johansen wrote: > From d6a1da349c76ac2ebe4774d1da9fb7e660df01d3 Mon Sep 17 00:00:00 2001 > From: John Johansen <john.johansen@canonical.com> > Date: Thu, 21 Mar 2013 05:04:13 -0700 > Subject: [PATCH] UBUNTU: SAUCE: Fix ptrace when task is in task_is_stopped() > state > > This patch fixes a regression in ptrace, introduced by commit 9e74eb39 > (backport of 9899d11f) which makes assumptions about ptrace behavior > which are not true in the 2.6.32 kernel. Seems to be correct. It fixes a problem introduced by a backport to fix CVE-2013-0871. Nice catch! Cheers, -- Luis > > BugLink: http://bugs.launchpad.net/bugs/1145234 > > 9899d11f makes the assumption that task_is_stopped() is not a valid state > in ptrace because it is built on top of a series of patches which change > how the TASK_STOPPED state is tracked (321fb561 which requires d79fdd6d > and several other patches). > > Because Lucid does not have the set of patches that make task_is_stopped() > an invalid state in ptrace_check_attach, partially revert 9e74eb39 so > that ptrace_check_attach() correctly handles task_is_stopped(). However > we must replace the assignment of TASK_TRACED with __TASK_TRACED to > ensure TASK_WAKEKILL is cleared. > > Signed-off-by: John Johansen <john.johansen@canonical.com> > --- > kernel/ptrace.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/kernel/ptrace.c b/kernel/ptrace.c > index d0036f0..d9c8c47 100644 > --- a/kernel/ptrace.c > +++ b/kernel/ptrace.c > @@ -81,14 +81,18 @@ void __ptrace_unlink(struct task_struct *child) > } > > /* Ensure that nothing can wake it up, even SIGKILL */ > -static bool ptrace_freeze_traced(struct task_struct *task) > +static bool ptrace_freeze_traced(struct task_struct *task, int kill) > { > - bool ret = false; > + bool ret = true; > > spin_lock_irq(&task->sighand->siglock); > - if (task_is_traced(task) && !__fatal_signal_pending(task)) { > + if (task_is_stopped(task) && !__fatal_signal_pending(task)) > task->state = __TASK_TRACED; > - ret = true; > + else if (!kill) { > + if (task_is_traced(task) && !__fatal_signal_pending(task)) > + task->state = __TASK_TRACED; > + else > + ret = false; > } > spin_unlock_irq(&task->sighand->siglock); > > @@ -131,7 +135,7 @@ int ptrace_check_attach(struct task_struct *child, int kill) > * child->sighand can't be NULL, release_task() > * does ptrace_unlink() before __exit_signal(). > */ > - if (kill || ptrace_freeze_traced(child)) > + if (ptrace_freeze_traced(child, kill)) > ret = 0; > } > read_unlock(&tasklist_lock); > -- > 1.8.1.2 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 21.03.2013 21:57, John Johansen wrote: > From d6a1da349c76ac2ebe4774d1da9fb7e660df01d3 Mon Sep 17 00:00:00 2001 > From: John Johansen <john.johansen@canonical.com> > Date: Thu, 21 Mar 2013 05:04:13 -0700 > Subject: [PATCH] UBUNTU: SAUCE: Fix ptrace when task is in task_is_stopped() > state > > This patch fixes a regression in ptrace, introduced by commit 9e74eb39 > (backport of 9899d11f) which makes assumptions about ptrace behavior > which are not true in the 2.6.32 kernel. > > BugLink: http://bugs.launchpad.net/bugs/1145234 > > 9899d11f makes the assumption that task_is_stopped() is not a valid state > in ptrace because it is built on top of a series of patches which change > how the TASK_STOPPED state is tracked (321fb561 which requires d79fdd6d > and several other patches). > > Because Lucid does not have the set of patches that make task_is_stopped() > an invalid state in ptrace_check_attach, partially revert 9e74eb39 so > that ptrace_check_attach() correctly handles task_is_stopped(). However > we must replace the assignment of TASK_TRACED with __TASK_TRACED to > ensure TASK_WAKEKILL is cleared. > > Signed-off-by: John Johansen <john.johansen@canonical.com> Ok, it seems the logic reverts back to before task_is_stopped was invalid and since before it waited for TASK_TRACED later on it looks to be sensible to switch that to __TASK_TRACED which is used now.
diff --git a/kernel/ptrace.c b/kernel/ptrace.c index d0036f0..d9c8c47 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -81,14 +81,18 @@ void __ptrace_unlink(struct task_struct *child) } /* Ensure that nothing can wake it up, even SIGKILL */ -static bool ptrace_freeze_traced(struct task_struct *task) +static bool ptrace_freeze_traced(struct task_struct *task, int kill) { - bool ret = false; + bool ret = true; spin_lock_irq(&task->sighand->siglock); - if (task_is_traced(task) && !__fatal_signal_pending(task)) { + if (task_is_stopped(task) && !__fatal_signal_pending(task)) task->state = __TASK_TRACED; - ret = true; + else if (!kill) { + if (task_is_traced(task) && !__fatal_signal_pending(task)) + task->state = __TASK_TRACED; + else + ret = false; } spin_unlock_irq(&task->sighand->siglock); @@ -131,7 +135,7 @@ int ptrace_check_attach(struct task_struct *child, int kill) * child->sighand can't be NULL, release_task() * does ptrace_unlink() before __exit_signal(). */ - if (kill || ptrace_freeze_traced(child)) + if (ptrace_freeze_traced(child, kill)) ret = 0; } read_unlock(&tasklist_lock);