linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: jjohansen@suse.de
To: akpm@linux-foundation.org
Cc: linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	Andreas Gruenbacher <agruen@suse.de>,
	John Johansen <jjohansen@suse.de>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>
Subject: [AppArmor 29/45] Fix __d_path() for lazy unmounts and make it unambiguous
Date: Thu, 25 Oct 2007 23:40:53 -0700	[thread overview]
Message-ID: <20071026064050.969904231@suse.de> (raw)
In-Reply-To: 20071026064024.243943043@suse.de

[-- Attachment #1: unambiguous-__d_path.diff --]
[-- Type: text/plain, Size: 7853 bytes --]

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 patch fixes that; if a process was in directory
/foo/bar and /foo got lazily unmounted, the old result was ``foobar'' (note the
missing slash), while the new result with this patch is ``foo/bar''.

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.  We need an
unambiguous result for AppArmor at least though, so we make sure that paths
will only start with a slash if the path leads all the way up to the root.

We also add a @fail_deleted argument, which allows to get rid of some of the
mess in sys_getcwd().

This patch leaves getcwd() and d_path() as they were before for everything
except for bind-mounted directories; for them, it reports ``/foo/bar'' instead
of ``foobar'' in the example described above.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
Signed-off-by: John Johansen <jjohansen@suse.de>
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>

---
 fs/dcache.c |  169 ++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 98 insertions(+), 71 deletions(-)

--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1763,52 +1763,51 @@ shouldnt_be_hashed:
 }
 
 /**
- * d_path - return the path of a dentry
+ * __d_path - return the path of a dentry
  * @dentry: dentry to report
  * @vfsmnt: vfsmnt to which the dentry belongs
  * @root: root dentry
  * @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
+ * 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 string " (deleted)" is appended. Note that this is ambiguous.
  *
- * Returns the buffer or an error code if the path was too long.
+ * If @dentry is not connected to @root, the path returned will be relative
+ * (i.e., it will not start with a slash).
  *
- * "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)
-{
-	char * end = buffer+buflen;
-	char * retval;
-	int namelen;
+static char *__d_path(struct dentry *dentry, struct vfsmount *vfsmnt,
+		      struct dentry *root, struct vfsmount *rootmnt,
+		      char *buffer, int buflen, int fail_deleted)
+{
+	int namelen, is_slash;
+
+	if (buflen < 2)
+		return ERR_PTR(-ENAMETOOLONG);
+	buffer += --buflen;
+	*buffer = '\0';
 
-	*--end = '\0';
-	buflen--;
+	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);
@@ -1822,33 +1821,72 @@ static char * __d_path( struct dentry *d
 		parent = dentry->d_parent;
 		prefetch(parent);
 		namelen = dentry->d_name.len;
-		buflen -= namelen + 1;
-		if (buflen < 0)
+		if (buflen < namelen + 1)
 			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;
+}
+
+static char *__connect_d_path(char *path, char *buffer)
+{
+	if (!IS_ERR(path) && *path != '/') {
+		/* Pretend that disconnected paths are hanging off the root. */
+		if (path == buffer)
+			path = ERR_PTR(-ENAMETOOLONG);
+		else
+			*--path = '/';
+	}
+	return path;
 }
 
 /* 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;
@@ -1868,9 +1906,8 @@ char * d_path(struct dentry *dentry, str
 	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);
+	res = __connect_d_path(res, buf);
 	dput(root);
 	mntput(rootmnt);
 	return res;
@@ -1917,10 +1954,10 @@ char *dynamic_dname(struct dentry *dentr
  */
 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;
@@ -1932,29 +1969,19 @@ asmlinkage long sys_getcwd(char __user *
 	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);
-		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);
+	cwd = __d_path(pwd, pwdmnt, root, rootmnt, page, PAGE_SIZE, 1);
+	cwd = __connect_d_path(cwd, page);
+	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;
+	}
 
 out:
 	dput(pwd);

