From owner-freebsd-fs@FreeBSD.ORG Sat Aug 5 23:59:30 2006 Return-Path: X-Original-To: freebsd-fs@freebsd.org Delivered-To: freebsd-fs@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 16F4A16A4EC for ; Sat, 5 Aug 2006 23:59:30 +0000 (UTC) (envelope-from bde@zeta.org.au) Received: from mailout2.pacific.net.au (mailout2.pacific.net.au [61.8.0.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id A364A43D70 for ; Sat, 5 Aug 2006 23:59:25 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy1.pacific.net.au (mailproxy1.pacific.net.au [61.8.2.162]) by mailout2.pacific.net.au (Postfix) with ESMTP id BE70210992A; Sun, 6 Aug 2006 09:59:24 +1000 (EST) Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailproxy1.pacific.net.au (8.13.4/8.13.4/Debian-3sarge1) with ESMTP id k75NxMuF029348; Sun, 6 Aug 2006 09:59:23 +1000 Date: Sun, 6 Aug 2006 09:59:22 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Michiel Pelt In-Reply-To: Message-ID: <20060806094905.V2709@delplex.bde.org> References: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-fs@freebsd.org Subject: Re: VOP_LOOKUP of .. in msdosfs X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Aug 2006 23:59:30 -0000 On Sat, 5 Aug 2006, Michiel Pelt wrote: > ... > The msdosfs_lookup function in msdosfs_lookup.c has code dealing with the > rootdirectory at the label foundroot:. At line 523 (release 6.1 code) of the > release code is this piece of code: > > pdp = vdp; > if (flags & ISDOTDOT) { > VOP_UNLOCK(pdp, 0, td); > error = deget(pmp, cluster, blkoff, &tdp); > vn_lock(pdp, LK_EXCLUSIVE | LK_RETRY, td); > if (error) > return (error); > *vpp = DETOV(tdp); > } > > This code has been copied from ffs code, but fails for msdosfs. > For starters the introduction of the 'pdp' pointer has no function > whatsoever. > > Consider a lookup for .. in the root. In msdosfs the /. and /.. directories > are represented by one and the same vnode (see deget() in msdosfs_denode.c). > The deget gets and locks the vnode that was unlocked by VOP_UNLOCK. The > vn_lock that follows fails with a panic because the vnode is already locked. It seems to have been correct in 5.2: % pdp = vdp; % if (flags & ISDOTDOT) { % VOP_UNLOCK(pdp, 0, td); % cnp->cn_flags |= PDIRUNLOCK; % error = deget(pmp, cluster, blkoff, &tdp); % if (error) { % vn_lock(pdp, LK_EXCLUSIVE | LK_RETRY, td); % cnp->cn_flags &= ~PDIRUNLOCK; % return (error); % } Apparently, deget() didn't lock, and we only needed to lock in the error case. Or maybe the problem went unnoticed because the error case is even rarer than the ISDOTDOT case. > So why does this not happen in practice? Because the ISDOTDOT case is > handled by vfs_lookup and never reaches VOP_LOOKUP. > > I did get the error because I forgot to set the VV_ROOT flag in the vnode > returned by VFS_ROOT ;-(. I think the whole ISDOTDOT case is unreachable for the root of an msdosfs file system, at least in 5.2, since msdosfs can't be mounted on "/" so ".." at the root is never in the current file system and so it must be handled by vfs_lookup() and VFS_ROOT must be set so that it is handled there. Bruce