[PATCH v2] ptrace: disable single step in __ptrace_unlink for protecting init task

[PATCH v2] ptrace: disable single step in __ptrace_unlink for protecting init task

[chen zhang]

I got below panic when doing fuzz test:

Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000005
CPU: 2 PID: 1 Comm: systemd Kdump: loaded Not tainted 6.1.0-rc1 #1
Hardware name: LENOVO 20L5A07XCD/20L5A07XCD, BIOS N24ET56W (1.31 ) 02/19/2020
Call Trace:
[  157.210356]  dump_stack_lvl+0x49/0x63
[  157.210364]  dump_stack+0x10/0x16
[  157.210368]  panic+0x10c/0x299
[  157.210375]  do_exit.cold+0x15/0x15
[  157.210381]  do_group_exit+0x35/0x90
[  157.210386]  get_signal+0x910/0x960
[  157.210390]  ? signal_wake_up_state+0x2e/0x40
[  157.210396]  ? complete_signal+0xd0/0x2c0
[  157.210402]  arch_do_signal_or_restart+0x37/0x7c0
[  157.210408]  ? send_signal_locked+0xf5/0x140
[  157.210416]  exit_to_user_mode_prepare+0x133/0x180
[  157.210423]  irqentry_exit_to_user_mode+0x9/0x20
[  157.210428]  noist_exc_debug+0xea/0x150
[  157.210433]  asm_exc_debug+0x34/0x40
[  157.210438] RIP: 0033:0x7fcf2a8e51c9
[  157.210442] Code: ff ff 73 01 c3 48 8b 0d c5 7c 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 ba 00 00 00 <0f> 05 c3 0f 1f 40 00 f3 0f 1e fa b8 ea 00 00 00 0f 05 48 3d 01 f0
[  157.210446] RSP: 002b:00007ffd7dc44678 EFLAGS: 00000302
[  157.210451] RAX: 00000000000000ba RBX: 000055f7c0363170 RCX: 000055f7c04d2820
[  157.210454] RDX: 00000000ffffffff RSI: ffffffffffffffff RDI: 000055f7c0363170
[  157.210457] RBP: 0000000000000001 R08: 0000000000000000 R09: 0000000000001dd0
[  157.210460] R10: 00007ffd7ddc9090 R11: 000000000000d7da R12: 0000000000000001
[  157.210463] R13: ffffffffffffffff R14: 000055f7bf3557c1 R15: 0000000000000000

If a task attaches init task and is single stepping it, when this task
exits, ptrace value will be cleaned. It causes SIGNAL_UNKILLABLE flag
cleaned, and init task will lose the protection. Init task maybe be killed
by SIGTRAP signal because of stepping enabled. So syscall tracing and
stepping should be turned off for protecting init task before ptrace value
is cleaned.

Signed-off-by: chen zhang <chenzhang@kylinos.cn>
---
change for v2: remove _TIF_SINGLESTEP because of some architecture has not defined _TIF_SINGLESTEP such as mips.
 
Thanks for your reply. I think kernel should not panic when the application has a bug, or a fault operation such as ctrl+c,

or the attack to the kernel. I have debugged this panic for several weeks. This patch can really prevent kernel panic on

my x86 machine. I agree your describe about kernel panic case. But when debugger exits, SIGNAL_UNKILLABLE flag will protect

init task at the first time. The SIGTRAP signal can not kill init task. Because of this patch, it disabled single step debug on

time. So SIGTRAP signal will not be sent, and the kernel panic can not appear. 
---
 kernel/ptrace.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 54482193e1ed..c8f7195bef1c 100644
--- 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;
 	/*
 	 * Clear all pending traps and TRAPPING.  TRAPPING should be
-- 
2.25.1

Re: [PATCH v2] ptrace: disable single step in __ptrace_unlink for protecting init task

[Oleg Nesterov]

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.