From owner-freebsd-current Tue Jul 16 11:15:49 2002 Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 1D6DC37B400 for ; Tue, 16 Jul 2002 11:15:43 -0700 (PDT) Received: from mail.speakeasy.net (mail13.speakeasy.net [216.254.0.213]) by mx1.FreeBSD.org (Postfix) with ESMTP id 959DB43E58 for ; Tue, 16 Jul 2002 11:15:42 -0700 (PDT) (envelope-from jhb@FreeBSD.org) Received: (qmail 28651 invoked from network); 16 Jul 2002 18:15:39 -0000 Received: from unknown (HELO server.baldwin.cx) ([216.27.160.63]) (envelope-sender ) by mail13.speakeasy.net (qmail-ldap-1.03) with DES-CBC3-SHA encrypted SMTP for ; 16 Jul 2002 18:15:39 -0000 Received: from laptop.baldwin.cx (gw1.twc.weather.com [216.133.140.1]) by server.baldwin.cx (8.11.6/8.11.6) with ESMTP id g6GIFY053624; Tue, 16 Jul 2002 14:15:35 -0400 (EDT) (envelope-from jhb@FreeBSD.org) Message-ID: X-Mailer: XFMail 1.5.2 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 In-Reply-To: <20020716062416.GT77219@elvis.mu.org> Date: Tue, 16 Jul 2002 14:15:39 -0400 (EDT) From: John Baldwin To: Alfred Perlstein Subject: Re: race condition in kern_descrip.c and fix Cc: "freebsd-current@freebsd.org" Cc: "freebsd-current@freebsd.org" , David Xu Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 16-Jul-2002 Alfred Perlstein wrote: > * David Xu [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 <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message