[PATCH] fix do_fork() return value

Noticed by Julie DeWandel <jdewand@redhat.com>.

do_fork() needs to return the pid (or error), not the pointer to the
resulting process structure.  The process structure may not even be
valid any more, since do_fork() has already woken the process up (and as
a result it might already have done its thing and gone away).

Besides, doing it this way cleans up the users, which all really just
wanted the pid or error number _anyway_.

This fixes the x86 users, other architectures need to be fixed up as
well.

arch/i386/kernel/process.c

@@ -212,7 +212,6 @@ __asm__(".align 4\n"
  */
 int kernel_thread(int (*fn)(void *), void * arg, unsigned long flags)
 {
-	struct task_struct *p;
 	struct pt_regs regs;
 
 	memset(&regs, 0, sizeof(regs));
@@ -228,8 +227,7 @@ int kernel_thread(int (*fn)(void *), void * arg, unsigned long flags)
 	regs.eflags = 0x286;
 
 	/* Ok, create the new process.. */
-	p = do_fork(flags | CLONE_VM | CLONE_UNTRACED, 0, &regs, 0, NULL, NULL);
-	return IS_ERR(p) ? PTR_ERR(p) : p->pid;
+	return do_fork(flags | CLONE_VM | CLONE_UNTRACED, 0, &regs, 0, NULL, NULL);
 }
 
 /*
@@ -518,15 +516,11 @@ struct task_struct * __switch_to(struct task_struct *prev_p, struct task_struct
 
 asmlinkage int sys_fork(struct pt_regs regs)
 {
-	struct task_struct *p;
-
-	p = do_fork(SIGCHLD, regs.esp, &regs, 0, NULL, NULL);
-	return IS_ERR(p) ? PTR_ERR(p) : p->pid;
+	return do_fork(SIGCHLD, regs.esp, &regs, 0, NULL, NULL);
 }
 
 asmlinkage int sys_clone(struct pt_regs regs)
 {
-	struct task_struct *p;
 	unsigned long clone_flags;
 	unsigned long newsp;
 	int __user *parent_tidptr, *child_tidptr;
@@ -537,8 +531,7 @@ asmlinkage int sys_clone(struct pt_regs regs)
 	child_tidptr = (int __user *)regs.edi;
 	if (!newsp)
 		newsp = regs.esp;
-	p = do_fork(clone_flags & ~CLONE_IDLETASK, newsp, &regs, 0, parent_tidptr, child_tidptr);
-	return IS_ERR(p) ? PTR_ERR(p) : p->pid;
+	return do_fork(clone_flags & ~CLONE_IDLETASK, newsp, &regs, 0, parent_tidptr, child_tidptr);
 }
 
 /*
@@ -553,10 +546,7 @@ asmlinkage int sys_clone(struct pt_regs regs)
  */
 asmlinkage int sys_vfork(struct pt_regs regs)
 {
-	struct task_struct *p;
-
-	p = do_fork(CLONE_VFORK | CLONE_VM | SIGCHLD, regs.esp, &regs, 0, NULL, NULL);
-	return IS_ERR(p) ? PTR_ERR(p) : p->pid;
+	return do_fork(CLONE_VFORK | CLONE_VM | SIGCHLD, regs.esp, &regs, 0, NULL, NULL);
 }
 
 /*

arch/i386/kernel/smpboot.c

@@ -493,7 +493,7 @@ static struct task_struct * __init fork_by_hand(void)
 	 * don't care about the eip and regs settings since
 	 * we'll never reschedule the forked task.
 	 */
-	return do_fork(CLONE_VM|CLONE_IDLETASK, 0, &regs, 0, NULL, NULL);
+	return copy_process(CLONE_VM|CLONE_IDLETASK, 0, &regs, 0, NULL, NULL);
 }
 
 #ifdef CONFIG_NUMA
@@ -793,6 +793,7 @@ static int __init do_boot_cpu(int apicid)
 	idle = fork_by_hand();
 	if (IS_ERR(idle))
 		panic("failed fork for CPU %d", cpu);
+	wake_up_forked_process(idle);
 
 	/*
 	 * We remove it from the pidhash and the runqueue

arch/i386/mach-voyager/voyager_smp.c

@@ -531,7 +531,7 @@ fork_by_hand(void)
 	struct pt_regs regs;
 	/* don't care about the eip and regs settings since we'll
 	 * never reschedule the forked task. */
-	return do_fork(CLONE_VM|CLONE_IDLETASK, 0, &regs, 0, NULL, NULL);
+	return copy_process(CLONE_VM|CLONE_IDLETASK, 0, &regs, 0, NULL, NULL);
 }
 
 

include/linux/sched.h

@@ -637,7 +637,8 @@ extern int allow_signal(int);
 extern task_t *child_reaper;
 
 extern int do_execve(char *, char __user * __user *, char __user * __user *, struct pt_regs *);
-extern struct task_struct *do_fork(unsigned long, unsigned long, struct pt_regs *, unsigned long, int __user *, int __user *);
+extern long do_fork(unsigned long, unsigned long, struct pt_regs *, unsigned long, int __user *, int __user *);
+extern struct task_struct * copy_process(unsigned long, unsigned long, struct pt_regs *, unsigned long, int __user *, int __user *);
 
 #ifdef CONFIG_SMP
 extern void wait_task_inactive(task_t * p);

kernel/fork.c

@@ -752,7 +752,7 @@ asmlinkage long sys_set_tid_address(int __user *tidptr)
  * parts of the process environment (as per the clone
  * flags). The actual kick-off is left to the caller.
  */
-static struct task_struct *copy_process(unsigned long clone_flags,
+struct task_struct *copy_process(unsigned long clone_flags,
 				 unsigned long stack_start,
 				 struct pt_regs *regs,
 				 unsigned long stack_size,
@@ -1067,7 +1067,7 @@ static inline int fork_traceflag (unsigned clone_flags)
  * It copies the process, and if successful kick-starts
  * it and waits for it to finish using the VM if required.
  */
-struct task_struct *do_fork(unsigned long clone_flags,
+long do_fork(unsigned long clone_flags,
 	      unsigned long stack_start,
 	      struct pt_regs *regs,
 	      unsigned long stack_size,
@@ -1076,6 +1076,7 @@ struct task_struct *do_fork(unsigned long clone_flags,
 {
 	struct task_struct *p;
 	int trace = 0;
+	long pid;
 
 	if (unlikely(current->ptrace)) {
 		trace = fork_traceflag (clone_flags);
@@ -1084,6 +1085,12 @@ struct task_struct *do_fork(unsigned long clone_flags,
 	}
 
 	p = copy_process(clone_flags, stack_start, regs, stack_size, parent_tidptr, child_tidptr);
+	/*
+	 * Do this prior waking up the new thread - the thread pointer
+	 * might get invalid after that point, if the thread exits quickly.
+	 */
+	pid = IS_ERR(p) ? PTR_ERR(p) : p->pid;
+
 	if (!IS_ERR(p)) {
 		struct completion vfork;
 
@@ -1104,7 +1111,7 @@ struct task_struct *do_fork(unsigned long clone_flags,
 		++total_forks;
 
 		if (unlikely (trace)) {
-			current->ptrace_message = (unsigned long) p->pid;
+			current->ptrace_message = pid;
 			ptrace_notify ((trace << 8) | SIGTRAP);
 		}
 
@@ -1119,7 +1126,7 @@ struct task_struct *do_fork(unsigned long clone_flags,
 			 */
 			set_need_resched();
 	}
-	return p;
+	return pid;
 }
 
 /* SLAB cache for signal_struct structures (tsk->signal) */