[patch] new sys_tkill2() system call, 2.5.70

[patch] new sys_tkill2() system call, 2.5.70

[Ingo Molnar]

the attached patch, against 2.5.70, adds a new system-call called
sys_tkill2():

	asmlinkage long sys_tkill2(int tgid, int pid, int sig);

this new syscall solves the problem of PID-reuse. The pthread_kill()  
interface itself cannot guarantee via current kernel mechanisms that a
thread wont go away (call pthread_exit()) before the signal is delivered -
due to threads being detached. If some other application creates threads
fast enough and makes the PID space overwrap, then it might happen that
the wrong application gets the signal.

the tgid never changes during the lifetime of the application, so
specifying tgid guarantees that pthread_kill() will only send signals to
threads within this application.

(i also added the rule of tgid == 0 meaning 'no tgid check' - this made it
possible to merge the previous sys_tkill() API into the new sys_tkill2()
API. The sys_tkill API is of course preserved.)

Ulrich says that this interface is OK and desired for glibc. The patch was
sanity-compiled & booted on x86 SMP.

	Ingo

--- linux/include/asm-i386/unistd.h.orig	
+++ linux/include/asm-i386/unistd.h	
@@ -273,6 +273,7 @@
 #define __NR_clock_gettime	(__NR_timer_create+6)
 #define __NR_clock_getres	(__NR_timer_create+7)
 #define __NR_clock_nanosleep	(__NR_timer_create+8)
+#define __NR_tkill2		268
 
 #define NR_syscalls 268
 
--- linux/arch/i386/kernel/entry.S.orig	
+++ linux/arch/i386/kernel/entry.S	
@@ -874,6 +874,7 @@ ENTRY(sys_call_table)
  	.long sys_clock_gettime		/* 265 */
  	.long sys_clock_getres
  	.long sys_clock_nanosleep
+	.long sys_tkill2
  
  
 nr_syscalls=(.-sys_call_table)/4
--- linux/kernel/signal.c.orig	
+++ linux/kernel/signal.c	
@@ -570,7 +570,7 @@ 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,
+static int check_kill_permission(int sig, struct siginfo *info,
 					struct task_struct *t)
 {
 	int error = -EINVAL;
@@ -1930,11 +1930,17 @@ sys_kill(int pid, int sig)
 	return kill_something_info(sig, &info, pid);
 }
 
-/*
- *  Send a signal to only one task, even if it's a CLONE_THREAD task.
+/**
+ *  sys_tkill2 - send signal to one specific thread
+ *  @tgid: the thread group ID of the thread - if 0 then no check is done.
+ *  @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_tkill(int pid, int sig)
+asmlinkage long sys_tkill2(int tgid, int pid, int sig)
 {
 	struct siginfo info;
 	int error;
@@ -1953,7 +1959,7 @@ sys_tkill(int pid, int sig)
 	read_lock(&tasklist_lock);
 	p = find_task_by_pid(pid);
 	error = -ESRCH;
-	if (p) {
+	if (p && (!tgid || (p->tgid == tgid))) {
 		error = check_kill_permission(sig, &info, p);
 		/*
 		 * The null signal is a permissions and process existence
@@ -1970,6 +1976,12 @@ sys_tkill(int pid, int sig)
 	return error;
 }
 
+asmlinkage long sys_tkill(int pid, int sig)
+{
+	return sys_tkill2(0, pid, sig);
+}
+
+
 asmlinkage long
 sys_rt_sigqueueinfo(int pid, int sig, siginfo_t __user *uinfo)
 {

Re: [patch] new sys_tkill2() system call, 2.5.70

[Linus Torvalds]

On Tue, 3 Jun 2003, Ingo Molnar wrote:
> 
> the attached patch, against 2.5.70, adds a new system-call called
> sys_tkill2():

I'd suggest changing the name. It's not "tkill2", it's a totally new 
system call with different inputs.

How about calling it "tgkill()" for "thread" and "group", which are the 
new inputs?

It would also seem a lot cleaner that the "any" value be "-1" (like it is
for the other kill functions), and it works for both tgid _and_ for pid,
so that

	tgkill(-1, pid, sig) == tkill(pid, sig) == kill thread
	tgkill(pid, -1, sig) == kill(pid, sig) == kill group

You made the "any" value be 0, and working only for the group. At least to
me, "0" historically means "this process group", while "-1" means "any"
for the signals.

("0" for "this thread group" might make sense, but if I read the patch 
correctly, you really have "0 == _any_ thread group").

Hmm?

		Linus

Re: [patch] new sys_tkill2() system call, 2.5.70

[Ingo Molnar]

On Tue, 3 Jun 2003, Linus Torvalds wrote:

> How about calling it "tgkill()" for "thread" and "group", which are the
> new inputs?

ok, agreed.

> It would also seem a lot cleaner that the "any" value be "-1" (like it
> is for the other kill functions), and it works for both tgid _and_ for
> pid, so that
> 
> 	tgkill(-1, pid, sig) == tkill(pid, sig) == kill thread
> 	tgkill(pid, -1, sig) == kill(pid, sig) == kill group

well, the current sys_tkill implementation does not recognize '-1' at all:

        /* This is only valid for single tasks */
        if (pid <= 0)
                return -EINVAL;

