[PATCH 71] RFC, fix ptrace(PTRACE_DETACH, signr) logic

[Oleg Nesterov]

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))