On 10/07, Roland McGrath wrote:
>
> > + if (context->siginfo)
> > + return ptrace_rw_siginfo(tracee, context, info, false);
> >
> > - return error;
> > + memset(info, 0, sizeof(*info));
> > + info->si_signo = SIGTRAP;
> > + info->si_code = context->ev_code; // XXX: ev_code was already cleared!!!
> > + info->si_pid = task_pid_vnr(tracee);
> > + info->si_uid = task_uid(tracee);
>
> Put that logic inside ptrace_rw_siginfo and call it unconditionally.
(and from the next review of PATCH 64)
> Then both helpers have everything in common, and they are static anyway,
> so you might as well just call ptrace_rw_siginfo from ptrace_request.
I mostly agree, but I'd like to keep this helper for now. I am not sure
yet, but perhaps it will be needed if we change detach to play with
ptrace_utrace_detached_ops. This was a reason why I added the separate
helper. But let me repeat I am not sure it will be really needed.
> It's
> going to check context->siginfo anyway. The forged info is a wrong lie if
> it's actually clear because it was set and then there was a SIGKILL, but we
> can't tell the difference so a race could yield that bogon anyway.
Yes. I swear, I was going to do
if (get_stop_code() == PTRACE_EVENT_SIGNAL)
return ptrace_rw_siginfo();
instead, not sure why I used ->siginfo != NULL.
This way, when we call ptrace_rw_siginfo() we know that ->siginfo
must be valid. But if we race with SIGKILL and ->siginfo was not
cleared yet we can return the wrong info which was filled by the
next dequeue_signal().
I'll recheck this, but looks like we have anotrher reason for
fatal_signal_pending() in ptrace_rw_siginfo().
Or, we can change ptrace_report_signal() (or even utrace_get_signal())
to check __fatal_signal_pending().
> Where exactly does ->siginfo get cleared in that case? I only see it
> cleared in the UTRACE_SIGNAL_REPORT case. But SIGKILL should not do any
> signal reports, it bypasses them and you might get only the DEATH event.
It does, or I missed something.
Yes, if utrace_get_signal() dequeues SIGKILL it doesn't report and
just returns.
But, ptrace_report_signal() does:
context->siginfo = info;
/*
* context->siginfo points to the caller's stack.
* Make sure the subsequent UTRACE_SIGNAL_REPORT clears
* ->siginfo before return from get_signal_to_deliver().
*/
utrace_control(task, engine, UTRACE_INTERRUPT);
this means that, even if killed, the tracee will call utrace_get_signal()
with utrace->interrupt == T. In this case UTRACE_SIGNAL_REPORT will be
reported anyway.
Right?
Oleg.
