vfs-fix-d_path-for-unreachable-paths.patch

vfs-fix-d_path-for-unreachable-paths.patch

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 2078 bytes --]

This patch apparently does something resembling what it's supposed to - the
first few lines of /proc/mounts now looks like:

% head -5 /proc/mounts
rootfs (unreachable)/ rootfs rw 0 0
/dev/root / ext3 rw,seclabel,noatime,nodiratime,user_xattr,acl,data=writeback,us
rquota,grpquota 0 0
/dev /dev tmpfs rw,seclabel,relatime,mode=755 0 0
/proc /proc proc rw,relatime 0 0
/sys /sys sysfs rw,relatime 0 0

The now-unreachable rootfs is the letftover initrd rootfs.

The patch commentary notes:

> This patch addresses all these issues, by prefixing such unreachable paths
> with "(unreachable)".  This isn't perfect since the returned path may
> still be a valid _relative_ path, and applications may not check the
> result of getcwd() for starting with a '/' before using it.

It turns out that some things don't check the contents of /proc/mounts for
starting with a / before using it either:

# /sbin/restorecon -v /etc/passwd
Full path required for exclude: (unreachable)/.

And strace shows it's a read of /proc/mounts, not a getcwd() call:

....
open("/proc/self/task/11479/attr/current", O_RDONLY) = 3
read(3, "staff_u:sysadm_r:setfiles_t:s0\0"..., 4095) = 31
close(3)                                = 0
uname({sys="Linux", node="turing-police.cc.vt.edu", ...}) = 0
open("/proc/mounts", O_RDONLY)          = 3
fstat(3, {st_mode=S_IFREG|0444, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fe
9ce36a000
read(3, "rootfs (unreachable)/ rootfs rw 0"..., 1024) = 1024
write(2, "Full path required for exclude: ("..., 48) = 48

The added "(unreachable) text also gives /etc/rc0.d/S01halt indigestion,
because it thinks it can do stuff like:

awk '$2 !~ /\/(|dev|proc|selinux|sys)$/ && $1 !~ /^\/dev\/ram/ { print $2 }' \
    /proc/mounts | sort -r | \
  while read line; do
    $UMOUNT -f $line
done

Somebody is buggy here, but I'm not sure who. The initrd for leaving a dangling
reference, the patch for breaking /proc/mounts, or /sbin/restorecon and the
shutdown script for being far too trusting of what the kernel tells it?


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: vfs-fix-d_path-for-unreachable-paths.patch

[Miklos Szeredi]

On Sat, 2009-09-05 at 10:22 -0400, Valdis.Kletnieks@vt.edu wrote:
> Somebody is buggy here, but I'm not sure who. The initrd for leaving a dangling
> reference, the patch for breaking /proc/mounts, or /sbin/restorecon and the
> shutdown script for being far too trusting of what the kernel tells it?
> 

Hugh Dickins reported a similar issue, where gnome-vfs chokes on
that /proc/mounts line as well.

I think the conclusion is that this change in /proc/mounts is not going
to work.  There are two solutions:

 1) leave out unreachable mounts from /proc/PID/mounts altogether
 2) completely revert /proc/PID/mounts to the old behavior

The first option is much less likely to break something than the current
patch, but it's still an ABI change.

So I think it's better to go with the second option.  Patches coming
up...

Thanks,
Miklos

[patch 1/2] vfs: seq_file: add helpers for data filling

[Miklos Szeredi]

From: Miklos Szeredi <mszeredi@suse.cz>

Add two helpers that allow access to the seq_file's own buffer, but
hides the internal details of seq_files.

This allows easier implementation of special purpose filling
functions.  It also cleans up some existing functions which duplicated
the seq_file logic.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/seq_file.c            |  112 ++++++++++++++++++++++++++++++++---------------
 include/linux/seq_file.h |    3 +
 2 files changed, 80 insertions(+), 35 deletions(-)

