(Was: ptrace: prevent PTRACE_SETREGS from corrupting stack)
Get series with b4
b4 am [email protected]Versions newest first
- v1 Latest4 patches 6 repliesIn review
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/4 | ptrace: introduce signal_wake_up_state() and ptrace_signal_wake_up()include/ kernel/ kernel/ | +18 | 0 replies | |
| 2/4 | ptrace: ensure arch_ptrace/ptrace_request can never race with SIGKILLarch/ kernel/ kernel/ | +55 | 13 replies | |
| 3/4 | ia64: kill thread_matches(), unexport ptrace_check_attach()arch/ include/ kernel/ | +1 | 1 reply | |
| 4/4 | wake_up_process() should be never used to wakeup a TASK_STOPPED/TRACED taskkernel/ | +2 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [PATCH 0/4] (Was: ptrace: prevent PTRACE_SETREGS from corrupting stack)
add lkml/cc's.
Quoted 9 lines · Linus Torvalds
Quoted 3 lines · Oleg Nesterov
Or we can do this after
wait_task_inactive()but then we need to take ->siglock again.Yes. We absolutely need siglock, since that would be exactly what would protect us against
signal_wake_up()(which is, I *think* the only thing that can ever wake up the TASK_TRACED/WAKEKILL cases).
Yes. And thus 4/4 probably should be 1/4.
Quoted 4 lines
And we'd need to make sure to re-set the WAKEKILL flag not just in all the callers of
ptrace_check_attach(), but also in the failure case ofwait_task_inactive(). I'm not sure it can actually fail if we cleared WAKEKILL, but it's all pretty subtle.
Afaics it can't fail if we clear WAKEKILL... So 2/4 assumes it should always succeed and adds the warning.
Quoted 2 lines
And when we *do* set the WAKEKILL bit again, we should make sure to wake the task in case the killable signal happened while it was clear.
Yes, yes, this is clear. And we need to ensure we can not race with attach-after-detach...
Quoted 3 lines
And I agree that this is all pretty scary and generally playing with another process' 'flags' field is some really nasty business. So I'm a bit worried about it.
Oh yes. And I was going to argue that (a much simpler) change which doesn't allow the tracee to return from ptrace_stop() is better. But then I recalled about set_task_blockstep() and changed my mind (see the changelog in 2/4).
Greg, this doesn't look like -stable material. But please let me know if you think 2/4 should be backported. With a couple of simple hacks in PTRACE_DETACH/LISTEN paths we can do this without 1/4 and without changes outside of ptrace.c. But again, probably we shouldn't do this.
Please review.
Oleg.