From owner-freebsd-stable Sun Oct 29 15:39:48 2000 Delivered-To: freebsd-stable@freebsd.org Received: from math.psu.edu (leibniz.math.psu.edu [146.186.130.2]) by hub.freebsd.org (Postfix) with ESMTP id A20CA37B661 for ; Sun, 29 Oct 2000 15:39:46 -0800 (PST) Received: from weyl.math.psu.edu (weyl.math.psu.edu [146.186.130.226]) by math.psu.edu (8.9.3/8.9.3) with ESMTP id SAA04429; Sun, 29 Oct 2000 18:39:45 -0500 (EST) Received: from localhost (viro@localhost) by weyl.math.psu.edu (8.9.3/8.9.3) with ESMTP id SAA28890; Sun, 29 Oct 2000 18:39:44 -0500 (EST) X-Authentication-Warning: weyl.math.psu.edu: viro owned process doing -bs Date: Sun, 29 Oct 2000 18:39:44 -0500 (EST) From: Alexander Viro To: Matt Dillon Cc: Jordan Hubbard , freebsd-stable@freebsd.org Subject: Re: Proposed patch related to August discussion on file descriptor races and kern/11629 In-Reply-To: <200010292300.e9TN0N071594@earth.backplane.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-stable@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Sun, 29 Oct 2000, Matt Dillon wrote: > Ah, ok, I see that hole now. It can occur if closef() blocks (which it > can). This also reveals another hole where two dup2() calls with the same > (in-use) destination can cause struct file leakage. > > I think the problem is solved by removing the code in dup2() that > deals with the case where the destination already exists and moving > it into finishdup(). Then using your description above in finishdup(). I'ld rather call it do_dup() then - considering what remains in dup2()... BTW, here's one more race: you are calling fdalloc() after the check for old being opened. close(2) in the wrong moment and... We solved it by grabbing the old file before equivalent of fdalloc(). Aha. And one more: fdalloc()/fdalloc() - think what happens if second fdalloc() happens while we are sleeping in malloc() from the first one. Not likely to happen (to get an overrun we need _two_ expansions during the sleep on malloc()), but I would definitely check whether we still have the same ->fd_nfiles and free()+retry if it had been changed. yup, that's what SCT had done there. Basically, at some point I said "screw it" and started to massage the stuff into SMP-safe form, explicitly protecting the manipulations with the descriptor table by rw-spinlock. Much easier to check correctness that way - just make sure that table is accessed only under its lock and that we grab all references before releasing said lock. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-stable" in the body of the message