vfs: make mounts and mountstats honor root dir like mountinfo does

Date: Wed, 17 Oct 2012 20:29:55 +0400

Change show_vfsmnt() and show_vfsstat() to show mountpoints relative to the root directory and skip mountpoints outside of chroot jail the same way as show_mountinfo() does.

---
 fs/proc_namespace.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

index 789e8d1..0f96f71 100644
@@ -91,6 +91,7 @@ static void show_type(struct seq_file *m, struct super_block *sb)
 
 static int show_vfsmnt(struct seq_file *m, struct vfsmount *mnt)
 {
+	struct proc_mounts *p = proc_mounts(m);
 	struct mount *r = real_mount(mnt);
 	int err = 0;
 	struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };

@@ -104,7 +105,10 @@ static int show_vfsmnt(struct seq_file *m, struct vfsmount *mnt)
 		mangle(m, r->mnt_devname ? r->mnt_devname : "none");
 	}
 	seq_putc(m, ' ');
-	seq_path(m, &mnt_path, " \t\n\\");
+	/* mountpoints outside of chroot jail will give SEQ_SKIP on this */
+	err = seq_path_root(m, &mnt_path, &p->root, " \t\n\\");
+	if (err)
+		goto out;
 	seq_putc(m, ' ');
 	show_type(m, sb);
 	seq_puts(m, __mnt_is_readonly(mnt) ? " ro" : " rw");

@@ -181,6 +185,7 @@ out:
 
 static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 {
+	struct proc_mounts *p = proc_mounts(m);
 	struct mount *r = real_mount(mnt);
 	struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };
 	struct super_block *sb = mnt_path.dentry->d_sb;

@@ -200,7 +205,10 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 
 	/* mount point */
 	seq_puts(m, " mounted on ");
-	seq_path(m, &mnt_path, " \t\n\\");
+	/* mountpoints outside of chroot jail will give SEQ_SKIP on this */
+	err = seq_path_root(m, &mnt_path, &p->root, " \t\n\\");
+	if (err)
+		goto out;
 	seq_putc(m, ' ');
 
 	/* file system type */

@@ -215,6 +223,7 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	}
 
 	seq_putc(m, '\n');
+out:
 	return err;
 }
 

ldv

Sigh... Repeat after me: We Do Not Break Userland. Care to explain why that change won't do just that?

This is definitely an information leak, and I cannot imagine how these unaccessible entries listed in /proc/self/mounts and /proc/self/mountstats could be used sanely.

ldv

How do you think any existing script running chrooted is going to parse /proc/mounts? One obvious way is to filter on known prefix and strip it away. With your change it will instantly break. Moreover, any script working with *new* semantics will break as soon as you boot an older kernel.

Existing behaviour had been there since mid-90s (at least) and I'm absolutely certain that such scripts do exist. I know that, since I had to write such. Again, we do not break userland. Especially since the behaviour you want is trivial to obtain - just create a new namespace instead of messing with chroot. Then, in that namespace, pivot_root the sucker to root and unmount everything else. Voila.

NAK. And you really need to realize that changing ABI in a way that breaks real-world userland code is not acceptable. Even when the new semantics is nicer. And when you cannot imagine how somebody could possibly write code that would depend on the old one.

