linux-kernel - [PATCH 6/8] vfork: do not setup child->vfork

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110727163345.GF23793@redhat.com>
Date:	Wed, 27 Jul 2011 18:33:45 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Roland McGrath <roland@...k.frob.com>,
	Tejun Heo <tj@...nel.org>
Cc:	Denys Vlasenko <dvlasenk@...hat.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Matt Fleming <matt.fleming@...ux.intel.com>,
	linux-kernel@...r.kernel.org
Subject: [PATCH 6/8] vfork: do not setup child->vfork_done beforehand

do_fork() allocates/initializes "struct completion" on stack and
sets p->vfork_done in advance, before the task becomes runnable.
This way clone_vfork_finish(p) can always trust ->vfork_done.

But, to make this restartable, we need to re-assign ->vfork_done
and wait in sys_restart_syscall(), and we must not do this if the
child has already passed complete_vfork_done().

With this patch we allocate/initialize and set child->vfork_done in
clone_vfork_finish() when we are going to actually wait. To prevent
the race with the child we use the fake VFORK_DONE_NOP marker set by
do_fork().

complete_vfork_done() does nothing if it sees VFORK_DONE_NOP but sets
->vfork_done = NULL.

This way clone_vfork_finish() can do xchg() and if it returns !NULL
we can be sure the child didn't exit yet, we can safely wait.

IOW, the logic is:

	- do_fork(CLONE_VFORK)

		child->vfork_done = VFORK_DONE_NOP; // != NULL

	- complete_vfork_done() 	// child

		vfork = xchg(tsk->vfork_done, NULL);

	- clone_vfork_finish()		// parent

		struct completion vfork_done;
		if (xchg(&child->vfork_done, &vfork_done) != NULL)
			 wait_for_completion(&vfork_done);

Signed-off-by: Oleg Nesterov <oleg@...hat.com>
---

 kernel/fork.c |   31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

--- 3.1/kernel/fork.c~6_no_vfork_done	2011-07-26 20:28:24.000000000 +0200
+++ 3.1/kernel/fork.c	2011-07-26 21:13:56.000000000 +0200
@@ -1443,28 +1443,40 @@ struct task_struct * __cpuinit fork_idle
 	return task;
 }
 
+#define VFORK_DONE_NOP		((struct completion *) 1)
+#define vfork_done_valid(vf)	((unsigned long)(vf) > 1)
+
 static void complete_vfork_done(struct task_struct *tsk)
 {
 	struct completion *vfork_done = xchg(&tsk->vfork_done, NULL);
 
-	if (vfork_done)
+	if (vfork_done_valid(vfork_done))
 		complete(vfork_done);
 }
 
-static long clone_vfork_finish(struct task_struct *child,
-				struct completion *vfork_done, long pid)
+static long clone_vfork_finish(struct task_struct *child, long pid)
 {
-	int killed = wait_for_completion_killable(vfork_done);
+	struct completion vfork_done;
+	int killed;
+
+	init_completion(&vfork_done);
 
+	/* complete_vfork_done() was already called? */
+	if (xchg(&child->vfork_done, &vfork_done) == NULL)
+		goto done;
+
+	killed = wait_for_completion_killable(&vfork_done);
 	if (killed) {
-		struct completion *steal = xchg(&child->vfork_done, NULL);
+		struct completion *steal = xchg(&child->vfork_done,
+							VFORK_DONE_NOP);
 		/* if we race with complete_vfork_done() we have to wait */
 		if (unlikely(!steal))
-			wait_for_completion(vfork_done);
+			wait_for_completion(&vfork_done);
 
 		return -EINTR;
 	}
 
+done:
 	ptrace_event(PTRACE_EVENT_VFORK_DONE, pid);
 	return pid;
 }
@@ -1526,8 +1538,6 @@ long do_fork(unsigned long clone_flags,
 	 * might get invalid after that point, if the thread exits quickly.
 	 */
 	if (!IS_ERR(p)) {
-		struct completion vfork;
-
 		trace_sched_process_fork(current, p);
 
 		nr = task_pid_vnr(p);
@@ -1536,9 +1546,8 @@ long do_fork(unsigned long clone_flags,
 			put_user(nr, parent_tidptr);
 
 		if (clone_flags & CLONE_VFORK) {
+			p->vfork_done = VFORK_DONE_NOP;
 			get_task_struct(p);
-			p->vfork_done = &vfork;
-			init_completion(&vfork);
 		}
 
 		audit_finish_fork(p);
@@ -1558,7 +1567,7 @@ long do_fork(unsigned long clone_flags,
 			ptrace_event(trace, nr);
 
 		if (clone_flags & CLONE_VFORK) {
-			nr = clone_vfork_finish(p, &vfork, nr);
+			nr = clone_vfork_finish(p, nr);
 			put_task_struct(p);
 		}
 	} else {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/