Re: linux-2.4.0-test7

From: Christian Ehrhardt (ehrhardt@mathematik.uni-ulm.de)
Date: Thu Aug 24 2000 - 09:55:38 EST


On Wed, Aug 23, 2000 at 07:55:28PM -0700, Linus Torvalds wrote:
> test7:
> - pre7
> - get rid of unnecessary kernel lock in fork()

I wonder if this was a good idea. Look at these code pieces from
do_fork which are now executed without any lock held:

| retval = -EAGAIN;
| if (atomic_read(&p->user->processes) >= p->rlim[RLIMIT_NPROC].rlim_cur)
| goto bad_fork_free;
| atomic_inc(&p->user->__count);
| atomic_inc(&p->user->processes);

This looks like an SMP race, that could allow a user to get more
processes than the number allowed by rlim. Mostly harmless, probably.

| /*
| * Counter increases are protected by
| * the kernel lock so nr_threads can't
| * increase under us (but it may decrease).
| */
| if (nr_threads >= max_threads)
| goto bad_fork_cleanup_count;

If this is really ok the comment should be removed and nr_threads
should better be atomic_t.

| p->pid = get_pid(clone_flags);

The pid returned by get_pid is not yet reserved, i.e. it won't be found
when get_pid searches the task list for used pids. Another call to get_pid
could theoretically return the same pid before the pid gets used here.
This is highly improbable, but if it actually happend it would probably
cause a big mess.

Are these races in fact impossible and I overlooked something or do
we take the risk because they either can't do to much harm or (like in
the last case) are too unlikely to happen?

    best regards Christian

-- 
THAT'S ALL FOLKS!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Aug 31 2000 - 21:00:13 EST