Date: Sun, 29 Oct 2000 17:22:25 -0500 (EST) From: Alexander Viro <viro@math.psu.edu> To: Matt Dillon <dillon@earth.backplane.com> Cc: Jordan Hubbard <jkh@winston.osd.bsdi.com>, freebsd-stable@freebsd.org Subject: Re: Proposed patch related to August discussion on file descriptor races and kern/11629 Message-ID: <Pine.GSO.4.21.0010291702140.27484-100000@weyl.math.psu.edu> In-Reply-To: <200010292152.e9TLqt371320@earth.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 29 Oct 2000, Matt Dillon wrote: > I would appreciate a review for correctness. Unfortunately I have only > one test box at the moment, and very little time available so this patch > is directly for 4.2, not -current. > > http://apollo.backplane.com/FreeBSD4/ Matt, one obvious comment (I promise to look through all that stuff, just that right now I've got my hands full with read()/mmap()/truncate() races in our -current, and it's not a pretty sight). In dup2() you have a window when new becomes unused even if it was used. I.e. you can get /* Both 0 and 1 are opened */ Thread A: Thread B: dup2(0,1); fd = open(...); with both threads succeeding and fd being 1. It _is_ a correct use (target descriptor is opened, so it's not a case of inherently racey userland) and result is clearly wrong: fd suddenly becomes an alias of stdin. Proposed fix: start with fhold(oldf = fdp->fd_ofiles[old]); then exchange oldf with fdp->fd_ofiles[new] + set flags, _then_ do if (oldf) closef(oldf); - obviously correct and has all manipulations of the table well-contained. Actually I would (OK, did) put the first part (prior to closef()) under the spinlock - the descriptor layer was the first part of SMP threading in our VFS, but that's another story. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-stable" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.GSO.4.21.0010291702140.27484-100000>