Re: [patch] proposed fix for ptrace() SMP race

From: Andrea Arcangeli (andrea@suse.de)
Date: Fri Sep 07 2001 - 08:28:58 EST


On Thu, Sep 06, 2001 at 10:21:14PM -0700, David Mosberger wrote:
> >>>>> On Fri, 7 Sep 2001 03:28:01 +0200, Andrea Arcangeli <andrea@suse.de> said:
>
> Andrea> For making sure the task isn't wakenup while it's under
> Andrea> ptrace we should just do that in
> Andrea> kernel/signal.c::ignored_signal() as far I can tell.
>
> This doesn't make sense: ignored_signal() is too late as
> handle_stop_signal() will already have woken up the task in response
> to a SIGCONT. Also, if you're suggesting to ignore SIGCONT while a
> PT_PTRACED is set, that certainly wouldn't be right. We only want to

correct, I suggest to ignore SIGCONT as well while PT_PTRACED is set.

> *delay* the wakeup while the ptrace() system call is running (which is
> much shorter than the period of time PT_PTRACED is set). So, as far
> as I can tell, you'd have to add more locking to the signal path,

Not more locking, just an additional check:

--- 2.4.10pre4aa1/kernel/signal.c.~1~ Sun Sep 2 20:04:01 2001
+++ 2.4.10pre4aa1/kernel/signal.c Fri Sep 7 15:22:23 2001
@@ -382,7 +382,7 @@
         switch (sig) {
         case SIGKILL: case SIGCONT:
                 /* Wake up the process if stopped. */
- if (t->state == TASK_STOPPED)
+ if (t->state == TASK_STOPPED && !(t->ptrace & PT_PTRACED))
                         wake_up_process(t);
                 t->exit_code = 0;
                 rm_sig_from_queue(SIGSTOP, t);

> So, I still think cpus_allowed is a safer and better approach at least

forget that. Also when you restore the cpus_allowed you won't
effectively wakup the task, it will just keep floating in the runqueue
but we won't try to reschedule the other idle cpus it so it's broken.

> Hmmh, looking at ptrace() more closely, the entire locking situation
> seems to be a bit confused. For example, what's stopping wait4() from
> releasing the task structure just after ptrace() released the
> tasklist_lock and before it checked child->state?

the get_task_struct()

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



This archive was generated by hypermail 2b29 : Fri Sep 07 2001 - 21:00:41 EST