Skip to contentSkip to conversation

vfs: show_vfsstat: do not ignore errors from show_devname method

PATCH Superseded Dmitry V. Levin 3 participants 17 messages Mar 19, 2015 – Mar 16, 2016 VFS

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 vfs: show_vfsstat: do not ignore errors from show_devname method

Explicitly check show_devname method return code and bail out in case of an error. This fixes regression introduced by commit 9d4d65748a5c.

---
 fs/proc_namespace.c | 2 ++
 1 file changed, 2 insertions(+)
fs/proc_namespace.c +2 -0
index 8db932d..a5644c4 100644
@@ -196,17 +196,19 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	if (sb->s_op->show_devname) {
 		seq_puts(m, "device ");
 		err = sb->s_op->show_devname(m, mnt_path.dentry);
+		if (err)
+			goto out;
 	} else {
 		if (r->mnt_devname) {
 			seq_puts(m, "device ");
 			mangle(m, r->mnt_devname);
 		} else
 			seq_puts(m, "no device");
 	}
 
 	/* mount point */
 	seq_puts(m, " mounted on ");
 	/* 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;
Signature · 1 line
ldv
#1 lore raw

PATCH RESEND 1/1 vfs: show_vfsstat: do not ignore errors from show_devname method

Date: Thu, 19 Mar 2015 11:10:54 +0000

Explicitly check show_devname method return code and bail out in case of an error. This fixes regression introduced by commit 9d4d65748a5c.

---
 fs/proc_namespace.c | 2 ++
 1 file changed, 2 insertions(+)
fs/proc_namespace.c +2 -0
index 8ebd9a3..876459559 100644
@@ -197,17 +197,19 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	if (sb->s_op->show_devname) {
 		seq_puts(m, "device ");
 		err = sb->s_op->show_devname(m, mnt_path.dentry);
+		if (err)
+			goto out;
 	} else {
 		if (r->mnt_devname) {
 			seq_puts(m, "device ");
 			mangle(m, r->mnt_devname);
 		} else
 			seq_puts(m, "no device");
 	}
 
 	/* mount point */
 	seq_puts(m, " mounted on ");
 	/* 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;
Signature · 1 line
ldv
#2 lore raw
Quoted 25 lines · Dmitry V. Levin

Date: Thu, 19 Mar 2015 11:10:54 +0000

Explicitly check show_devname method return code and bail out in case of an error. This fixes regression introduced by commit 9d4d65748a5c.

---
 fs/proc_namespace.c | 2 ++
 1 file changed, 2 insertions(+)
fs/proc_namespace.c +2 -0
index 8ebd9a3..876459559 100644
@@ -197,17 +197,19 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	if (sb->s_op->show_devname) {
 		seq_puts(m, "device ");
 		err = sb->s_op->show_devname(m, mnt_path.dentry);
+		if (err)
+			goto out;
 	} else {
 		if (r->mnt_devname) {
 			seq_puts(m, "device ");
 			mangle(m, r->mnt_devname);

fs/proc_namespace.c static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt) · line 215

Quoted 10 lines
fs/proc_namespace.c +0 -0
 		} else
 			seq_puts(m, "no device");
 	}
 
 	/* mount point */
 	seq_puts(m, " mounted on ");
 	/* 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;

I would suggest this should also remove now spurious initialization of err to 0 and now always-true !err check prior to calling ->show_stats.

Signature · 1 line
Mateusz Guzik
#3 lore raw

PATCH 1/3 vfs: show_vfsmnt: remove redundant initialization of error code

As err variable is now always checked right after the first assignment, its initialization is redundant and could be safely removed.

---
 fs/proc_namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/proc_namespace.c +1 -1
index 876459559..cbc9c27 100644
@@ -95,9 +95,9 @@ static int show_vfsmnt(struct seq_file *m, struct vfsmount *mnt)
 {
 	struct proc_mounts *p = m->private;
 	struct mount *r = real_mount(mnt);
-	int err = 0;
 	struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };
 	struct super_block *sb = mnt_path.dentry->d_sb;
+	int err;
 
 	if (sb->s_op->show_devname) {
 		err = sb->s_op->show_devname(m, mnt_path.dentry);
Signature · 1 line
ldv
#4 lore raw

PATCH RESEND 1/3 vfs: show_vfsmnt: remove redundant initialization of error code

Date: Wed Nov 18 21:07:10 2015 +0000

As err variable is now always checked right after the first assignment, its initialization is redundant and could be safely removed.

---
 fs/proc_namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/proc_namespace.c +1 -1
index 876459559..cbc9c27 100644
@@ -95,9 +95,9 @@ static int show_vfsmnt(struct seq_file *m, struct vfsmount *mnt)
 {
 	struct proc_mounts *p = m->private;
 	struct mount *r = real_mount(mnt);
-	int err = 0;
 	struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };
 	struct super_block *sb = mnt_path.dentry->d_sb;
+	int err;
 
 	if (sb->s_op->show_devname) {
 		err = sb->s_op->show_devname(m, mnt_path.dentry);
Signature · 1 line
ldv
#5 lore raw

PATCH 2/3 vfs: show_mountinfo: cleanup error code checks

In reply to Mateusz Guzik

Check err variable right after each assignment. This change makes initialization of err redundant, so remove the initialization.

---
 fs/proc_namespace.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)
fs/proc_namespace.c +11 -9
index cbc9c27..7a6b2f3 100644
@@ -131,16 +131,17 @@ 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 };
-	int err = 0;
+	int err;
 
 	seq_printf(m, "%i %i %u:%u ", r->mnt_id, r->mnt_parent->mnt_id,
 		   MAJOR(sb->s_dev), MINOR(sb->s_dev));
-	if (sb->s_op->show_path)
+	if (sb->s_op->show_path) {
 		err = sb->s_op->show_path(m, mnt->mnt_root);
-	else
+		if (err)
+			goto out;
+	} else {
 		seq_dentry(m, mnt->mnt_root, " \t\n\\");
-	if (err)
-		goto out;
+	}
 	seq_putc(m, ' ');
 
 	/* mountpoints outside of chroot jail will give SEQ_SKIP on this */
