VOP_LOOKUP of .. in msdosfs
Bruce Evans
bde at zeta.org.au
Sat Aug 5 23:59:30 UTC 2006
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
More information about the freebsd-fs
mailing list