[patch] updated: tgkill patch for safe inter-thread signals

[patch] updated: tgkill patch for safe inter-thread signals

[Ulrich Drepper]

[-- Attachment #1: Type: text/plain, Size: 2294 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

This are updated versions of the patch Ingo sent some time ago to
implement a new tgkill() syscall which can in theory replace kill() and
tkill().

The first patch is just Ingo's patch, relative to the current BK
sources, plus the fix to send the correct si_pid value.

The second patch implementation the additional functionality Linus
proposed.  They allow using tgkill as a replacement for kill/tkill by
recognizing special -1 values.  This is the interdiff:

- --- linux-2.5/kernel/signal.c   2003-07-07 12:39:36.000000000 -0700
+++ linux-2.5/kernel/signal.c   2003-07-07 13:03:15.000000000 -0700
@@ -2104,8 +2104,12 @@
        int error;
        struct task_struct *p;

+       /* Handle tgkill(tgid, -1, sig) like kill(tgid, sig).  */
+       if (pid == -1)
+               return sys_kill(tgid, sig);
+
        /* This is only valid for single tasks */
- -       if (pid <= 0 || tgid <= 0)
+       if (pid <= 0 || tgid < -1 || tgid == 0)
                return -EINVAL;

        info.si_signo = sig;
@@ -2117,7 +2121,8 @@
        read_lock(&tasklist_lock);
        p = find_task_by_pid(pid);
        error = -ESRCH;
- -       if (p && (p->tgid == tgid)) {
+       /* Handle tgkill(-1, pid, sig) like tkill(pid, sig).  */
+       if (p && (p->tgid == tgid || tgid == -1)) {
                error = check_kill_permission(sig, &info, p);
                /*
                 * The null signal is a permissions and process existence


I personally would think that this addition is overkill since we cannot
get rid of the kill/tkill syscalls anyway.

I've tested both patches.  Having one of them included is really needed
since many/most people still run with limited PID ranges (maybe due to
legacy apps breaking) and the PID reuse can cause problems.

Let me know if there are any problems.  I'll try to address them right away.

- --
- --------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE/CdSE2ijCOnn/RHQRAiytAJwJq8BYPRu9Ep6U8krubKafW7nVwwCeKsq7
LKsQB68ROJYi8OrdUPFOZ5M=
=HdOW
-----END PGP SIGNATURE-----

[-- Attachment #2: d-kernel-tgkill1 --]
[-- Type: text/plain, Size: 2747 bytes --]

--- linux-2.5/arch/i386/kernel/entry.S-old	2003-06-24 09:40:12.000000000 -0700
+++ linux-2.5/arch/i386/kernel/entry.S	2003-07-07 12:36:55.000000000 -0700
@@ -876,5 +876,6 @@ ENTRY(sys_call_table)
  	.long sys_clock_nanosleep
 	.long sys_statfs64
 	.long sys_fstatfs64	
+	.long sys_tgkill
  
 nr_syscalls=(.-sys_call_table)/4
--- linux-2.5/include/asm-i386/unistd.h-old	2003-06-24 09:40:13.000000000 -0700
+++ linux-2.5/include/asm-i386/unistd.h	2003-07-07 12:32:37.000000000 -0700
@@ -275,8 +275,9 @@
 #define __NR_clock_nanosleep	(__NR_timer_create+8)
 #define __NR_statfs64		268
 #define __NR_fstatfs64		269
+#define __NR_tgkill		270
 
-#define NR_syscalls 270
+#define NR_syscalls 271
 
 /* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */
 
--- linux-2.5/kernel/signal.c-old	2003-07-05 12:08:48.000000000 -0700
+++ linux-2.5/kernel/signal.c	2003-07-07 12:39:36.000000000 -0700
@@ -579,8 +579,8 @@ static int rm_from_queue(unsigned long m
 /*
  * Bad permissions for sending the signal
  */
-static inline int check_kill_permission(int sig, struct siginfo *info,
-					struct task_struct *t)
+static int check_kill_permission(int sig, struct siginfo *info,
+				 struct task_struct *t)
 {
 	int error = -EINVAL;
 	if (sig < 0 || sig > _NSIG)
@@ -2088,6 +2088,52 @@ sys_kill(int pid, int sig)
 	return kill_something_info(sig, &info, pid);
 }
 
+/**
+ *  sys_tkill - send signal to one specific thread
+ *  @tgid: the thread group ID of the thread
+ *  @pid: the PID of the thread
+ *  @sig: signal to be sent
+ *
+ *  This syscall also checks the tgid and returns -ESRCH even if the PID
+ *  exists but it's not belonging to the target process anymore. This
+ *  method solves the problem of threads exiting and PIDs getting reused.
+ */
+asmlinkage long sys_tgkill(int tgid, int pid, int sig)
+{
+	struct siginfo info;
+	int error;
+	struct task_struct *p;
+
+	/* This is only valid for single tasks */
+	if (pid <= 0 || tgid <= 0)
+		return -EINVAL;
+
+	info.si_signo = sig;
+	info.si_errno = 0;
+	info.si_code = SI_TKILL;
+	info.si_pid = current->tgid;
+	info.si_uid = current->uid;
+
+	read_lock(&tasklist_lock);
+	p = find_task_by_pid(pid);
+	error = -ESRCH;
+	if (p && (p->tgid == tgid)) {
+		error = check_kill_permission(sig, &info, p);
+		/*
+		 * The null signal is a permissions and process existence
+		 * probe.  No signal is actually delivered.
+		 */
+		if (!error && sig && p->sighand) {
+			spin_lock_irq(&p->sighand->siglock);
+			handle_stop_signal(sig, p);
+			error = specific_send_sig_info(sig, &info, p);
+			spin_unlock_irq(&p->sighand->siglock);
+		}
+	}
+	read_unlock(&tasklist_lock);
+	return error;
+}
+
 /*
  *  Send a signal to only one task, even if it's a CLONE_THREAD task.
  */

[-- Attachment #3: d-kernel-tgkill2 --]
[-- Type: text/plain, Size: 2943 bytes --]

--- linux-2.5/arch/i386/kernel/entry.S-old	2003-06-24 09:40:12.000000000 -0700
+++ linux-2.5/arch/i386/kernel/entry.S	2003-07-07 12:36:55.000000000 -0700
@@ -876,5 +876,6 @@ ENTRY(sys_call_table)
  	.long sys_clock_nanosleep
 	.long sys_statfs64
 	.long sys_fstatfs64	
+	.long sys_tgkill
  
 nr_syscalls=(.-sys_call_table)/4
--- linux-2.5/include/asm-i386/unistd.h-old	2003-06-24 09:40:13.000000000 -0700
+++ linux-2.5/include/asm-i386/unistd.h	2003-07-07 12:32:37.000000000 -0700
@@ -275,8 +275,9 @@
 #define __NR_clock_nanosleep	(__NR_timer_create+8)
 #define __NR_statfs64		268
 #define __NR_fstatfs64		269
+#define __NR_tgkill		270
 
-#define NR_syscalls 270
+#define NR_syscalls 271
 
 /* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */
 
--- linux-2.5/kernel/signal.c-old	2003-07-05 12:08:48.000000000 -0700
+++ linux-2.5/kernel/signal.c	2003-07-07 13:03:15.000000000 -0700
@@ -579,8 +579,8 @@ static int rm_from_queue(unsigned long m
 /*
  * Bad permissions for sending the signal
  */
-static inline int check_kill_permission(int sig, struct siginfo *info,
-					struct task_struct *t)
+static int check_kill_permission(int sig, struct siginfo *info,
+				 struct task_struct *t)
 {
 	int error = -EINVAL;
 	if (sig < 0 || sig > _NSIG)
@@ -2088,6 +2088,57 @@ sys_kill(int pid, int sig)
 	return kill_something_info(sig, &info, pid);
 }
 
+/**
+ *  sys_tkill - send signal to one specific thread
+ *  @tgid: the thread group ID of the thread
+ *  @pid: the PID of the thread
+ *  @sig: signal to be sent
+ *
+ *  This syscall also checks the tgid and returns -ESRCH even if the PID
+ *  exists but it's not belonging to the target process anymore. This
+ *  method solves the problem of threads exiting and PIDs getting reused.
+ */
+asmlinkage long sys_tgkill(int tgid, int pid, int sig)
+{
+	struct siginfo info;
+	int error;
+	struct task_struct *p;
+
+	/* Handle tgkill(tgid, -1, sig) like kill(tgid, sig).  */
+	if (pid == -1)
+		return sys_kill(tgid, sig);
+
+	/* This is only valid for single tasks */
+	if (pid <= 0 || tgid < -1 || tgid == 0)
+		return -EINVAL;
+
+	info.si_signo = sig;
+	info.si_errno = 0;
+	info.si_code = SI_TKILL;
+	info.si_pid = current->tgid;
+	info.si_uid = current->uid;
+
+	read_lock(&tasklist_lock);
+	p = find_task_by_pid(pid);
+	error = -ESRCH;
+	/* Handle tgkill(-1, pid, sig) like tkill(pid, sig).  */
+	if (p && (p->tgid == tgid || tgid == -1)) {
+		error = check_kill_permission(sig, &info, p);
+		/*
+		 * The null signal is a permissions and process existence
+		 * probe.  No signal is actually delivered.
+		 */
+		if (!error && sig && p->sighand) {
+			spin_lock_irq(&p->sighand->siglock);
+			handle_stop_signal(sig, p);
+			error = specific_send_sig_info(sig, &info, p);
+			spin_unlock_irq(&p->sighand->siglock);
+		}
+	}
+	read_unlock(&tasklist_lock);
+	return error;
+}
+
 /*
  *  Send a signal to only one task, even if it's a CLONE_THREAD task.
  */