Messages in this thread | | | Date | Tue, 25 Oct 2022 12:19:00 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH v2] ptrace: disable single step in __ptrace_unlink for protecting init task |
| |
On 10/25, chen zhang wrote: > > Thanks for your reply. I think kernel should not panic when the > application has a bug, or a fault operation such as ctrl+c,
a) init is special. If it exits, the kernel panics. This is by design.
b) debugger can always crash the tracee. In particular if it exits without ptrace(PTRACE_DETACH) which implies user_disable_single_step().
> This patch can really prevent kernel panic on > my x86 machine.
Really? You ignored this part of my previous email,
Not to mention I don't understand how your patch can actually help. If nothing else,
- debugger does ptrace(PTRACE_SINGLESTEP), this wakes the tracee up
- the tracee enters force_sig_info_to_task(SIGTRAP) after single step
- debugger exits, __ptrace_unlink() clears ptrace/TIF_SINGLESTEP
- force_sig_info_to_task() clears SIGNAL_UNKILLABLE, the traced init will be killed.
Am I wrong?
Finally,
> --- a/kernel/ptrace.c > +++ b/kernel/ptrace.c > @@ -130,6 +130,8 @@ void __ptrace_unlink(struct task_struct *child) > put_cred(old_cred); > > spin_lock(&child->sighand->siglock); > + if (unlikely(child->signal->flags & SIGNAL_UNKILLABLE)) > + user_disable_single_step(child); > child->ptrace = 0; > /*
I don't understnd why do you call user_disable_single_step() with ->siglock held, but this is minor.
user_disable_single_step(child) assumes that child is stopped and frozen, see ptrace_freeze_traced(). This is not necessarily true if __ptrace_unlink() is called by the exiting tracer, so the patch is wrong in any case.
Nack, sorry.
Oleg.
| |