@@ -168,12 +169,13 @@ static int show_mountinfo(struct seq_file *m, struct vfsmount *mnt)
 	seq_puts(m, " - ");
 	show_type(m, sb);
 	seq_putc(m, ' ');
-	if (sb->s_op->show_devname)
+	if (sb->s_op->show_devname) {
 		err = sb->s_op->show_devname(m, mnt->mnt_root);
-	else
+		if (err)
+			goto out;
+	} else {
 		mangle(m, r->mnt_devname ? r->mnt_devname : "none");
-	if (err)
-		goto out;
+	}
 	seq_puts(m, sb->s_flags & MS_RDONLY ? " ro" : " rw");
 	err = show_sb_opts(m, sb);
 	if (err)
Signature · 1 line
ldv
#6 lore raw

PATCH RESEND 2/3 vfs: show_mountinfo: cleanup error code checks

Date: Wed Nov 18 21:08:33 2015 +0000

Check err variable right after each assignment. This change makes initialization of err redundant, so remove the initialization.

---
 fs/proc_namespace.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)
fs/proc_namespace.c +11 -9
index cbc9c27..7a6b2f3 100644
@@ -131,16 +131,17 @@ 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 };
-	int err = 0;
+	int err;
 
 	seq_printf(m, "%i %i %u:%u ", r->mnt_id, r->mnt_parent->mnt_id,
 		   MAJOR(sb->s_dev), MINOR(sb->s_dev));