-- 


  parent reply	other threads:[~2007-10-26  7:09 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-26  6:40 [AppArmor 00/45] AppArmor security module overview jjohansen
2007-10-26  6:40 ` [AppArmor 01/45] Pass struct vfsmount to the inode_create LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 02/45] Pass struct path down to remove_suid and children jjohansen
2007-10-26  6:40 ` [AppArmor 03/45] Add a vfsmount parameter to notify_change() jjohansen
2007-10-26  6:40 ` [AppArmor 04/45] Pass struct vfsmount to the inode_setattr LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 05/45] Add struct vfsmount parameter to vfs_mkdir() jjohansen
2007-10-26  6:40 ` [AppArmor 06/45] Pass struct vfsmount to the inode_mkdir LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 07/45] Add a struct vfsmount parameter to vfs_mknod() jjohansen
2007-10-26  6:40 ` [AppArmor 08/45] Pass struct vfsmount to the inode_mknod LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 09/45] Add a struct vfsmount parameter to vfs_symlink() jjohansen
2007-10-26  6:40 ` [AppArmor 10/45] Pass struct vfsmount to the inode_symlink LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 11/45] Pass struct vfsmount to the inode_readlink " jjohansen
2007-10-26  6:40 ` [AppArmor 12/45] Add struct vfsmount parameters to vfs_link() jjohansen
2007-10-26  6:40 ` [AppArmor 13/45] Pass the struct vfsmounts to the inode_link LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 14/45] Add a struct vfsmount parameter to vfs_rmdir() jjohansen
2007-10-26  6:40 ` [AppArmor 15/45] Pass struct vfsmount to the inode_rmdir LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 16/45] Call lsm hook before unhashing dentry in vfs_rmdir() jjohansen
2007-10-26  6:40 ` [AppArmor 17/45] Add a struct vfsmount parameter to vfs_unlink() jjohansen
2007-10-26  6:40 ` [AppArmor 18/45] Pass struct vfsmount to the inode_unlink LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 19/45] Add struct vfsmount parameters to vfs_rename() jjohansen
2007-10-26  7:37   ` Al Viro
2007-10-26 18:23     ` John Johansen
2007-10-26 20:33       ` Al Viro
2007-10-26  6:40 ` [AppArmor 20/45] Pass struct vfsmount to the inode_rename LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 21/45] Add a struct vfsmount parameter to vfs_setxattr() jjohansen
2007-10-26  6:40 ` [AppArmor 22/45] Pass struct vfsmount to the inode_setxattr LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 23/45] Add a struct vfsmount parameter to vfs_getxattr() jjohansen
2007-10-26  6:40 ` [AppArmor 24/45] Pass struct vfsmount to the inode_getxattr LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 25/45] Add a struct vfsmount parameter to vfs_listxattr() jjohansen
2007-10-26  6:40 ` [AppArmor 26/45] Pass struct vfsmount to the inode_listxattr LSM hook jjohansen
2007-10-26  6:40 ` [AppArmor 27/45] Add a struct vfsmount parameter to vfs_removexattr() jjohansen
2007-10-26  6:40 ` [AppArmor 28/45] Pass struct vfsmount to the inode_removexattr LSM hook jjohansen
2007-10-26  6:40 ` jjohansen [this message]
2007-10-26  6:40 ` [AppArmor 30/45] Make d_path() consistent across mount operations jjohansen
2007-10-26  6:40 ` [AppArmor 31/45] Add d_namespace_path() to compute namespace relative pathnames jjohansen
2007-10-26  6:40 ` [AppArmor 32/45] Enable LSM hooks to distinguish operations on file descriptors from operations on pathnames jjohansen
2007-10-26 11:30   ` Miklos Szeredi
2007-10-26 11:45     ` Miklos Szeredi
2007-10-26 18:49     ` John Johansen
2007-10-26 20:24     ` Andreas Gruenbacher
2007-10-26 20:58       ` Miklos Szeredi
2007-10-26 21:56         ` Andreas Gruenbacher
2007-10-26  6:40 ` [AppArmor 33/45] Pass struct file down the inode_*xattr security LSM hooks jjohansen
2007-10-26  6:40 ` [AppArmor 34/45] Factor out sysctl pathname code jjohansen
2007-10-26  9:24   ` James Morris
2007-10-26  6:40 ` [AppArmor 35/45] Allow permission functions to tell between parent and leaf checks jjohansen
2007-10-26 12:32   ` Stephen Smalley
2007-10-26 18:26     ` John Johansen
2007-10-26  6:41 ` [AppArmor 36/45] Export audit subsystem for use by modules jjohansen
2007-10-26  6:41 ` [AppArmor 37/45] AppArmor: Main Part jjohansen
2007-10-26  6:41 ` [AppArmor 38/45] AppArmor: Module and LSM hooks jjohansen
2007-10-26  6:41 ` [AppArmor 39/45] AppArmor: Profile loading and manipulation, pathname matching jjohansen
2007-10-26  6:41 ` [AppArmor 40/45] AppArmor: all the rest jjohansen
2007-10-26  6:41 ` [AppArmor 41/45] add simple network toggles to apparmor jjohansen
2007-10-26  6:41 ` [AppArmor 42/45] Add AppArmor LSM to security/Makefile jjohansen
2007-10-26  6:41 ` [AppArmor 43/45] Switch to vfs_permission() in do_path_lookup() jjohansen
2007-10-26  6:41 ` [AppArmor 44/45] Switch to vfs_permission() in sys_fchdir() jjohansen
2007-10-26  6:41 ` [AppArmor 45/45] Fix file_permission() jjohansen
2007-10-26  7:04 ` [AppArmor 00/45] AppArmor security module overview John Johansen
2007-10-26 14:37 ` Arjan van de Ven
2007-10-26 18:34   ` John Johansen
2007-10-26 20:15     ` Arjan van de Ven
2007-10-26 20:44   ` Andreas Gruenbacher
2007-10-26 21:13     ` Arjan van de Ven
2007-10-26 21:24       ` Andreas Gruenbacher
2007-10-26 22:16       ` Crispin Cowan
2007-10-26 22:23         ` Arjan van de Ven
2007-10-27 20:47   ` Christoph Hellwig
2007-10-28 14:25     ` Andreas Gruenbacher
  -- strict thread matches above, loose matches on Subject: below --
2007-05-14 11:06 jjohansen
2007-05-14 11:06 ` [AppArmor 29/45] Fix __d_path() for lazy unmounts and make it unambiguous jjohansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071026064050.969904231@suse.de \
    --to=jjohansen@suse.de \
    --cc=agruen@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).