Messages in this thread | | | Date | Sun, 16 Dec 2007 19:24:02 +0300 | From | Oleg Nesterov <> | Subject | Re: [PATCH] arch_ptrace_stop |
| |
Roland, I am sorry for delay,
On 12/13, Roland McGrath wrote: > > > 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(). > > Currently the siglock is always held throughout. The case this change > addresses is when no SIGKILL was already pending before we took the lock. > Currently, a new SIGKILL cannot come in until we've released the lock, > which is after we've set TASK_TRACED. The signal's sender will hold the > lock while checking each thread's state, waking up any in TASK_TRACED. > > When arch_ptrace_stop_needed() is true, we release the siglock for an > unknown period (might block, etc). If a SIGKILL is sent there, it becomes > pending while we are in TASK_RUNNING or a normal blocked state. Next we > finish arch_ptrace_stop() and reacquire the siglock.
Yes, yes, I see.
> Now entering > TASK_TRACED would leave us unkillable because SIGKILL is already pending > and nothing else (except PTRACE_CONT et al) will try to wake us up.
But this doesn't differ from the case when SIGKILL was already pending when we enter ptrace_stop, and arch_ptrace_stop_needed() == false, that was my point.
Yes, arch_ptrace_stop() can take a long time, might block, etc. But what about TIF_SYSCALL_TRACE ? The task can recieve SIGKILL while executing the syscall which can also block and so on, but do_syscall_trace(entryexit == 1) doesn't check the pending signal.
I should clarify my question. What I can't understand is the subtle dependency on the result of arch_ptrace_stop_needed(). This means that it is hard to predict the behaviour.
IOW, can't we
- ignore the pending SIGKILL (current behaviour) -- OR -- - always check it unconditionally, before setting TASK_TRACED
? This looks a bit more consistent to me.
Please also note "before setting TASK_TRACED" above. With this patch we set TASK_TRACED under ->siglock, and then change the ->state to TASK_RUNNING if killed == 1. Minor, but this doesn't look correct, we can fool the tracer which does ptrace_check_attach().
Oleg.
| |