On Tue, 29 Apr 2014 13:20:58 -0700 Matthew Dempsky <[email protected]>
wrote:
> When tracing a process in another pid namespace, it's important for
> fork event messages to contain the child's pid as seen from the
> tracer's pid namespace, not the parent's. Otherwise, the tracer won't
> be able to correlate the fork event with later SIGTRAP signals it
> receives from the child.
>
> We still risk a race condition if a ptracer from a different pid
> namespace attaches after we compute the pid_t value. However, sending
> a bogus fork event message in this unlikely scenario is still a vast
> improvement over the status quo where we always send bogus fork event
> messages to debuggers in a different pid namespace than the forking
> process.
More Oleg review would be nice, please ;)
> @@ -129,6 +130,36 @@ static inline void ptrace_event(int event, unsigned long
> message)
> }
>
> /**
> + * ptrace_event_pid - possibly stop for a ptrace event notification
> + * @event: %PTRACE_EVENT_* value to report
> + * @pid: process identifier for %PTRACE_GETEVENTMSG to return
> + *
> + * Check whether @event is enabled and, if so, report @event and @pid
> + * to the ptrace parent. @pid is reported as the pid_t seen from the
> + * the ptrace parent's pid namespace.
> + *
> + * Called without locks.
> + */
> +static inline void ptrace_event_pid(int event, struct pid *pid)
> +{
> + /*
> + * FIXME: There's a potential race if a ptracer in a different pid
> + * namespace than parent attaches between computing message below and
> + * and when we acquire tasklist_lock in ptrace_stop().
> + */
Well that's a scary comment. If we're going to leave the code in this
state then please carefully describe (within this comment) the
*consequences* of the race. Does the kernel crash? Give away your ssh
keys? If not then what.
And how would userspace recognize and/or recover from the race?
> + unsigned long message = 0;
> + struct pid_namespace *ns;
> +
> + rcu_read_lock();
> + ns = task_active_pid_ns(rcu_dereference(current->parent));
> + if (ns)
> + message = pid_nr_ns(pid, ns);
> + rcu_read_unlock();
> +
> + ptrace_event(event, message);
> +}
>
> ...
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
