On 16-Jul-2002 Alfred Perlstein wrote:
> * David Xu <[EMAIL PROTECTED]> [020715 22:31] wrote:
>> I found a race condition in kern_descrip.c, the race is in function falloc(),
>> it opens a race window at line 1147:
>
> You're right, however I'd appreciate it if you'd look deeper into the
> possiblity of races in this code before committing this patch to make
> sure we don't want to do this another way.
Here's a way w/o goto loops (caution, untested):
Index: kern_descrip.c
===================================================================
RCS file: /usr/cvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.149
diff -u -r1.149 kern_descrip.c
--- kern_descrip.c 29 Jun 2002 01:50:24 -0000 1.149
+++ kern_descrip.c 16 Jul 2002 17:27:00 -0000
@@ -1107,32 +1107,27 @@
register struct file *fp, *fq;
int error, i;
+ /*
+ * Allocate a new file descriptor.
+ */
+ fp = uma_zalloc(file_zone, M_WAITOK | M_ZERO);
sx_xlock(&filelist_lock);
if (nfiles >= maxfiles) {
sx_xunlock(&filelist_lock);
+ uma_zfree(file_zone, fp);
tablefull("file");
return (ENFILE);
}
nfiles++;
- sx_xunlock(&filelist_lock);
+
/*
- * Allocate a new file descriptor.
* If the process has file descriptor zero open, add to the list
* of open files at that point, otherwise put it at the front of
* the list of open files.
*/
- fp = uma_zalloc(file_zone, M_WAITOK);
- bzero(fp, sizeof(*fp));
-
- /*
- * wait until after malloc (which may have blocked) returns before
- * allocating the slot, else a race might have shrunk it if we had
- * allocated it before the malloc.
- */
FILEDESC_LOCK(p->p_fd);
if ((error = fdalloc(td, 0, &i))) {
FILEDESC_UNLOCK(p->p_fd);
- sx_xlock(&filelist_lock);
nfiles--;
sx_xunlock(&filelist_lock);
uma_zfree(file_zone, fp);
@@ -1144,9 +1139,6 @@
fp->f_cred = crhold(td->td_ucred);
fp->f_ops = &badfileops;
fp->f_seqcount = 1;
- FILEDESC_UNLOCK(p->p_fd);
- sx_xlock(&filelist_lock);
- FILEDESC_LOCK(p->p_fd);
if ((fq = p->p_fd->fd_ofiles[0])) {
LIST_INSERT_AFTER(fq, fp, f_list);
} else {
This is more how fork1() works. If you want to optimize the maxfiles
check, then you could grab the lock and do that check before the malloc()
and then continue on with the rest of the function like it is after this
patch. I.e., you would do the maxfiles check twice.
--
John Baldwin <[EMAIL PROTECTED]> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
