Re: [PATCH] arch_ptrace_stop

[Oleg Nesterov]

Roland McGrath wrote:
>
> +static int sigkill_pending(struct task_struct *tsk)
> +{
> +     return ((sigismember(&tsk->pending.signal, SIGKILL) ||
> +              sigismember(&tsk->signal->shared_pending.signal, SIGKILL)) &&
> +             !unlikely(sigismember(&tsk->blocked, SIGKILL)));
> +}

How is it possible that SIGKILL is blocked?

>  static void ptrace_stop(int exit_code, int nostop_code, siginfo_t *info)
>  {
> +     int killed = 0;
> +
> +     if (arch_ptrace_stop_needed(exit_code, info)) {
> +             /*
> +              * The arch code has something special to do before a
> +              * ptrace stop.  This is allowed to block, e.g. for faults
> +              * on user stack pages.  We can't keep the siglock while
> +              * calling arch_ptrace_stop, so we must release it now.
> +              * To preserve proper semantics, we must do this before
> +              * any signal bookkeeping like checking group_stop_count.
> +              * Meanwhile, a SIGKILL could come in before we retake the
> +              * siglock.  That must prevent us from sleeping in TASK_TRACED.
> +              * So after regaining the lock, we must check for SIGKILL.
> +              */
> +             spin_unlock_irq(&current->sighand->siglock);
> +             arch_ptrace_stop(exit_code, info);
> +             spin_lock_irq(&current->sighand->siglock);
> +             killed = sigkill_pending(current);
> +     }
> +
>       /*
>        * If there is a group stop in progress,
>        * we must participate in the bookkeeping.
> @@ -1604,7 +1635,7 @@ static void ptrace_stop(int exit_code, i
>       spin_unlock_irq(&current->sighand->siglock);
>       try_to_freeze();
>       read_lock(&tasklist_lock);
> -     if (may_ptrace_stop()) {
> +     if (!unlikely(killed) && may_ptrace_stop()) {

Could you please explain this change in more details?

Currently ptrace_stop() schedules in TASK_TRACED state even if we have a
pending SIGKILL. With this patch this is still possible, but unless
arch_ptrace_stop_needed() is true and thus we will check sigkill_pending().

Suppose the task was SIGKILL'ed and does ptrace_notify(PTRACE_EVENT_EXIT),
now the resulting action depends on arch_ptrace_stop_needed().

I don't claim this is wrong, just trying to understand.

Thanks,

Oleg.

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