Date: Fri, 1 Mar 2013 09:57:26 -0500 (EST) From: Rick Macklem <rmacklem@uoguelph.ca> To: Sergey Kandaurov <pluknet@freebsd.org> Cc: FreeBSD Filesystems <freebsd-fs@freebsd.org> Subject: Re: should vn_fullpath1() ever return a path with "." in it? Message-ID: <298688524.3444408.1362149846756.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <CAE-mSOJYATENbP48_7QJQEm8Y2X%2B22cS6nD77OGM7jYvYMBHmQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Sergey Kandaurov wrote:
> On 1 March 2013 04:58, Rick Macklem <rmacklem@uoguelph.ca> wrote:
> > Kostik Belousov wrote:
> >> On Wed, Feb 27, 2013 at 09:59:22PM -0500, Rick Macklem wrote:
> >> > Hi,
> >> >
> >> > Sergey Kandaurov reported a problem where getcwd() returns a
> >> > path with "/./" imbedded in it for an NFSv4 mount. This is
> >> > caused by a mount point crossing on the server when at the
> >> > server's root because vn_fullpath1() uses VV_ROOT to spot
> >> > mount point crossings.
> >> >
> >> > The current workaround is to use the sysctls:
> >> > debug.disablegetcwd=1
> >> > debug.disablefullpath=1
> >> >
> >> > However, it would be nice to fix this when vn_fullpath1()
> >> > is being used.
> >> >
> >> > A simple fix is to have vn_fullpath1() fail when it finds
> >> > "." as a directory match in the path. When vn_fullpath1()
> >> > fails, the syscalls fail and that allows the libc algorithm
> >> > to be used (which works for this case because it doesn't
> >> > depend on VV_ROOT being set, etc).
> >> >
> >> > So, I am wondering if a patch (I have attached one) that
> >> > makes vn_fullpath1() fail when it matches "." will break
> >> > anything else? (I don't think so, since the code checks
> >> > for VV_ROOT in the loop above the check for a match of
> >> > ".", but I am not sure?)
> >> >
> >> > Thanks for any input w.r.t. this, rick
> >>
> >> > --- kern/vfs_cache.c.sav 2013-02-27 20:44:42.000000000 -0500
> >> > +++ kern/vfs_cache.c 2013-02-27 21:10:39.000000000 -0500
> >> > @@ -1333,6 +1333,20 @@ vn_fullpath1(struct thread *td, struct v
> >> > startvp, NULL, 0, 0);
> >> > break;
> >> > }
> >> > + if (buf[buflen] == '.' && (buf[buflen + 1] == '\0' ||
> >> > + buf[buflen + 1] == '/')) {
> >> > + /*
> >> > + * Fail if it matched ".". This should only happen
> >> > + * for NFSv4 mounts that cross server mount points.
> >> > + */
> >> > + CACHE_RUNLOCK();
> >> > + vrele(vp);
> >> > + numfullpathfail1++;
> >> > + error = ENOENT;
> >> > + SDT_PROBE(vfs, namecache, fullpath, return,
> >> > + error, vp, NULL, 0, 0);
> >> > + break;
> >> > + }
> >> > buf[--buflen] = '/';
> >> > slash_prefixed = 1;
> >> > }
> >>
> >> I do not quite understand this. Did the dvp (parent) vnode returned
> >> by
> >> VOP_VPTOCNP() equal to vp (child) vnode in the case of the "." name
> >> ?
> >> It must be, for the correct operation, but also it should cause the
> >> almost
> >> infinite loop in the vn_fullpath1(). The loop is not really
> >> infinite
> >> due
> >> to a limited size of the buffer where the infinite amount of "./"
> >> is
> >> placed.
> >>
> >> Anyway, I think we should do better than this patch, even if it is
> >> legitimate. I think that the better place to check the condition is
> >> the
> >> default implementation of VOP_VPTOCNP(). Am I right that this is
> >> where
> >> it broke for you ?
> >>
> >> diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c
> >> index 00d064e..1dd0185 100644
> >> --- a/sys/kern/vfs_default.c
> >> +++ b/sys/kern/vfs_default.c
> >> @@ -856,8 +856,12 @@ vop_stdvptocnp(struct vop_vptocnp_args *ap)
> >> error = ENOMEM;
> >> goto out;
> >> }
> >> - bcopy(dp->d_name, buf + i, dp->d_namlen);
> >> - error = 0;
> >> + if (dp->d_namlen == 1 && dp->d_name[0] == '.') {
> >> + error = ENOENT;
> >> + } else {
> >> + bcopy(dp->d_name, buf + i, dp->d_namlen);
> >> + error = 0;
> >> + }
> >> goto out;
> >> }
> >> } while (len > 0 || !eofflag);
> >
> > Yes, this patch fixes the problem too. If you think it is safe to
> > do this, I can commit the patch in mid-April. Maybe Sergey can
> > test it?
> >
> > Thanks yet again, rick
> >
>
> Hi Rick
> Sorry but I am no longer able to test NFSv4.
>
No problem. I can reproduce the problem, so I think it's fine w.r.t.
testing to see if it fixes the bug.
Thanks, rick
> --
> wbr,
> pluknet
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?298688524.3444408.1362149846756.JavaMail.root>