Re: [RFC] Race condition?

[Dave Hansen]

This is a multi-part message in MIME format.
--------------040303030603070001070305
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Kasper Dupont wrote:
> Is there a race condition in this piece of code from do_fork in
> linux/kernel/fork.c? I cannot see what prevents two processes
> from calling this at the same time and both successfully fork
> even though the user had only one process left.
> 
>         if (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur
>                       && !capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RESOURCE))
>                 goto bad_fork_free;
> 
>         atomic_inc(&p->user->__count);
>         atomic_inc(&p->user->processes);

I don't see any locking in the call chain leading to this function, so 
I think you're right.  The attached patch fixes this.  It costs an 
extra 2 atomic ops in the failure case, but otherwise just makes the 
processes++ operation earlier.

Patch is against 2.5.27, but applies against 30.
-- 
Dave Hansen
[e-mail]

--------------040303030603070001070305
Content-Type: text/plain;
 name="fork-up-race-2.5.27.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="fork-up-race-2.5.27.patch"

--- linux-2.5.27-clean/kernel/fork.c	Sat Jul 20 12:11:07 2002
+++ linux/kernel/fork.c	Fri Aug  2 09:35:17 2002
@@ -628,13 +628,15 @@
 		goto fork_out;
 
 	retval = -EAGAIN;
-	if (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur) {
-		if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RESOURCE))
+	atomic_inc(&p->user->processes);
+	if (atomic_read(&p->user->processes) > p->rlim[RLIMIT_NPROC].rlim_cur) {
+		if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RESOURCE)) {
+			atomic_dec(&p->user->processes);
 			goto bad_fork_free;
+		}
 	}
 
 	atomic_inc(&p->user->__count);
-	atomic_inc(&p->user->processes);
 
 	/*
 	 * Counter increases are protected by

--------------040303030603070001070305--