it's a simple path to send signals to a single thread and nothing more.

are you suggesting to extend sys_tgkill() functionality to also detect -1
for the PID, and do a process-signal send? I dont think that's necessary,
because it overlaps the already existing sys_kill() functionality - and
probably it would just end up being an internal call to
kill_something_info() anyway.

if the goal is completeness then < -1 negative pid values should probably
be recognized as 'process group' values as well?

then sys_tgkill would basically be a super-kill(), encompassing all the
kill() semantics we have currently, extended with the group-specific send
functionality.

	Ingo

Re: [patch] new sys_tkill2() system call, 2.5.70

[Linus Torvalds]

On Tue, 3 Jun 2003, Ingo Molnar wrote:
> 
> are you suggesting to extend sys_tgkill() functionality to also detect -1
> for the PID, and do a process-signal send?

I don't care much, but that zero special case is definitely not a good 
idea.

You might make the thing acceptable to me by just removing the "zero means 
everythign", and replacing that logic with

	if (!tgid)
		tgid = current->tgid;

which is similar to how we handle zeroes in some other places. In other 
words, a zero is not a "wildcard", it means "_this_ thread group".

		Linus

Re: [patch] new sys_tkill2() system call, 2.5.70

[Ingo Molnar]

On Tue, 3 Jun 2003, Linus Torvalds wrote:

> > are you suggesting to extend sys_tgkill() functionality to also detect -1
> > for the PID, and do a process-signal send?
> 
> I don't care much, but that zero special case is definitely not a good 
> idea.
> 
> You might make the thing acceptable to me by just removing the "zero means 
> everythign", and replacing that logic with
> 
> 	if (!tgid)
> 		tgid = current->tgid;
> 
> which is similar to how we handle zeroes in some other places. In other 
> words, a zero is not a "wildcard", it means "_this_ thread group".

i did it this way mostly because i wanted to avoid duplicating code, and
wanted to 'fold' tkill into the new variant.

in the attached patch sys_tgkill(tgid,pid,sig) means only that, nothing
more: send the signal to the thread specified by (tgid,pid) - return
-ESRCH if it does not exist in that combination.

	Ingo

--- linux/include/asm-i386/unistd.h.orig	
+++ linux/include/asm-i386/unistd.h	
@@ -273,6 +273,7 @@
 #define __NR_clock_gettime	(__NR_timer_create+6)
 #define __NR_clock_getres	(__NR_timer_create+7)
 #define __NR_clock_nanosleep	(__NR_timer_create+8)
+#define __NR_tgkill		268
 
 #define NR_syscalls 268
 
--- linux/arch/i386/kernel/entry.S.orig	
+++ linux/arch/i386/kernel/entry.S	
@@ -874,6 +874,7 @@ ENTRY(sys_call_table)
  	.long sys_clock_gettime		/* 265 */
  	.long sys_clock_getres
  	.long sys_clock_nanosleep
+	.long sys_tgkill
  
  
 nr_syscalls=(.-sys_call_table)/4
--- linux/kernel/signal.c.orig	
+++ linux/kernel/signal.c	
@@ -570,7 +570,7 @@ 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,
+static int check_kill_permission(int sig, struct siginfo *info,
 					struct task_struct *t)
 {
 	int error = -EINVAL;
@@ -1930,6 +1930,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->pid;
+	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.
  */