Date: Wed, 27 Dec 2006 14:10:17 GMT From: Bruce Evans <bde@zeta.org.au> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/92785: Using exported filesystem on OS/2 NFS client causes filesystem freeze Message-ID: <200612271410.kBREAHXo044451@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/92785; it has been noted by GNATS.
From: Bruce Evans <bde@zeta.org.au>
To: Kostik Belousov <kostikbel@gmail.com>
Cc: Ulrich Spoerlein <uspoerlein@gmail.com>, bug-followup@FreeBSD.org
Subject: Re: kern/92785: Using exported filesystem on OS/2 NFS client causes
filesystem freeze
Date: Thu, 28 Dec 2006 01:02:20 +1100 (EST)
On Wed, 27 Dec 2006, Kostik Belousov wrote:
> On Wed, Dec 27, 2006 at 11:43:36AM +0100, Ulrich Spoerlein wrote:
>> While the patch for UFS seems to quiesce the lock-leak in nfsd I now
>> exported a FAT32 filesystem. All Linux clients can access the export
>> just fine, mounting it from OS/2 will result in the following panic,
>> after issuing a 'dir' in the mounted drive. The dir-command will print
>> the first line (the "."-DIR) and when trying to look up ".." the nfs
>> server paniced.
>>
>> panic: lockmgr: locking against myself
>
> I did not read the code thoroughly, and I prefer not to touch
> msdosfs. But, anyway, try this:
Me too.
> Index: sys/fs/msdosfs/msdosfs_lookup.c
> ===================================================================
> RCS file: /usr/local/arch/ncvs/src/sys/fs/msdosfs/msdosfs_lookup.c,v
> retrieving revision 1.47
> diff -u -r1.47 msdosfs_lookup.c
> --- sys/fs/msdosfs/msdosfs_lookup.c 22 Jan 2006 21:09:38 -0000 1.47
> +++ sys/fs/msdosfs/msdosfs_lookup.c 27 Dec 2006 10:59:17 -0000
> @@ -520,16 +520,16 @@
> * that point backwards in the directory structure.
> */
> pdp = vdp;
> - if (flags & ISDOTDOT) {
> + if (dp->de_StartCluster == scn && isadir) {
> + VREF(vdp); /* we want ourself, ie "." */
> + *vpp = vdp;
> + } else 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);
> - } else if (dp->de_StartCluster == scn && isadir) {
> - VREF(vdp); /* we want ourself, ie "." */
> - *vpp = vdp;
> } else {
> if ((error = deget(pmp, cluster, blkoff, &tdp)) != 0)
> return (error);
>
Funny, this looks like a bug that was first reported for msdosfs. Here
is some old mail about it.
From bde@zeta.org.au Sun Aug 6 09:59:24 2006 +1000
Date: Sun, 6 Aug 2006 09:59:22 +1000 (EST)
From: Bruce Evans <bde@zeta.org.au>
X-X-Sender: bde@delplex.bde.org
To: Michiel Pelt <pelt22@gmail.com>
cc: freebsd-fs@freebsd.org
Subject: Re: VOP_LOOKUP of .. in msdosfs
In-Reply-To: <cee5e2ee0608050817u67f0c08cw73ec68f54400375b@mail.gmail.com>
Message-ID: <20060806094905.V2709@delplex.bde.org>
References: <cee5e2ee0608050817u67f0c08cw73ec68f54400375b@mail.gmail.com>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
Status: O
X-Status:
X-Keywords:
X-UID: 360
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
%%%
I don't understand the changes near here since 5.2 (even in ffs). If
you do, then please touch a lot of file systems that have the bug :-).
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200612271410.kBREAHXo044451>