linux-kernel - Re: pidns : PR_SET_PDEATHSIG + SIGKILL regression

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20091003171029.GA30442@us.ibm.com>
Date:	Sat, 3 Oct 2009 10:10:29 -0700
From:	Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
To:	Daniel Lezcano <dlezcano@...ibm.com>
Cc:	Sukadev Bhattiprolu <sukadev@...ibm.com>,
	Linux Containers <containers@...ts.osdl.org>, oleg@...hat.com,
	roland@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: pidns : PR_SET_PDEATHSIG + SIGKILL regression


Cc Oleg and Roland and moving discussion to LKML.

Daniel Lezcano [dlezcano@...ibm.com] wrote:
> Hi,
>
> I noticed a changed behaviour with the PR_SET_PDEATHSIG and SIGKILL  
> between different kernel versions.
>
> With a kernel 2.6.27.21-78.2.41.fc9.x86_64, the SIGKILL signal is  
> delivered to the child process when the parent dies but with a 2.6.31  
> kernel version that don't happen.
>
> The program below shows the problem. I remember there was were some  
> modifications about not killing the init process of the container from  
> inside, but in this case, that happens _conceptually_ from outside.  
> Keeping this feature is very important to be able to wipe out the  
> container when the parent process of the container dies.

(Test case moved to attachment).

---
Container init must not be immune to signals from parent. But as pointed
out by Daniel Lezcano: 

https://lists.linux-foundation.org/pipermail/containers/2009-October/021121.html

container-init is currently immune to signals from parent, if sent via
->pdeath_signal. This is because the siginfo for ->pdeath_signal is set to
SEND_SIG_NOINFO which is considered special.

This quick patch passes in siginfo explicitly (just like we do when sending
SIGCHLD to parent) and seems to fix the problem. Not though sure if
->pdeath_signal needs to be 'is_si_special()'.

Changelog [v2]:
	- [Oleg Nesterov] Add missing initializer, ->si_code = SI_USER
	- [Sukadev Bhattiprolu] Use 'tgid' of parent instead of 'pid'.

---
 kernel/exit.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Index: linux-2.6/kernel/exit.c
===================================================================
--- linux-2.6.orig/kernel/exit.c	2009-10-02 19:23:00.000000000 -0700
+++ linux-2.6/kernel/exit.c	2009-10-03 10:02:42.000000000 -0700
@@ -738,8 +738,20 @@ static struct task_struct *find_new_reap
 static void reparent_thread(struct task_struct *father, struct task_struct *p,
 				struct list_head *dead)
 {
-	if (p->pdeath_signal)
-		group_send_sig_info(p->pdeath_signal, SEND_SIG_NOINFO, p);
+	if (p->pdeath_signal) {
+		struct siginfo info;
+
+		info.si_code = SI_USER;
+		info.si_signo = p->pdeath_signal;
+		info.si_errno = 0;
+
+		rcu_read_lock();
+		info.si_pid = task_tgid_nr_ns(father, task_active_pid_ns(p));
+		info.si_uid = __task_cred(father)->uid;
+		rcu_read_unlock();
+
+		group_send_sig_info(p->pdeath_signal, &info, p);
+	}
 
 	list_move_tail(&p->sibling, &p->real_parent->children);
 

View attachment "pdeath.c" of type "text/x-csrc" (932 bytes)