Forgot to mention...
I didn't test this patch and it is in fact buggy, we should
set sig = context->siginfo->si_signo when !valid_signal(sig).
This is just RFC to know what do you think.
On 10/06, Oleg Nesterov wrote:
>
> On 10/06, Jan Kratochvil wrote:
> >
> > On Tue, 06 Oct 2009 15:10:10 +0200, Oleg Nesterov wrote:
> > > Once again. Suppose that the tracer does ptrace(PTRACE_DETACH, SIGXXX).
> > > Currently, if the next thacer attaches right after this detach it has no
> > > way to intercept SIGXXX, it will be never reported via ptrace_signal().
> >^
> > No matter if it gets reported to the new tracer still SIGXXX should never
> > get
> > lost.
>
> OK, great. Roland, do you agree?
>
> If yes, perhaps we can avoid ptrace_utrace_detached_ops logic? At least
> for the first version which can be sent to lkml.
>
> If the tracer does ptrace(DETACH, SIGNR) - just send this signal, that
> is all.
>
> If the tracer exits (valid_signal() == F), send the already reported
> signal. Otherwise, if the tracee didn't report a signal - do nothing.
>
> What do you think?
>
> ---
>
> kernel/ptrace.c | 36 +++++++++++++++++++++++++++++++-----
> 1 file changed, 31 insertions(+), 5 deletions(-)
>
> --- PU/kernel/ptrace.c~71_PARTLY_FIX_DETACH 2009-10-06 02:25:02.000000000
> +0200
> +++ PU/kernel/ptrace.c 2009-10-06 18:12:12.000000000 +0200
> @@ -82,6 +82,33 @@ static struct utrace_engine *ptrace_look
> &ptrace_utrace_ops, NULL);
> }
>
> +static void detach_signal(struct task_struct *tracee,
> + struct ptrace_context *context, int sig)
> +{
> + siginfo_t *info = NULL;
> +
> + switch (get_stop_code(context)) {
> + case PTRACE_EVENT_SYSCALL_ENTRY:
> + case PTRACE_EVENT_SYSCALL_EXIT:
> + if (valid_signal(sig))
> + info = SEND_SIG_PRIV;
> + break;
> +
> + case PTRACE_EVENT_SIGNAL:
> + if (valid_signal(sig) && sig != context->signr)
> + info = SEND_SIG_NOINFO;
> + else
> + info = context->siginfo;
> + break;
> + }
> +
> + // XXX: if info == context->siginfo we can race with SIGKILL,
> + // but afaics this is harmless ? send_sig_info() uses *info
> + // under ->siglock, the worst case SIGKILL will be sent twice.
> + if (info)
> + send_sig_info(sig, info, tracee);
> +}
> +
> static void ptrace_detach_task(struct task_struct *child, int sig)
> {
> struct utrace_engine *engine = ptrace_lookup_engine(child);
> @@ -90,6 +117,9 @@ static void ptrace_detach_task(struct ta
> if (unlikely(IS_ERR(engine)))
> return;
>
> + if (sig)
> + detach_signal(child, ptrace_context(engine), sig);
> +
> ret = utrace_control(child, engine, UTRACE_DETACH);
> WARN_ON(ret && ret != -EINPROGRESS &&
> ret != -ESRCH && ret != -EALREADY);
> @@ -668,12 +698,8 @@ static void ptrace_do_detach(struct task
> */
> detach = tracee->ptrace != 0;
> release = false;
> - if (likely(detach)) {
> - // XXX: temporary hack
> - if (data && valid_signal(data))
> - send_sig(data, tracee, 1);
> + if (likely(detach))
> release = __ptrace_detach(current, tracee);
> - }
> write_unlock_irq(&tasklist_lock);
>
> if (unlikely(release))
