[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20221025101858.GB17158@redhat.com>
Date: Tue, 25 Oct 2022 12:19:00 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: chen zhang <chenzhang@...inos.cn>
Cc: chenzhang_0901@....com, linux-kernel@...r.kernel.org
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.
Powered by blists - more mailing lists