[PATCH] Fix d_path for lazy unmounts

Here is a bugfix to d_path. First, when d_path() hits a lazily unmounted mount point, it tries to prepend the name of the lazily unmounted dentry to the path name. It gets this wrong, and also overwrites the slash that separates the name from the following pathname component. This is demonstrated by the attached test case, which prints "getcwd returned d_path-bugsubdir" with the bug. The correct result would be "getcwd returned d_path-bug/subdir". It could be argued that the name of the root dentry should not be part of the result of d_path in the first place. On the other hand, what the unconnected namespace was once reachable as may provide some useful hints to users, and so that seems okay. Second, it isn't always possible to tell from the __d_path result whether the specified root and rootmnt (i.e., the chroot) was reached: lazy unmounts of bind mounts will produce a path that does start with a non-slash so we can tell from that, but other lazy unmounts will produce a path that starts with a slash, just like "ordinary" paths. The attached patch cleans up __d_path() to fix the bug with overlapping pathname components. It also adds a @fail_deleted argument, which allows to get rid of some of the mess in sys_getcwd(). Grabbing the dcache_lock can then also be moved into __d_path(). The patch also makes sure that paths will only start with a slash for paths which are connected to the root and rootmnt. The @fail_deleted argument could be added to d_path() as well: this would allow callers to recognize deleted files, without having to resort to the ambiguous check for the " (deleted)" string at the end of the pathnames. This is not currently done, but it might be worthwhile. Signed-off-by: Andreas Gruenbacher <agruen@suse.de> Cc: Neil Brown <neilb@suse.de> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Christoph Hellwig <hch@lst.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

[PATCH] Fix d_path for lazy unmounts
Here is a bugfix to d_path. First, when d_path() hits a lazily unmounted mount point, it tries to prepend the name of the lazily unmounted dentry to the path name. It gets this wrong, and also overwrites the slash that separates the name from the following pathname component. This is demonstrated by the attached test case, which prints "getcwd returned d_path-bugsubdir" with the bug. The correct result would be "getcwd returned d_path-bug/subdir". It could be argued that the name of the root dentry should not be part of the result of d_path in the first place. On the other hand, what the unconnected namespace was once reachable as may provide some useful hints to users, and so that seems okay. Second, it isn't always possible to tell from the __d_path result whether the specified root and rootmnt (i.e., the chroot) was reached: lazy unmounts of bind mounts will produce a path that does start with a non-slash so we can tell from that, but other lazy unmounts will produce a path that starts with a slash, just like "ordinary" paths. The attached patch cleans up __d_path() to fix the bug with overlapping pathname components. It also adds a @fail_deleted argument, which allows to get rid of some of the mess in sys_getcwd(). Grabbing the dcache_lock can then also be moved into __d_path(). The patch also makes sure that paths will only start with a slash for paths which are connected to the root and rootmnt. The @fail_deleted argument could be added to d_path() as well: this would allow callers to recognize deleted files, without having to resort to the ambiguous check for the " (deleted)" string at the end of the pathnames. This is not currently done, but it might be worthwhile. Signed-off-by: Andreas Gruenbacher <agruen@suse.de> Cc: Neil Brown <neilb@suse.de> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Christoph Hellwig <hch@lst.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Andreas Gruenbacher · Linus Torvalds
1 parent 5c3bd438cc
Showing 1 changed file with 80 additions and 70 deletions Side-by-side Diff
fs/dcache.c
@@ -1739,45 +1739,41 @@
  * @rootmnt: vfsmnt to which the root dentry belongs
  * @buffer: buffer to return value in
  * @buflen: buffer length
+ * @fail_deleted: what to return for deleted files
  *
- * Convert a dentry into an ASCII path name. If the entry has been deleted
- * the string " (deleted)" is appended. Note that this is ambiguous.
+ * Convert a dentry into an ASCII path name. If the entry has been deleted,
+ * then if @fail_deleted is true, ERR_PTR(-ENOENT) is returned. Otherwise,
+ * the the string " (deleted)" is appended. Note that this is ambiguous.
  *
- * Returns the buffer or an error code if the path was too long.
- *
- * "buflen" should be positive. Caller holds the dcache_lock.
+ * Returns the buffer or an error code.
  */