If a script relies on this behaviour, it is likely to be broken already because the kernel strips chroot prefix somehow (I've just checked back to 2.6.32). The main purpose of this patch is to exclude from listing those mount points that do not belong to the chroot at all.

I'm very curious, what could your scripts do with paths outside chroot, assuming that prefix removal was done by the kernel?

Occasionally, we do. If you say there was no chroot prefix removal in the kernel before, and there seems to be a prefix removal for at least 5 years, then there must have been a break some years ago that went unnoticed.

ldv

D'oh! Yes, we did. And yes, it was quite a few years ago, actually - back in 2000. I plead being low on sleep and lower on caffeine...

For a moment I was even afraid that it was my own doing, but no - it came in https://lkml.org/lkml/2000/1/16/54. Werner Almesberger. IIRC, I hadn't realized that it was going to cause fun problems back then - basically, by that point 2.3 and 2.2 had already diverged a lot and quite a few things got written off as "oh, well - upgrade from 2.2 to 2.4 is going to be not far from building the system from scratch anyway, and nobody sane would be using 2.3 on anything other than scratch filesystem - IDE broken more often than not, memory corruption aplenty, etc."

My apologies. Hmm... Filtering the out-of-root ones out is probably OK, but let's put that into -next after 3.18-rc1 and see if anyone yells.

Thanks, I won't be able to locate the change that fast.

It also means that the phrase "show mountpoints relative to the root directory" I used in the commit message is irrelevant and should be removed to avoid confusion.

BTW, the cleanup patch (vfs: cleanup show_mountinfo) could be safely applied anytime - it's completely harmless.

ldv

Date: Wed, 17 Oct 2012 20:29:36 +0400

Starting with commit v3.2-rc4-1-g02125a8, seq_path_root() no longer changes the value of its "struct path *root" argument. Starting with commit v3.2-rc7-104-g8c9379e, the "struct path *root" argument of seq_path_root() is const.
As result, the temporary variable "root" in show_mountinfo() that holds a copy of struct path root is no longer needed.

---
 fs/proc_namespace.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 73ca174..789e8d1 100644
@@ -125,7 +125,6 @@ static int show_mountinfo(struct seq_file *m, struct vfsmount *mnt)
 	struct mount *r = real_mount(mnt);
 	struct super_block *sb = mnt->mnt_sb;
 	struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };
-	struct path root = p->root;
 	int err = 0;
 
 	seq_printf(m, "%i %i %u:%u ", r->mnt_id, r->mnt_parent->mnt_id,

@@ -139,7 +138,7 @@ static int show_mountinfo(struct seq_file *m, struct vfsmount *mnt)
 	seq_putc(m, ' ');
 
 	/* mountpoints outside of chroot jail will give SEQ_SKIP on this */
-	err = seq_path_root(m, &mnt_path, &root, " \t\n\\");
+	err = seq_path_root(m, &mnt_path, &p->root, " \t\n\\");
 	if (err)
 		goto out;
 

ldv

As we already show mountpoints relative to the root directory, thanks to the change made back in 2000, change show_vfsmnt() and show_vfsstat() to skip out-of-root mountpoints the same way as show_mountinfo() does.

---
 fs/proc_namespace.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

index 789e8d1..0f96f71 100644
@@ -91,6 +91,7 @@ static void show_type(struct seq_file *m, struct super_block *sb)
 
 static int show_vfsmnt(struct seq_file *m, struct vfsmount *mnt)
 {
+	struct proc_mounts *p = proc_mounts(m);
 	struct mount *r = real_mount(mnt);
 	int err = 0;
 	struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };

@@ -104,7 +105,10 @@ static int show_vfsmnt(struct seq_file *m, struct vfsmount *mnt)
 		mangle(m, r->mnt_devname ? r->mnt_devname : "none");
 	}
 	seq_putc(m, ' ');
-	seq_path(m, &mnt_path, " \t\n\\");
+	/* mountpoints outside of chroot jail will give SEQ_SKIP on this */
+	err = seq_path_root(m, &mnt_path, &p->root, " \t\n\\");
+	if (err)
+		goto out;
 	seq_putc(m, ' ');
 	show_type(m, sb);
 	seq_puts(m, __mnt_is_readonly(mnt) ? " ro" : " rw");

@@ -181,6 +185,7 @@ out:
 
 static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 {
+	struct proc_mounts *p = proc_mounts(m);
 	struct mount *r = real_mount(mnt);
 	struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };
 	struct super_block *sb = mnt_path.dentry->d_sb;

@@ -200,7 +205,10 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 
 	/* mount point */
 	seq_puts(m, " mounted on ");
-	seq_path(m, &mnt_path, " \t\n\\");
+	/* mountpoints outside of chroot jail will give SEQ_SKIP on this */
+	err = seq_path_root(m, &mnt_path, &p->root, " \t\n\\");
+	if (err)
+		goto out;
 	seq_putc(m, ' ');
 
 	/* file system type */

@@ -215,6 +223,7 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	}
 
 	seq_putc(m, '\n');
+out:
 	return err;
 }
 

ldv