-	if (sb->s_op->show_path)
+	if (sb->s_op->show_path) {
 		err = sb->s_op->show_path(m, mnt->mnt_root);
-	else
+		if (err)
+			goto out;
+	} else {
 		seq_dentry(m, mnt->mnt_root, " \t\n\\");
-	if (err)
-		goto out;
+	}
 	seq_putc(m, ' ');
 
 	/* mountpoints outside of chroot jail will give SEQ_SKIP on this */
@@ -168,12 +169,13 @@ static int show_mountinfo(struct seq_file *m, struct vfsmount *mnt)
 	seq_puts(m, " - ");
 	show_type(m, sb);
 	seq_putc(m, ' ');
-	if (sb->s_op->show_devname)
+	if (sb->s_op->show_devname) {
 		err = sb->s_op->show_devname(m, mnt->mnt_root);
-	else
+		if (err)
+			goto out;
+	} else {
 		mangle(m, r->mnt_devname ? r->mnt_devname : "none");
-	if (err)
-		goto out;
+	}
 	seq_puts(m, sb->s_flags & MS_RDONLY ? " ro" : " rw");
 	err = show_sb_opts(m, sb);
 	if (err)
Signature · 1 line
ldv
#7 lore raw

PATCH 3/3 vfs: show_vfsstat: remove redundant initialization and check of error code

In reply to Mateusz Guzik

As err variable is now always checked right after each assignment, its initialization is redundant and could be safely removed. For the same reason, the last check of err is also redundant and could be removed as well.

---
 fs/proc_namespace.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