Index: linux-2.6/fs/seq_file.c
===================================================================
--- linux-2.6.orig/fs/seq_file.c	2009-09-03 19:40:10.000000000 +0200
+++ linux-2.6/fs/seq_file.c	2009-09-04 14:58:04.000000000 +0200
@@ -419,6 +419,44 @@ char *mangle_path(char *s, char *p, char
 EXPORT_SYMBOL(mangle_path);
 
 /**
+ * seq_get_buf - get buffer to write arbitrary data to
+ * @m: the seq_file handle
+ * @bufp: the beginning of the buffer is stored here
+ *
+ * Return the number of bytes available in the buffer, or zero if
+ * there's no space.
+ */
+size_t seq_get_buf(struct seq_file *m, char **bufp)
+{
+	BUG_ON(m->count > m->size);
+	if (m->count < m->size)
+		*bufp = m->buf + m->count;
+	else
+		*bufp = NULL;
+
+	return m->size - m->count;
+}
+
+/**
+ * seq_commit - commit data to the buffer
+ * @m: the seq_file handle
+ * @num: the number of bytes to commit
+ *
+ * Commit @num bytes of data written to a buffer previously acquired
+ * by seq_buf_get.  To signal an error condition, or that the data
+ * didn't fit in the available space, pass a negative @num value.
+ */
+void seq_commit(struct seq_file *m, int num)
+{
+	if (num < 0) {
+		m->count = m->size;
+	} else {
+		BUG_ON(m->count + num > m->size);
+		m->count += num;
+	}
+}
+
+/**
  * seq_path - seq_file interface to print a pathname
  * @m: the seq_file handle
  * @path: the struct path to print
@@ -429,20 +467,21 @@ EXPORT_SYMBOL(mangle_path);
  */
 int seq_path(struct seq_file *m, struct path *path, char *esc)
 {
-	if (m->count < m->size) {
-		char *s = m->buf + m->count;
-		char *p = d_path(path, s, m->size - m->count);
+	char *buf;
+	size_t size = seq_get_buf(m, &buf);
+	int res = -1;
+
+	if (size) {
+		char *p = d_path(path, buf, size);
 		if (!IS_ERR(p)) {
-			s = mangle_path(s, p, esc);
-			if (s) {
-				p = m->buf + m->count;
-				m->count = s - m->buf;
-				return s - p;
-			}
+			char *end = mangle_path(buf, p, esc);
+			if (end)
+				res = end - buf;
 		}
 	}
-	m->count = m->size;
-	return -1;
+	seq_commit(m, res);
+
+	return res;
 }
 EXPORT_SYMBOL(seq_path);
 
@@ -454,26 +493,28 @@ EXPORT_SYMBOL(seq_path);
 int seq_path_root(struct seq_file *m, struct path *path, struct path *root,
 		  char *esc)
 {
-	int err = -ENAMETOOLONG;
-	if (m->count < m->size) {
-		char *s = m->buf + m->count;
+	char *buf;
+	size_t size = seq_get_buf(m, &buf);
+	int res = -ENAMETOOLONG;
+
+	if (size) {
 		char *p;
 
 		spin_lock(&dcache_lock);
-		p = __d_path(path, root, s, m->size - m->count);
+		p = __d_path(path, root, buf, size);
 		spin_unlock(&dcache_lock);
-		err = PTR_ERR(p);
+		res = PTR_ERR(p);
 		if (!IS_ERR(p)) {
-			s = mangle_path(s, p, esc);
-			if (s) {
-				p = m->buf + m->count;
-				m->count = s - m->buf;
-				return 0;
-			}
+			char *end = mangle_path(buf, p, esc);
+			if (end)
+				res = end - buf;
+			else
+				res = -ENAMETOOLONG;
 		}
 	}
-	m->count = m->size;
-	return err;
+	seq_commit(m, res);
+
+	return res < 0 ? res : 0;
 }
 
 /*
@@ -481,20 +522,21 @@ int seq_path_root(struct seq_file *m, st
  */
 int seq_dentry(struct seq_file *m, struct dentry *dentry, char *esc)
 {
-	if (m->count < m->size) {
-		char *s = m->buf + m->count;
-		char *p = dentry_path(dentry, s, m->size - m->count);
+	char *buf;
+	size_t size = seq_get_buf(m, &buf);
+	int res = -1;
+
+	if (size) {
+		char *p = dentry_path(dentry, buf, size);
 		if (!IS_ERR(p)) {
-			s = mangle_path(s, p, esc);
-			if (s) {
-				p = m->buf + m->count;
-				m->count = s - m->buf;
-				return s - p;
-			}
+			char *end = mangle_path(buf, p, esc);
+			if (end)
+				res = end - buf;
 		}
 	}
-	m->count = m->size;
-	return -1;
+	seq_commit(m, res);
+
+	return res;
 }
 
 int seq_bitmap(struct seq_file *m, const unsigned long *bits,
Index: linux-2.6/include/linux/seq_file.h
===================================================================
--- linux-2.6.orig/include/linux/seq_file.h	2009-09-04 13:17:03.000000000 +0200
+++ linux-2.6/include/linux/seq_file.h	2009-09-04 14:26:49.000000000 +0200
@@ -48,6 +48,9 @@ int seq_write(struct seq_file *seq, cons
 int seq_printf(struct seq_file *, const char *, ...)
 	__attribute__ ((format (printf,2,3)));
 
+size_t seq_get_buf(struct seq_file *m, char **bufp);
+void seq_commit(struct seq_file *m, int num);
+
 int seq_path(struct seq_file *, struct path *, char *);
 int seq_dentry(struct seq_file *, struct dentry *, char *);
 int seq_path_root(struct seq_file *m, struct path *path, struct path *root,

[patch 2/2] vfs: revert /proc/mounts to old behavior for unreachable mountpoints

[Miklos Szeredi]

From: Miklos Szeredi <mszeredi@suse.cz>

"vfs: fix d_path() for unreachable paths" prefixes unreachable paths
with "(unreachable)" in the result of getcwd(2), /proc/*/mounts,
/proc/*/cwd, /proc/*/fd/*, etc...

Hugh Dickins reported that an old version of gnome-vfs-daemon crashes
because it finds an entry in /proc/mounts where the mountpoint is
unreachable.

This patch reverts /proc/mounts to the old behavior (or rather a less
crazy version of the old behavior).

Reported-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Reported-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/namespace.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

Index: linux-2.6/fs/namespace.c
===================================================================
--- linux-2.6.orig/fs/namespace.c	2009-09-04 15:25:36.000000000 +0200
+++ linux-2.6/fs/namespace.c	2009-09-04 15:40:25.000000000 +0200
@@ -789,6 +789,61 @@ static void show_type(struct seq_file *m
 	}
 }
 
+/*
+ * Same as d_path() except it doesn't stick "(unreachable)" in front
+ * of unreachable paths.
+ */
+static char *d_path_compat(struct path *path, char *buf, int buflen)
+{
+	char *res;
+	struct path root;
+	struct path tmp;
+
+	read_lock(&current->fs->lock);
+	root = current->fs->root;
+	path_get(&root);
+	read_unlock(&current->fs->lock);
+	spin_lock(&dcache_lock);
+	tmp = root;
+	res = __d_path(path, &tmp, buf, buflen);
+	if (!IS_ERR(res) &&
+	    (tmp.mnt != root.mnt || tmp.dentry != root.dentry)) {
+		/*
+		 * Unreachable path found, redo with the global root
+		 * so we get a normal looking path.
+		 */
+		res = __d_path(path, &tmp, buf, buflen);
+	}
+	spin_unlock(&dcache_lock);
+	path_put(&root);
+
+	return res;
+}
+
+/*
+ * Some old programs break if /proc/mounts contains a mountpoint
+ * beginning with "(unreachable)".  Revert this back to the old way of
+ * displaying the path from the global root instead.
+ */
+static int show_path_old(struct seq_file *m, struct path *path, char *esc)
+{
+	char *buf;
+	size_t size = seq_get_buf(m, &buf);
+	int res = -1;
+
+	if (size) {
+		char *p = d_path_compat(path, buf, size);
+		if (!IS_ERR(p)) {
+			char *end = mangle_path(buf, p, esc);
+			if (end)
+				res = end - buf;
+		}
+	}
+	seq_commit(m, res);
+
+	return res;
+}
+
 static int show_vfsmnt(struct seq_file *m, void *v)
 {
 	struct vfsmount *mnt = list_entry(v, struct vfsmount, mnt_list);
@@ -797,7 +852,7 @@ static int show_vfsmnt(struct seq_file *
 
 	mangle(m, mnt->mnt_devname ? mnt->mnt_devname : "none");
 	seq_putc(m, ' ');
-	seq_path(m, &mnt_path, " \t\n\\");
+	show_path_old(m, &mnt_path, " \t\n\\");
 	seq_putc(m, ' ');
 	show_type(m, mnt->mnt_sb);
 	seq_puts(m, __mnt_is_readonly(mnt) ? " ro" : " rw");

Re: [patch 1/2] vfs: seq_file: add helpers for data filling

[Andrew Morton]

On Mon, 07 Sep 2009 10:38:24 +0200
Miklos Szeredi <miklos@szeredi.hu> wrote:

> Add two helpers that allow access to the seq_file's own buffer, but
> hides the internal details of seq_files.
> 
> This allows easier implementation of special purpose filling
> functions.  It also cleans up some existing functions which duplicated
> the seq_file logic.

This patch conflicts with
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch,
below.

I reworked your patch so that it removes the effects of
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch
and so that your patch is staged after
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch.

Your patch appears to fix the same thing as
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch,
so I _could_ have just dropped
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch. 
However it's unclear to me at thsi stage that your patch will be
merged.


I have a mountain of VFS patches piled up here:

vfs-fix-vfs_rename_dir-for-fs_rename_does_d_move-filesystems.patch
raw-fix-rawctl-compat-ioctls-breakage-on-amd64-and-itanic.patch
vfs-improve-comment-describing-fget_light.patch
libfs-make-simple_read_from_buffer-conventional.patch
fs-inodec-add-dev-id-and-inode-number-for-debugging-in-init_special_inode.patch
vfs-split-generic_forget_inode-so-that-hugetlbfs-does-not-have-to-copy-it.patch
fs-fix-overflow-in-sys_mount-for-in-kernel-calls.patch
vfs-optimization-for-touch_atime.patch
vfs-optimize-touch_time-too.patch
vfs-optimize-touch_time-too-fix.patch
ecryptfs-another-lockdep-issue.patch
vfs-explicitly-cast-s_maxbytes-in-fiemap_check_ranges.patch
vfs-change-sb-s_maxbytes-to-a-loff_t.patch
vfs-remove-redundant-position-check-in-do_sendfile.patch
#vfs-fix-d_path-for-unreachable-paths.patch: Valdis.Kletnieks@vt.edu issues
fs-remove-unneeded-dcache_unhashed-tricks.patch
fs-improve-remountro-vs-buffercache-coherency.patch
fs-improve-remountro-vs-buffercache-coherency-fix.patch
#
#
#fs-new-truncate-helpers.patch and friends: TBU?
fs-new-truncate-helpers.patch
fs-use-new-truncate-helpers.patch
fs-introduce-new-truncate-sequence.patch
fs-convert-simple-fs-to-new-truncate.patch
tmpfs-convert-to-use-the-new-truncate-convention.patch
#ext2-convert-to-use-the-new-truncate-convention.patch: Jan wanted update. Nick agreed
ext2-convert-to-use-the-new-truncate-convention.patch
ext2-convert-to-use-the-new-truncate-convention-fix.patch
fat-convert-to-use-the-new-truncate-convention.patch
btrfs-convert-to-use-the-new-truncate-convention.patch
jfs-convert-to-use-the-new-truncate-convention.patch
udf-convert-to-use-the-new-truncate-convention.patch
minix-convert-to-use-the-new-truncate-convention.patch
#
seq_file-return-a-negative-error-code-when-seq_path_root-fails.patch
vfs-seq_file-add-helpers-for-data-filling.patch
vfs-revert-proc-mounts-to-old-behavior-for-unreachable-mountpoints.patch


We're going to have to work out what to do if viro remains disappeared. I
guess I'll just send them all out to you guys to review during the merge
window and we'll go through them.





From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>

seq_path_root() is returning a return value of successful __d_path()
instead of returning a negative value when mangle_path() failed.

This is not a bug so far because nobody is using return value of
seq_path_root().

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/seq_file.c |    1 +
 1 file changed, 1 insertion(+)

diff -puN fs/seq_file.c~seq_file-return-a-negative-error-code-when-seq_path_root-fails fs/seq_file.c
--- a/fs/seq_file.c~seq_file-return-a-negative-error-code-when-seq_path_root-fails
+++ a/fs/seq_file.c
@@ -470,6 +470,7 @@ int seq_path_root(struct seq_file *m, st
 				m->count = s - m->buf;
 				return 0;
 			}
+			err = -ENAMETOOLONG;
 		}
 	}
 	m->count = m->size;
_