[PATCH 1/3] signals: cleanup security_task_kill() usage/implementation

[Oleg Nesterov]

Every implementation of ->task_kill() does nothing when the signal comes from
the kernel. This is correct, but means that check_kill_permission() should call
security_task_kill() only for SI_FROMUSER() case, and we can remove the same
check from ->task_kill() implementations.

(sadly, check_kill_permission() is the last user of signal->session/__session
 but we can't s/task_session_nr/task_session/ here).

NOTE: Eric W. Biederman pointed out cap_task_kill() should die, and I think he
is very right.

Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>

 kernel/signal.c            |   27 ++++++++++++++-------------
 security/commoncap.c       |    3 ---
 security/selinux/hooks.c   |    3 ---
 security/smack/smack_lsm.c |    9 ---------
 4 files changed, 14 insertions(+), 28 deletions(-)

--- 25/kernel/signal.c~1_LSM_KILL	2008-02-25 18:12:57.000000000 +0300
+++ 25/kernel/signal.c	2008-02-25 18:15:38.000000000 +0300
@@ -526,22 +526,23 @@ static int rm_from_queue(unsigned long m
 static int check_kill_permission(int sig, struct siginfo *info,
 				 struct task_struct *t)
 {
-	int error = -EINVAL;
+	int error;
+
 	if (!valid_signal(sig))
-		return error;
+		return -EINVAL;
 
-	if (info == SEND_SIG_NOINFO || (!is_si_special(info) && SI_FROMUSER(info))) {
-		error = audit_signal_info(sig, t); /* Let audit system see the signal */
-		if (error)
-			return error;
-		error = -EPERM;
-		if (((sig != SIGCONT) ||
-			(task_session_nr(current) != task_session_nr(t)))
-		    && (current->euid ^ t->suid) && (current->euid ^ t->uid)
-		    && (current->uid ^ t->suid) && (current->uid ^ t->uid)
-		    && !capable(CAP_KILL))
+	if (info != SEND_SIG_NOINFO && (is_si_special(info) || SI_FROMKERNEL(info)))
+		return 0;
+
+	error = audit_signal_info(sig, t); /* Let audit system see the signal */
+	if (error)
 		return error;
-	}
+
+	if (((sig != SIGCONT) || (task_session_nr(current) != task_session_nr(t)))
+	    && (current->euid ^ t->suid) && (current->euid ^ t->uid)
+	    && (current->uid ^ t->suid) && (current->uid ^ t->uid)
+	    && !capable(CAP_KILL))
+		return -EPERM;
 
 	return security_task_kill(t, info, sig, 0);
 }
--- 25/security/commoncap.c~1_LSM_KILL	2008-02-25 18:07:54.000000000 +0300
+++ 25/security/commoncap.c	2008-02-25 18:52:16.000000000 +0300
@@ -543,9 +543,6 @@ int cap_task_setnice (struct task_struct
 int cap_task_kill(struct task_struct *p, struct siginfo *info,
 				int sig, u32 secid)
 {
-	if (info != SEND_SIG_NOINFO && (is_si_special(info) || SI_FROMKERNEL(info)))
-		return 0;
-
 	/*
 	 * Running a setuid root program raises your capabilities.
 	 * Killing your own setuid root processes was previously
--- 25/security/smack/smack_lsm.c~1_LSM_KILL	2008-02-15 16:59:20.000000000 +0300
+++ 25/security/smack/smack_lsm.c	2008-02-25 18:54:01.000000000 +0300
@@ -1094,15 +1094,6 @@ static int smack_task_kill(struct task_s
 			   int sig, u32 secid)
 {
 	/*
-	 * Special cases where signals really ought to go through
-	 * in spite of policy. Stephen Smalley suggests it may
-	 * make sense to change the caller so that it doesn't
-	 * bother with the LSM hook in these cases.
-	 */
-	if (info != SEND_SIG_NOINFO &&
-	    (is_si_special(info) || SI_FROMKERNEL(info)))
-		return 0;
-	/*
 	 * Sending a signal requires that the sender
 	 * can write the receiver.
 	 */
--- 25/security/selinux/hooks.c~1_LSM_KILL	2008-02-15 16:59:20.000000000 +0300
+++ 25/security/selinux/hooks.c	2008-02-25 18:57:23.000000000 +0300
@@ -3194,9 +3194,6 @@ static int selinux_task_kill(struct task
 	if (rc)
 		return rc;
 
-	if (info != SEND_SIG_NOINFO && (is_si_special(info) || SI_FROMKERNEL(info)))
-		return 0;
-
 	if (!sig)
 		perm = PROCESS__SIGNULL; /* null signal; existence test */
 	else

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/