should vn_fullpath1() ever return a path with "." in it?

[Konstantin Belousov]

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);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20130228/4a75349e/attachment.sig>