fs/proc_namespace.c +2 -3
index 7a6b2f3..3f1190d 100644
@@ -193,7 +193,7 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	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;
-	int err = 0;
+	int err;
 
 	/* device */
 	if (sb->s_op->show_devname) {
@@ -224,8 +224,7 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	/* optional statistics */
 	if (sb->s_op->show_stats) {
 		seq_putc(m, ' ');
-		if (!err)
-			err = sb->s_op->show_stats(m, mnt_path.dentry);
+		err = sb->s_op->show_stats(m, mnt_path.dentry);
 	}
 
 	seq_putc(m, '\n');
Signature · 1 line
ldv
#8 lore raw

PATCH RESEND 3/3 vfs: show_vfsstat: remove redundant initialization and check of error code

Date: Wed Nov 18 21:09:45 2015 +0000

As err variable is now always checked right after each assignment, its initialization is redundant and could be safely removed. For the same reason, the last check of err is also redundant and could be removed as well.

---
 fs/proc_namespace.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
fs/proc_namespace.c +2 -3
index 7a6b2f3..3f1190d 100644
@@ -193,7 +193,7 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	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;
-	int err = 0;
+	int err;
 
 	/* device */
 	if (sb->s_op->show_devname) {
@@ -224,8 +224,7 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	/* optional statistics */
 	if (sb->s_op->show_stats) {
 		seq_putc(m, ' ');
-		if (!err)
-			err = sb->s_op->show_stats(m, mnt_path.dentry);
+		err = sb->s_op->show_stats(m, mnt_path.dentry);
 	}
 
 	seq_putc(m, '\n');
Signature · 1 line
ldv
#9 lore raw

PATCH RESEND v2 1/4 vfs: show_vfsstat: do not ignore errors from show_devname method

In reply to Mateusz Guzik

Date: Thu, 19 Mar 2015 11:10:54 +0000

Explicitly check show_devname method return code and bail out in case of an error. This fixes regression introduced by commit 9d4d65748a5c.

---
 fs/proc_namespace.c | 2 ++
 1 file changed, 2 insertions(+)
fs/proc_namespace.c +2 -0
index 8ebd9a3..876459559 100644
@@ -197,17 +197,19 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	if (sb->s_op->show_devname) {
 		seq_puts(m, "device ");
 		err = sb->s_op->show_devname(m, mnt_path.dentry);
+		if (err)
+			goto out;
 	} else {
 		if (r->mnt_devname) {
 			seq_puts(m, "device ");
 			mangle(m, r->mnt_devname);
 		} else
 			seq_puts(m, "no device");
 	}
 
 	/* mount point */
 	seq_puts(m, " mounted on ");
 	/* 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;
Signature · 1 line
ldv
#10 lore raw

PATCH RESEND 2/4 vfs: show_vfsmnt: remove redundant initialization of error code

In reply to Mateusz Guzik

Date: Wed, 18 Nov 2015 21:07:10 +0000

As err variable is now always checked right after the first assignment, its initialization is redundant and could be safely removed.

---
 fs/proc_namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/proc_namespace.c +1 -1
index 876459559..cbc9c27 100644
@@ -95,9 +95,9 @@ static int show_vfsmnt(struct seq_file *m, struct vfsmount *mnt)
 {
 	struct proc_mounts *p = m->private;
 	struct mount *r = real_mount(mnt);
-	int err = 0;
 	struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt };
 	struct super_block *sb = mnt_path.dentry->d_sb;
+	int err;
 
 	if (sb->s_op->show_devname) {
 		err = sb->s_op->show_devname(m, mnt_path.dentry);
Signature · 1 line
ldv
#11 lore raw

PATCH RESEND 3/4 vfs: show_mountinfo: cleanup error code checks

In reply to Mateusz Guzik

Wed, 18 Nov 2015 21:08:33 +0000

Check err variable right after each assignment. This change makes initialization of err redundant, so remove the initialization.

---
 fs/proc_namespace.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)
fs/proc_namespace.c +11 -9
index cbc9c27..7a6b2f3 100644
@@ -131,16 +131,17 @@ 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 };
-	int err = 0;
+	int err;
 
 	seq_printf(m, "%i %i %u:%u ", r->mnt_id, r->mnt_parent->mnt_id,
 		   MAJOR(sb->s_dev), MINOR(sb->s_dev));
-	if (sb->s_op->show_path)
+	if (sb->s_op->show_path) {
 		err = sb->s_op->show_path(m, mnt->mnt_root);
-	else
+		if (err)
+			goto out;
+	} else {
 		seq_dentry(m, mnt->mnt_root, " \t\n\\");
-	if (err)
-		goto out;
+	}
 	seq_putc(m, ' ');
 
 	/* mountpoints outside of chroot jail will give SEQ_SKIP on this */
@@ -168,12 +169,13 @@ static int show_mountinfo(struct seq_file *m, struct vfsmount *mnt)
 	seq_puts(m, " - ");
 	show_type(m, sb);
 	seq_putc(m, ' ');
-	if (sb->s_op->show_devname)
+	if (sb->s_op->show_devname) {
 		err = sb->s_op->show_devname(m, mnt->mnt_root);
-	else
+		if (err)
+			goto out;
+	} else {
 		mangle(m, r->mnt_devname ? r->mnt_devname : "none");
-	if (err)
-		goto out;
+	}
 	seq_puts(m, sb->s_flags & MS_RDONLY ? " ro" : " rw");
 	err = show_sb_opts(m, sb);
 	if (err)
Signature · 1 line
ldv
#12 lore raw

PATCH RESEND 4/4 vfs: show_vfsstat: remove redundant initialization and check of error code

In reply to Mateusz Guzik

Date: Wed, 18 Nov 2015 21:09:45 +0000

As err variable is now always checked right after each assignment, its initialization is redundant and could be safely removed. For the same reason, the last check of err is also redundant and could be removed as well.

---
 fs/proc_namespace.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
fs/proc_namespace.c +2 -3
index 7a6b2f3..3f1190d 100644
@@ -193,7 +193,7 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	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;
-	int err = 0;
+	int err;
 
 	/* device */
 	if (sb->s_op->show_devname) {
@@ -224,8 +224,7 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	/* optional statistics */
 	if (sb->s_op->show_stats) {
 		seq_putc(m, ' ');
-		if (!err)
-			err = sb->s_op->show_stats(m, mnt_path.dentry);
+		err = sb->s_op->show_stats(m, mnt_path.dentry);
 	}
 
 	seq_putc(m, '\n');
Signature · 1 line
ldv
#13 lore raw

PATCH RESEND v2 1/1 vfs: show_vfsstat: do not ignore errors from show_devname method

In reply to Dmitry V. Levin

Date: Thu, 19 Mar 2015 11:10:54 +0000

Explicitly check show_devname method return code and bail out in case of an error. This fixes regression introduced by commit 9d4d65748a5c.

---
 fs/proc_namespace.c | 2 ++
 1 file changed, 2 insertions(+)
fs/proc_namespace.c +2 -0
index 8ebd9a3..876459559 100644
@@ -197,17 +197,19 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 	if (sb->s_op->show_devname) {
 		seq_puts(m, "device ");
 		err = sb->s_op->show_devname(m, mnt_path.dentry);
+		if (err)
+			goto out;
 	} else {
 		if (r->mnt_devname) {
 			seq_puts(m, "device ");
 			mangle(m, r->mnt_devname);
 		} else
 			seq_puts(m, "no device");
 	}
 
 	/* mount point */
 	seq_puts(m, " mounted on ");
 	/* 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;
Signature · 1 line
ldv
#14 lore raw

PATCH RESEND v3 1/1 vfs: show_vfsstat: do not ignore errors from show_devname method

Date: Thu, 19 Mar 2015 11:10:54 +0000

Explicitly check show_devname method return code and bail out in case of an error. This fixes regression introduced by commit 9d4d65748a5c.

---
 fs/proc_namespace.c | 2 ++
 1 file changed, 2 insertions(+)
fs/proc_namespace.c +2 -0
index 2256e7e..3f1190d 100644
@@ -190,26 +190,28 @@ out:
 static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 {
 	struct proc_mounts *p = m->private;
 	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;
 	int err;
 
 	/* device */
 	if (sb->s_op->show_devname) {
 		seq_puts(m, "device ");
 		err = sb->s_op->show_devname(m, mnt_path.dentry);
+		if (err)
+			goto out;
 	} else {
 		if (r->mnt_devname) {
 			seq_puts(m, "device ");
 			mangle(m, r->mnt_devname);
 		} else
 			seq_puts(m, "no device");
 	}
 
 	/* mount point */
 	seq_puts(m, " mounted on ");
 	/* 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;
Signature · 1 line
ldv
#15 lore raw

PATCH RESEND v4 1/1 vfs: show_vfsstat: do not ignore errors from show_devname method

Date: Thu, 19 Mar 2015 11:10:54 +0000

Explicitly check show_devname method return code and bail out in case of an error. This fixes regression introduced by commit 9d4d65748a5c.

---
 fs/proc_namespace.c | 2 ++
 1 file changed, 2 insertions(+)
fs/proc_namespace.c +2 -0
index 2256e7e..3f1190d 100644
@@ -190,26 +190,28 @@ out:
 static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
 {
 	struct proc_mounts *p = m->private;
 	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;
 	int err;
 
 	/* device */
 	if (sb->s_op->show_devname) {
 		seq_puts(m, "device ");
 		err = sb->s_op->show_devname(m, mnt_path.dentry);
+		if (err)
+			goto out;
 	} else {
 		if (r->mnt_devname) {
 			seq_puts(m, "device ");
 			mangle(m, r->mnt_devname);
 		} else
 			seq_puts(m, "no device");
 	}
 
 	/* mount point */
 	seq_puts(m, " mounted on ");
 	/* 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;
Signature · 1 line
ldv
maintainer
#16 lore raw
Quoted 4 lines · Dmitry V. Levin

Date: Thu, 19 Mar 2015 11:10:54 +0000

Explicitly check show_devname method return code and bail out in case of an error. This fixes regression introduced by commit 9d4d65748a5c.

Applied weeks ago, still not migrated into for-next; my apologies, thought I pushed it to Linus at some point last cycle. I'll take it to for-next in a few.