Hi Linus,
If the master thread of a thread group (PID==TGID) performs an execve() then
it is possible to end up with two or more thread groups with the same TGID.
Would you be willing to have an execve() on the master thread kill all the
other threads? (And thus be slightly closer to POSIX compliance).
Similarly, if the master thread exits, should all the other threads be killed
too?
If a subsidary thread does an execve(), then what should happen? I can see two
ways of handling it: (1) Make it seem that the master thread did an execve(),
and kill all the other threads (which is closer to POSIX); and (2) allow the
child thread to depart the thread group and set up by itself (as happens now).
Of course, comparing Linux thread groups to POSIX threads doesn't hold up very
well:-)
David
Benjamin LaHaise <[email protected]> wrote:
> > It seems that the TGID must also be a valid PID so that signal handling
> > will work correctly...
> >
> > This could be solved by making PIDs independent of processes, but that's
> > really not very nice.
>
> Oh, interesting. Note that we already allocate from the pid space for other
> things, so it's entirely doable.
But we still have to be able to send signals to the TGID. If there's a
guaranteed PID of the same value, then that isn't a problem (except at the
moment where multiple independent thread groups of the same TGID can be
constructed).
This could be done by making the PID independent of the process and giving it
an ops table to direct signal handling and to control /proc/<pid> dir lookup.
BTW what other things are PIDs allocated for? I can see that PIDs won't be
allocated if they are in use as PIDs, TGIDs and PGIDs. As far as I can see,
get_pid() is only called by do_fork().
> Another choice would be to simply disallow the thread group leader from
> exec'ing.
Hmmm... Maybe... Return what error, though?
> Linux threads traditionally try to remain as close to process behaviour as
> possible, and allowing an individual thread to exec is very useful, so I'm
> still in favour of allocating a new tgid.
Yes, I agree that letting subsidary threads to escape the group is reasonable,
but the master thread is definitely trickier:-/
As mentioned above, there'd be nothing backing the TGID, so kill() would have
to search a list of thread groups too...
David
On Thu, Feb 28, 2002 at 03:36:27PM +0000, David Howells wrote:
> > How about: (3) make execve allocate a new thread group id?
>
> It seems that the TGID must also be a valid PID so that signal handling will
> work correctly...
>
> This could be solved by making PIDs independent of processes, but that's
> really not very nice.
Oh, interesting. Note that we already allocate from the pid space for
other things, so it's entirely doable. Another choice would be to simply
disallow the thread group leader from exec'ing. Linux threads traditionally
try to remain as close to process behaviour as possible, and allowing an
individual thread to exec is very useful, so I'm still in favour of
allocating a new tgid.
-ben
--
"A man with a bass just walked in,
and he's putting it down
on the floor."
Benjamin LaHaise <[email protected]> wrote:
> On Thu, Feb 28, 2002 at 02:52:36PM +0000, David Howells wrote:
> >
> > If the master thread of a thread group (PID==TGID) performs an execve()
> > then it is possible to end up with two or more thread groups with the same
> > TGID.
>
> How about: (3) make execve allocate a new thread group id?
It seems that the TGID must also be a valid PID so that signal handling will
work correctly...
This could be solved by making PIDs independent of processes, but that's
really not very nice.
David
On Thu, 28 Feb 2002, David Howells wrote:
>
> If the master thread of a thread group (PID==TGID) performs an execve() then
> it is possible to end up with two or more thread groups with the same TGID.
No.
When they have the same TGID, they _are_ the same thread group.
There is absolutely _zero_ correlation between thread ID and MM. Never has
been, never will be. An execve() is a total non-event from a TGID
perspective.
If you expect POSIX behaviour, you do not do execve's from the master.
It's that simple.
(Note that if you want POSIX behaviour, you're catching execve() anyway,
so the matter is moot).
Linus
> When they have the same TGID, they _are_ the same thread group.
Apparently not so... Doing an execve() on the master thread breaks this
internally by removing the master thread from the thread-group ring without
resetting the TGID on all subsidiary threads. So if one of the subsidiary
threads thinks its TGID is x before an execve on the master, then if it tries
to use that TGID after the execve on the master, it will affect the master's
new thread group and not itself.
> There is absolutely _zero_ correlation between thread ID and MM. Never has
> been, never will be. An execve() is a total non-event from a TGID
> perspective.
Except that the execve() _can_ (a) change the TGID and (b) result in two
effective thread groups of the same TGID as far as the kernel is concerned.
> If you expect POSIX behaviour, you do not do execve's from the master.
> It's that simple.
My concern is not so much dealing with POSIX behaviour as coping with
behaviour that can happen without getting snookered.
David
On Thu, 28 Feb 2002, David Howells wrote:
>
> Except that the execve() _can_ (a) change the TGID and (b) result in two
> effective thread groups of the same TGID as far as the kernel is concerned.
You're right. I did that for security reasons - making sure that nobody
can execve a suid application and then keep on sending signals to it under
the auspices of thread groups. I'd forgotten about that thing.
Maybe killing the other threads on execve _is_ the right thing after all,
if that also gives us POSIX behaviour.
Who actually maintains the pthread library? I don't think they use
CLONE_THREAD at all yet, right?
Linus
--On Thursday, February 28, 2002 16:20:21 -0800 Linus Torvalds
<[email protected]> wrote:
> Maybe killing the other threads on execve _is_ the right thing after all,
> if that also gives us POSIX behaviour.
>
> Who actually maintains the pthread library? I don't think they use
> CLONE_THREAD at all yet, right?
As far as I know, Linuxthreads is maintained by Ullrich Drepper, and it
doesn't use CLONE_THREAD. IBM's NGPT pthread library does use
CLONE_THREAD, and I'm the one who's handling kernel issues related to it.
I could code up something that does a 'kill the thread group on execve' if
you'd like me to.
Dave McCracken
======================================================================
Dave McCracken IBM Linux Base Kernel Team 1-512-838-3059
[email protected] T/L 678-3059
Hi Linus,
I've attached a patch to kill all subsidiary threads in a thread group when
the main thread exits. I've made it against 2.5.6-pre1 since -pre2 fails to
compile in the IDE code somewhere.
I've tested it, and it appears to work.
Note that it sends the subsidiary threads SIGKILL with SI_DETHREAD.
Note also that subsidiary threads doing an execve() just leave the thread
group (rather than forcing the master thread to do an execve() which would be
more POSIX like).
David
diff -uNr linux-2.5.6-pre1/fs/exec.c linux-execve-256p1/fs/exec.c
--- linux-2.5.6-pre1/fs/exec.c Wed Feb 27 08:26:55 2002
+++ linux-execve-256p1/fs/exec.c Fri Mar 1 15:37:36 2002
@@ -513,23 +513,49 @@
/*
* 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.
+ * - if a master thread (PID==TGID) is doing this, then all subsidiary threads
+ * will be killed (otherwise there will end up being two independent thread
+ * groups with the same TGID).
+ * - if a subsidary thread is doing this, then it just leaves the thread group
*/
-static inline void de_thread(struct task_struct *tsk)
+static void de_thread(struct task_struct *tsk)
{
- if (!list_empty(&tsk->thread_group)) {
- write_lock_irq(&tasklist_lock);
+ struct task_struct *sub;
+ struct list_head *head, *ptr;
+ struct siginfo info;
+ int pause;
+
+ write_lock_irq(&tasklist_lock);
+
+ if (tsk->tgid != tsk->pid) {
+ /* subsidiary thread - just escapes the group */
+ list_del_init(&tsk->thread_group);
+ tsk->tgid = tsk->pid;
+ pause = 0;
+ }
+ else {
+ /* master thread - kill all subsidiary threads */
+ info.si_signo = SIGKILL;
+ info.si_errno = 0;
+ info.si_code = SI_DETHREAD;
+ info.si_pid = current->pid;
+ info.si_uid = current->uid;
+
+ head = tsk->thread_group.next;
list_del_init(&tsk->thread_group);
- write_unlock_irq(&tasklist_lock);
+
+ list_for_each(ptr,head) {
+ sub = list_entry(ptr,struct task_struct,thread_group);
+ send_sig_info(SIGKILL,&info,sub);
+ }
+
+ pause = 1;
}
- /* Minor oddity: this might stay the same. */
- tsk->tgid = tsk->pid;
+ write_unlock_irq(&tasklist_lock);
+
+ /* give the subsidiary threads a chance to clean themselves up */
+ if (pause) yield();
}
int flush_old_exec(struct linux_binprm * bprm)
@@ -570,7 +596,8 @@
flush_thread();
- de_thread(current);
+ if (!list_empty(¤t->thread_group))
+ de_thread(current);
if (bprm->e_uid != current->euid || bprm->e_gid != current->egid ||
permission(bprm->file->f_dentry->d_inode,MAY_READ))
diff -uNr linux-2.5.6-pre1/include/asm-i386/siginfo.h linux-execve-256p1/include/asm-i386/siginfo.h
--- linux-2.5.6-pre1/include/asm-i386/siginfo.h Wed Feb 27 08:27:01 2002
+++ linux-execve-256p1/include/asm-i386/siginfo.h Fri Mar 1 15:25:47 2002
@@ -108,6 +108,7 @@
#define SI_ASYNCIO -4 /* sent by AIO completion */
#define SI_SIGIO -5 /* sent by queued SIGIO */
#define SI_TKILL -6 /* sent by tkill system call */
+#define SI_DETHREAD -7 /* sent by execve() killing subsidiary threads */
#define SI_FROMUSER(siptr) ((siptr)->si_code <= 0)
#define SI_FROMKERNEL(siptr) ((siptr)->si_code > 0)
David Howells wrote:
> Hi Linus,
>
> I've attached a patch to kill all subsidiary threads in a thread group when
> the main thread exits. I've made it against 2.5.6-pre1 since -pre2 fails to
> compile in the IDE code somewhere.
Just please enable support for IDE chipset tuning and it should at least
compile.
--On Friday, March 01, 2002 16:38:11 +0000 David Howells
<[email protected]> wrote:
> I've attached a patch to kill all subsidiary threads in a thread group
> when the main thread exits. I've made it against 2.5.6-pre1 since -pre2
> fails to compile in the IDE code somewhere.
You beat me to it. Nice work.
Dave McCracken
======================================================================
Dave McCracken IBM Linux Base Kernel Team 1-512-838-3059
[email protected] T/L 678-3059