-static char * __d_path( struct dentry *dentry, struct vfsmount *vfsmnt,
-			struct dentry *root, struct vfsmount *rootmnt,
-			char *buffer, int buflen)
+static char *__d_path(struct dentry *dentry, struct vfsmount *vfsmnt,
+		      struct dentry *root, struct vfsmount *rootmnt,
+		      char *buffer, int buflen, int fail_deleted)
 {
-	char * end = buffer+buflen;
-	char * retval;
-	int namelen;
+	int namelen, is_slash;
  
-	*--end = '\0';
-	buflen--;
+	if (buflen < 2)
+		return ERR_PTR(-ENAMETOOLONG);
+	buffer += --buflen;
+	*buffer = '\0';
+
+	spin_lock(&dcache_lock);
 	if (!IS_ROOT(dentry) && d_unhashed(dentry)) {
-		buflen -= 10;
-		end -= 10;
-		if (buflen < 0)
+		if (fail_deleted) {
+			buffer = ERR_PTR(-ENOENT);
+			goto out;
+		}
+		if (buflen < 10)
 			goto Elong;
-		memcpy(end, " (deleted)", 10);
+		buflen -= 10;
+		buffer -= 10;
+		memcpy(buffer, " (deleted)", 10);
 	}
-
-	if (buflen < 1)
-		goto Elong;
-	/* Get '/' right */
-	retval = end-1;
-	*retval = '/';
-
-	for (;;) {
+	while (dentry != root || vfsmnt != rootmnt) {
 		struct dentry * parent;
  
-		if (dentry == root && vfsmnt == rootmnt)
-			break;
 		if (dentry == vfsmnt->mnt_root || IS_ROOT(dentry)) {
-			/* Global root? */
 			spin_lock(&vfsmount_lock);
 			if (vfsmnt->mnt_parent == vfsmnt) {
 				spin_unlock(&vfsmount_lock);
  
  
  
  
  
  
  
  
@@ -1791,33 +1787,60 @@
 		parent = dentry->d_parent;
 		prefetch(parent);
 		namelen = dentry->d_name.len;
-		buflen -= namelen + 1;
-		if (buflen < 0)
+		if (buflen <= namelen)
 			goto Elong;
-		end -= namelen;
-		memcpy(end, dentry->d_name.name, namelen);
-		*--end = '/';
-		retval = end;
+		buflen -= namelen + 1;
+		buffer -= namelen;
+		memcpy(buffer, dentry->d_name.name, namelen);
+		*--buffer = '/';
 		dentry = parent;
 	}
+	/* Get '/' right */
+	if (*buffer != '/')
+		*--buffer = '/';
  
-	return retval;
+out:
+	spin_unlock(&dcache_lock);
+	return buffer;
  
 global_root:
+	/*
+	 * We went past the (vfsmount, dentry) we were looking for and have
+	 * either hit a root dentry, a lazily unmounted dentry, an
+	 * unconnected dentry, or the file is on a pseudo filesystem.
+	 */
 	namelen = dentry->d_name.len;
-	buflen -= namelen;
-	if (buflen < 0)
+	is_slash = (namelen == 1 && *dentry->d_name.name == '/');
+	if (is_slash || (dentry->d_sb->s_flags & MS_NOUSER)) {
+		/*
+		 * Make sure we won't return a pathname starting with '/'.
+		 *
+		 * Historically, we also glue together the root dentry and
+		 * remaining name for pseudo filesystems like pipefs, which
+		 * have the MS_NOUSER flag set. This results in pathnames
+		 * like "pipe:[439336]".
+		 */
+		if (*buffer == '/') {
+			buffer++;
+			buflen++;
+		}
+		if (is_slash)
+			goto out;
+	}
+	if (buflen < namelen)
 		goto Elong;
-	retval -= namelen-1;	/* hit the slash */
-	memcpy(retval, dentry->d_name.name, namelen);
-	return retval;
+	buffer -= namelen;
+	memcpy(buffer, dentry->d_name.name, namelen);
+	goto out;
+
 Elong:
-	return ERR_PTR(-ENAMETOOLONG);
+	buffer = ERR_PTR(-ENAMETOOLONG);
+	goto out;
 }
  
 /* write full pathname into buffer and return start of pathname */
-char * d_path(struct dentry *dentry, struct vfsmount *vfsmnt,
-				char *buf, int buflen)
+char *d_path(struct dentry *dentry, struct vfsmount *vfsmnt, char *buf,
+	     int buflen)
 {
 	char *res;
 	struct vfsmount *rootmnt;
@@ -1827,9 +1850,7 @@
 	rootmnt = mntget(current->fs->rootmnt);
 	root = dget(current->fs->root);
 	read_unlock(&current->fs->lock);
-	spin_lock(&dcache_lock);
-	res = __d_path(dentry, vfsmnt, root, rootmnt, buf, buflen);
-	spin_unlock(&dcache_lock);
+	res = __d_path(dentry, vfsmnt, root, rootmnt, buf, buflen, 0);
 	dput(root);
 	mntput(rootmnt);
 	return res;
  
@@ -1855,10 +1876,10 @@
  */
 asmlinkage long sys_getcwd(char __user *buf, unsigned long size)
 {
-	int error;
+	int error, len;
 	struct vfsmount *pwdmnt, *rootmnt;
 	struct dentry *pwd, *root;
-	char *page = (char *) __get_free_page(GFP_USER);
+	char *page = (char *) __get_free_page(GFP_USER), *cwd;
  
 	if (!page)
 		return -ENOMEM;
  
@@ -1870,29 +1891,18 @@
 	root = dget(current->fs->root);
 	read_unlock(&current->fs->lock);
  
-	error = -ENOENT;
-	/* Has the current directory has been unlinked? */
-	spin_lock(&dcache_lock);
-	if (pwd->d_parent == pwd || !d_unhashed(pwd)) {
-		unsigned long len;
-		char * cwd;
+	cwd = __d_path(pwd, pwdmnt, root, rootmnt, page, PAGE_SIZE, 1);
+	error = PTR_ERR(cwd);
+	if (IS_ERR(cwd))
+		goto out;
  
-		cwd = __d_path(pwd, pwdmnt, root, rootmnt, page, PAGE_SIZE);
-		spin_unlock(&dcache_lock);
-
-		error = PTR_ERR(cwd);
-		if (IS_ERR(cwd))
-			goto out;
-
-		error = -ERANGE;
-		len = PAGE_SIZE + page - cwd;
-		if (len <= size) {
-			error = len;
-			if (copy_to_user(buf, cwd, len))
-				error = -EFAULT;
-		}
-	} else
-		spin_unlock(&dcache_lock);
+	error = -ERANGE;
+	len = PAGE_SIZE + page - cwd;
+	if (len <= size) {
+		error = len;
+		if (copy_to_user(buf, cwd, len))
+			error = -EFAULT;
+	}
  
 out:
 	dput(pwd);
...	...	@@ -1739,45 +1739,41 @@
1739	1739	* @rootmnt: vfsmnt to which the root dentry belongs
1740	1740	* @buffer: buffer to return value in
1741	1741	* @buflen: buffer length
	1742	+ * @fail_deleted: what to return for deleted files
1742	1743	*
1743		- * Convert a dentry into an ASCII path name. If the entry has been deleted
1744		- * the string " (deleted)" is appended. Note that this is ambiguous.
	1744	+ * Convert a dentry into an ASCII path name. If the entry has been deleted,
	1745	+ * then if @fail_deleted is true, ERR_PTR(-ENOENT) is returned. Otherwise,
	1746	+ * the the string " (deleted)" is appended. Note that this is ambiguous.
1745	1747	*
1746		- * Returns the buffer or an error code if the path was too long.
1747		- *
1748		- * "buflen" should be positive. Caller holds the dcache_lock.
	1748	+ * Returns the buffer or an error code.
1749	1749	*/
1750		-static char * __d_path( struct dentry dentry, struct vfsmount vfsmnt,
1751		- struct dentry root, struct vfsmount rootmnt,
1752		- char *buffer, int buflen)
	1750	+static char __d_path(struct dentry dentry, struct vfsmount *vfsmnt,
	1751	+ struct dentry root, struct vfsmount rootmnt,
	1752	+ char *buffer, int buflen, int fail_deleted)
1753	1753	{
1754		- char * end = buffer+buflen;
1755		- char * retval;
1756		- int namelen;
	1754	+ int namelen, is_slash;
1757	1755
1758		- *--end = '\0';
1759		- buflen--;
	1756	+ if (buflen < 2)
	1757	+ return ERR_PTR(-ENAMETOOLONG);
	1758	+ buffer += --buflen;
	1759	+ *buffer = '\0';
	1760	+
	1761	+ spin_lock(&dcache_lock);
1760	1762	if (!IS_ROOT(dentry) && d_unhashed(dentry)) {
1761		- buflen -= 10;
1762		- end -= 10;
1763		- if (buflen < 0)
	1763	+ if (fail_deleted) {
	1764	+ buffer = ERR_PTR(-ENOENT);
	1765	+ goto out;
	1766	+ }
	1767	+ if (buflen < 10)
1764	1768	goto Elong;
1765		- memcpy(end, " (deleted)", 10);
	1769	+ buflen -= 10;
	1770	+ buffer -= 10;
	1771	+ memcpy(buffer, " (deleted)", 10);
1766	1772	}
1767		-
1768		- if (buflen < 1)
1769		- goto Elong;
1770		- /* Get '/' right */
1771		- retval = end-1;
1772		- *retval = '/';
1773		-
1774		- for (;;) {
	1773	+ while (dentry != root \|\| vfsmnt != rootmnt) {
1775	1774	struct dentry * parent;
1776	1775
1777		- if (dentry == root && vfsmnt == rootmnt)
1778		- break;
1779	1776	if (dentry == vfsmnt->mnt_root \|\| IS_ROOT(dentry)) {
1780		- /* Global root? */
1781	1777	spin_lock(&vfsmount_lock);
1782	1778	if (vfsmnt->mnt_parent == vfsmnt) {
1783	1779	spin_unlock(&vfsmount_lock);
1784	1780
1785	1781
1786	1782
1787	1783
1788	1784
1789	1785
1790	1786
1791	1787
...	...	@@ -1791,33 +1787,60 @@
1791	1787	parent = dentry->d_parent;
1792	1788	prefetch(parent);
1793	1789	namelen = dentry->d_name.len;
1794		- buflen -= namelen + 1;
1795		- if (buflen < 0)
	1790	+ if (buflen <= namelen)
1796	1791	goto Elong;
1797		- end -= namelen;
1798		- memcpy(end, dentry->d_name.name, namelen);
1799		- *--end = '/';
1800		- retval = end;
	1792	+ buflen -= namelen + 1;
	1793	+ buffer -= namelen;
	1794	+ memcpy(buffer, dentry->d_name.name, namelen);
	1795	+ *--buffer = '/';
1801	1796	dentry = parent;
1802	1797	}
	1798	+ /* Get '/' right */
	1799	+ if (*buffer != '/')
	1800	+ *--buffer = '/';
1803	1801
1804		- return retval;
	1802	+out:
	1803	+ spin_unlock(&dcache_lock);
	1804	+ return buffer;
1805	1805
1806	1806	global_root:
	1807	+ /*
	1808	+ * We went past the (vfsmount, dentry) we were looking for and have
	1809	+ * either hit a root dentry, a lazily unmounted dentry, an
	1810	+ * unconnected dentry, or the file is on a pseudo filesystem.
	1811	+ */
1807	1812	namelen = dentry->d_name.len;
1808		- buflen -= namelen;
1809		- if (buflen < 0)
	1813	+ is_slash = (namelen == 1 && *dentry->d_name.name == '/');
	1814	+ if (is_slash \|\| (dentry->d_sb->s_flags & MS_NOUSER)) {
	1815	+ /*
	1816	+ * Make sure we won't return a pathname starting with '/'.
	1817	+ *
	1818	+ * Historically, we also glue together the root dentry and
	1819	+ * remaining name for pseudo filesystems like pipefs, which
	1820	+ * have the MS_NOUSER flag set. This results in pathnames
	1821	+ * like "pipe:[439336]".
	1822	+ */
	1823	+ if (*buffer == '/') {
	1824	+ buffer++;
	1825	+ buflen++;
	1826	+ }
	1827	+ if (is_slash)
	1828	+ goto out;
	1829	+ }
	1830	+ if (buflen < namelen)
1810	1831	goto Elong;
1811		- retval -= namelen-1; /* hit the slash */
1812		- memcpy(retval, dentry->d_name.name, namelen);
1813		- return retval;
	1832	+ buffer -= namelen;
	1833	+ memcpy(buffer, dentry->d_name.name, namelen);
	1834	+ goto out;
	1835	+
1814	1836	Elong:
1815		- return ERR_PTR(-ENAMETOOLONG);
	1837	+ buffer = ERR_PTR(-ENAMETOOLONG);
	1838	+ goto out;
1816	1839	}
1817	1840
1818	1841	/* write full pathname into buffer and return start of pathname */
1819		-char * d_path(struct dentry dentry, struct vfsmount vfsmnt,
1820		- char *buf, int buflen)
	1842	+char d_path(struct dentry dentry, struct vfsmount vfsmnt, char buf,
	1843	+ int buflen)
1821	1844	{
1822	1845	char *res;
1823	1846	struct vfsmount *rootmnt;
...	...	@@ -1827,9 +1850,7 @@
1827	1850	rootmnt = mntget(current->fs->rootmnt);
1828	1851	root = dget(current->fs->root);
1829	1852	read_unlock(&current->fs->lock);
1830		- spin_lock(&dcache_lock);
1831		- res = __d_path(dentry, vfsmnt, root, rootmnt, buf, buflen);
1832		- spin_unlock(&dcache_lock);
	1853	+ res = __d_path(dentry, vfsmnt, root, rootmnt, buf, buflen, 0);
1833	1854	dput(root);
1834	1855	mntput(rootmnt);
1835	1856	return res;
1836	1857
...	...	@@ -1855,10 +1876,10 @@
1855	1876	*/
1856	1877	asmlinkage long sys_getcwd(char __user *buf, unsigned long size)
1857	1878	{
1858		- int error;
	1879	+ int error, len;
1859	1880	struct vfsmount pwdmnt, rootmnt;
1860	1881	struct dentry pwd, root;
1861		- char page = (char ) __get_free_page(GFP_USER);
	1882	+ char page = (char ) __get_free_page(GFP_USER), *cwd;
1862	1883
1863	1884	if (!page)
1864	1885	return -ENOMEM;
1865	1886
...	...	@@ -1870,29 +1891,18 @@
1870	1891	root = dget(current->fs->root);
1871	1892	read_unlock(&current->fs->lock);
1872	1893
1873		- error = -ENOENT;
1874		- /* Has the current directory has been unlinked? */
1875		- spin_lock(&dcache_lock);
1876		- if (pwd->d_parent == pwd \|\| !d_unhashed(pwd)) {
1877		- unsigned long len;
1878		- char * cwd;
	1894	+ cwd = __d_path(pwd, pwdmnt, root, rootmnt, page, PAGE_SIZE, 1);
	1895	+ error = PTR_ERR(cwd);
	1896	+ if (IS_ERR(cwd))
	1897	+ goto out;
1879	1898
1880		- cwd = __d_path(pwd, pwdmnt, root, rootmnt, page, PAGE_SIZE);
1881		- spin_unlock(&dcache_lock);
1882		-
1883		- error = PTR_ERR(cwd);
1884		- if (IS_ERR(cwd))
1885		- goto out;
1886		-
1887		- error = -ERANGE;
1888		- len = PAGE_SIZE + page - cwd;
1889		- if (len <= size) {
1890		- error = len;
1891		- if (copy_to_user(buf, cwd, len))
1892		- error = -EFAULT;
1893		- }
1894		- } else
1895		- spin_unlock(&dcache_lock);
	1899	+ error = -ERANGE;
	1900	+ len = PAGE_SIZE + page - cwd;
	1901	+ if (len <= size) {
	1902	+ error = len;
	1903	+ if (copy_to_user(buf, cwd, len))
	1904	+ error = -EFAULT;
	1905	+ }
1896	1906
1897	1907	out:
1898	1908	dput(pwd);