problem with execve() and CLONE_SIGHAND

Xavier Leroy (Xavier.Leroy@inria.fr)
Mon, 30 Dec 1996 11:25:33 +0100 (MET)


The 2.0 and 2.1 kernels currently have a problem with execve() when
the calling process is sharing its signal handlers with other
processes, via the CLONE_SIGHAND option to clone(). Namely,
the signal handler table remains shared even after the process that
calls execve() has started to execute the new program.

This is clearly bad, since we have processes executing different texts
that share the same signal handlers; thus, a signal handler installed
by one of the processes may not make sense in the others.

The patch below fixes the problem by making a private copy of the
signal handler table at execve() time in case it's shared. It also
turns the "oom" in exec_mmap into a regular ENOMEM error.

The patch is against 2.0.27 and has been tested on that version of the
kernel, but it seems to apply to 2.1.18 also (though I haven't
tested).

This problem has some impact on LinuxThreads, the POSIX 1003.1c thread
library for Linux that I'm developing. Therefore, I'd love if it could
be fixed in 2.0.28 and not only in the 2.1 branch.

Best regards,

- Xavier Leroy

diff -u -r linux-2.0.27/fs/binfmt_aout.c linux-2.0.27patched/fs/binfmt_aout.c
--- linux-2.0.27/fs/binfmt_aout.c Sat Aug 17 20:19:28 1996
+++ linux-2.0.27patched/fs/binfmt_aout.c Mon Dec 30 10:41:35 1996
@@ -298,8 +298,10 @@
if (ex.a_data + ex.a_bss > rlim)
return -ENOMEM;

- /* OK, This is the point of no return */
- flush_old_exec(bprm);
+ /* OK, This is the point of no return
+ (except out-of-memory conditions) */
+ if (flush_old_exec(bprm) == -1)
+ return -ENOMEM;

current->mm->end_code = ex.a_text +
(current->mm->start_code = N_TXTADDR(ex));
diff -u -r linux-2.0.27/fs/binfmt_elf.c linux-2.0.27patched/fs/binfmt_elf.c
--- linux-2.0.27/fs/binfmt_elf.c Sun Sep 8 18:50:21 1996
+++ linux-2.0.27patched/fs/binfmt_elf.c Mon Dec 30 10:41:44 1996
@@ -549,8 +549,10 @@
}
}

- /* OK, This is the point of no return */
- flush_old_exec(bprm);
+ /* OK, This is the point of no return
+ (except out-of-memory conditions) */
+ if (flush_old_exec(bprm) == -1)
+ return -ENOMEM;

current->mm->end_data = 0;
current->mm->end_code = 0;
diff -u -r linux-2.0.27/fs/exec.c linux-2.0.27patched/fs/exec.c
--- linux-2.0.27/fs/exec.c Sat Nov 30 11:21:18 1996
+++ linux-2.0.27patched/fs/exec.c Mon Dec 30 10:43:11 1996
@@ -361,20 +361,16 @@
return result;
}

-static void exec_mmap(void)
+static int exec_mmap(void)
{
/*
* The clear_page_tables done later on exec does the right thing
* to the page directory when shared, except for graceful abort
- * (the oom is wrong there, too, IMHO)
*/
if (current->mm->count > 1) {
struct mm_struct *mm = kmalloc(sizeof(*mm), GFP_KERNEL);
- if (!mm) {
- /* this is wrong, I think. */
- oom(current);
- return;
- }
+ if (!mm)
+ return -1;
*mm = *current->mm;
mm->def_flags = 0; /* should future lockings be kept? */
mm->count = 1;
@@ -385,10 +381,35 @@
current->mm->count--;
current->mm = mm;
new_page_tables(current);
- return;
+ return 0;
}
exit_mmap(current->mm);
clear_page_tables(current);
+ return 0;
+}
+
+/*
+ * This function makes a copy of the signal table of the current
+ * process if it's shared with other processes,
+ * so that flush_old_signal will not modify other processes's signal tables,
+ * and the exec()ed program can set its own signal handlers without
+ * disturbing the other processes.
+ */
+
+static inline int unshare_signals(void)
+{
+ struct signal_struct *sig;
+
+ if (current->sig->count > 1) {
+ sig = kmalloc(sizeof(*sig), GFP_KERNEL);
+ if (!sig)
+ return -1;
+ sig->count = 1;
+ memcpy(sig->action, current->sig->action, sizeof(sig->action));
+ current->sig->count--;
+ current->sig = sig;
+ }
+ return 0;
}

/*
@@ -431,7 +452,7 @@
}
}

-void flush_old_exec(struct linux_binprm * bprm)
+int flush_old_exec(struct linux_binprm * bprm)
{
int i;
int ch;
@@ -449,8 +470,13 @@
}
current->comm[i] = '\0';

+ /* Make sure we have a private signal table. */
+ if (unshare_signals() == -1)
+ return -1;
+
/* Release all of the old mmap stuff. */
- exec_mmap();
+ if (exec_mmap() == -1)
+ return -1;

flush_thread();

@@ -460,6 +486,8 @@

flush_old_signals(current->sig);
flush_old_files(current->files);
+
+ return 0;
}

/*
diff -u -r linux-2.0.27/include/linux/binfmts.h linux-2.0.27patched/include/linux/binfmts.h
--- linux-2.0.27/include/linux/binfmts.h Wed May 8 17:28:01 1996
+++ linux-2.0.27patched/include/linux/binfmts.h Mon Dec 30 10:38:48 1996
@@ -54,7 +54,7 @@
extern int prepare_binprm(struct linux_binprm *);
extern void remove_arg_zero(struct linux_binprm *);
extern int search_binary_handler(struct linux_binprm *,struct pt_regs *);
-extern void flush_old_exec(struct linux_binprm * bprm);
+extern int flush_old_exec(struct linux_binprm * bprm);
extern unsigned long setup_arg_pages(unsigned long p, struct linux_binprm * bprm);
extern unsigned long copy_strings(int argc,char ** argv,unsigned long *page,
unsigned long p, int from_kmem);