Re: SCO: "thread creation is about a thousand times faster than on

From: Linus Torvalds (torvalds@transmeta.com)
Date: Sun Aug 27 2000 - 19:25:07 EST

In article <20000827051241.A24890@hq.fsmlabs.com>,
 <yodaiken@fsmlabs.com> wrote:
>
>POSIX says nothing, as far as I can tell, about the effect of exec on
>threads. However, it does say pending signals must be inherited. It's
>unclear what should happen in Linux, but it might be good for the new
>process to still be a thread in the thread group -- although it won't
>be sharing memory anymore.

No. Simply for security purposes we _definitely_ need to "de-thread" the
process when it does an exec. Otherwise we have "interesting" issues
with suid execs that are part of a non-suid thread group.

We could choose to just not allow suid execs, but I think they do
potentially make sense, and I don't see why Linux shouldn't allow a
thread to exec a suid binary (we break the shared VM part automatically,
we might as well break the shared thread part).

Note that for _pthreads_ this kind of exec is illegal anyway. Silly
POSIX threads standard had to take user-level threading models into
account, so under POSIX threads an execve() needs to kill off all other
threads. Another senseless limitation - one that we can _emulate_, of
course, but not one that we should consider part of the native thread
environment.

Btw, here's a simple thread-group patch. It doesn't actually _do_
anything with the thread group part, but this is the kind of frame-work
I've envisioned. Completely untested, of course, but with these kinds
of security issues taken into account.

Comments? At least it's small and simple..

                                Linus 

----
diff -u --recursive --new-file penguin/linux/fs/exec.c linux/fs/exec.c
--- penguin/linux/fs/exec.c	Mon Aug 14 10:19:55 2000
+++ linux/fs/exec.c	Sun Aug 27 17:29:52 2000
@@ -496,6 +496,25 @@
 	write_unlock(&files->file_lock);
 }
 
+/*
+ * An execve() will automatically "de-thread" the process.
+ * Note: we don't have to hold the tasklist_lock to test
+ * whether we migth need to do this. If we're not part of
+ * a thread group, there is no way we can become one
+ * dynamically. And if we are, we only need to protect the
+ * unlink - even if we race with the last other thread exit,
+ * at worst the list_del_init() might end up being a no-op.
+ */
+static inline void de_thread(struct task_struct *tsk)
+{
+	if (!list_empty(&tsk->thread_group)) {
+		write_lock_irq(&tasklist_lock);
+		list_del_init(&tsk->thread_group);
+		write_unlock_irq(&tasklist_lock);
+	}
+	tsk->tgid = tsk->pid;
+}
+
 int flush_old_exec(struct linux_binprm * bprm)
 {
 	char * name;
@@ -533,6 +552,8 @@
 	current->comm[i] = '\0';
 
 	flush_thread();
+
+	de_thread(current);
 
 	if (bprm->e_uid != current->euid || bprm->e_gid != current->egid || 
 	    permission(bprm->file->f_dentry->d_inode,MAY_READ))
diff -u --recursive --new-file penguin/linux/include/linux/sched.h linux/include/linux/sched.h
--- penguin/linux/include/linux/sched.h	Wed Aug 23 11:35:07 2000
+++ linux/include/linux/sched.h	Sun Aug 27 17:31:18 2000
@@ -38,6 +38,7 @@
 #define CLONE_PTRACE	0x00002000	/* set if we want to let tracing continue on the child too */
 #define CLONE_VFORK	0x00004000	/* set if the parent wants the child to wake it up on mm_release */
 #define CLONE_PARENT	0x00008000	/* set if we want to have the same parent as the cloner */
+#define CLONE_THREAD	0x00010000	/* set if we want to clone the "thread group" */
 
 /*
  * These are the constant used to fake the fixed-point load-average
@@ -311,6 +312,7 @@
 	pid_t pgrp;
 	pid_t tty_old_pgrp;
 	pid_t session;
+	pid_t tgid;
 	/* boolean value for session group leader */
 	int leader;
 	/* 
@@ -319,6 +321,7 @@
 	 * p->p_pptr->pid)
 	 */
 	struct task_struct *p_opptr, *p_pptr, *p_cptr, *p_ysptr, *p_osptr;
+	struct list_head thread_group;
 
 	/* PID hash table linkage. */
 	struct task_struct *pidhash_next;
@@ -435,6 +438,7 @@
     prev_task:		&tsk,						\
     p_opptr:		&tsk,						\
     p_pptr:		&tsk,						\
+    thread_group:	LIST_HEAD_INIT(tsk.thread_group),		\
     wait_chldexit:	__WAIT_QUEUE_HEAD_INITIALIZER(tsk.wait_chldexit),\
     real_timer:		{						\
 	function:		it_real_fn				\
@@ -818,6 +822,7 @@
 	nr_threads--;
 	unhash_pid(p);
 	REMOVE_LINKS(p);
+	list_del(&p->thread_group);
 	write_unlock_irq(&tasklist_lock);
 }
 
diff -u --recursive --new-file penguin/linux/kernel/fork.c linux/kernel/fork.c
--- penguin/linux/kernel/fork.c	Wed Aug 23 11:33:48 2000
+++ linux/kernel/fork.c	Sun Aug 27 17:32:08 2000
@@ -661,7 +661,13 @@
 	 * Let it rip!
 	 */
 	retval = p->pid;
+	p->tgid = retval;
+	INIT_LIST_HEAD(&p->thread_group);
 	write_lock_irq(&tasklist_lock);
+	if (clone_flags & CLONE_THREAD) {
+		p->tgid = current->tgid;
+		list_add(&p->thread_group, &current->thread_group);
+	}
 	SET_LINKS(p);
 	hash_pid(p);
 	nr_threads++;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

This archive was generated by hypermail 2b29 : Thu Aug 31 2000 - 21:00:20 EST