proc: Fix proc_kill_inodes to kill dentries on all proc superblocks

PATCH RESEND In review Eric W. Biederman 4 participants 5 messages Nov 1, 2007 Proc filesystem

Share & tools

Eric W. Biederman

author

2007-11-01 15:39 UTC #0 lore raw

PATCH RESEND 1/1 proc: Fix proc_kill_inodes to kill dentries on all proc superblocks

It appears we overlooked support for removing generic proc files when we added support for multiple proc super blocks. Handle that now.

Signed-off-by Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>

---
 fs/proc/generic.c  |   38 +++++++++++++++++++++-----------------
 fs/proc/internal.h |    2 ++
 fs/proc/root.c     |    2 +-
 3 files changed, 24 insertions(+), 18 deletions(-)

fs/proc/generic.c +21 ~~-17~~

index 1bdb624..3906770 100644
@@ -561,28 +561,32 @@ static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp
 static void proc_kill_inodes(struct proc_dir_entry *de)
 {
 	struct list_head *p;
-	struct super_block *sb = proc_mnt->mnt_sb;
+	struct super_block *sb;
 
 	/*
 	 * Actually it's a partial revoke().
 	 */
-	file_list_lock();
-	list_for_each(p, &sb->s_files) {
-		struct file * filp = list_entry(p, struct file, f_u.fu_list);
-		struct dentry * dentry = filp->f_path.dentry;
-		struct inode * inode;
-		const struct file_operations *fops;
-
-		if (dentry->d_op != &proc_dentry_operations)
-			continue;
-		inode = dentry->d_inode;
-		if (PDE(inode) != de)
-			continue;
-		fops = filp->f_op;
-		filp->f_op = NULL;
-		fops_put(fops);
+	spin_lock(&sb_lock);
+	list_for_each_entry(sb, &proc_fs_type.fs_supers, s_instances) {
+		file_list_lock();
+		list_for_each(p, &sb->s_files) {
+			struct file * filp = list_entry(p, struct file, f_u.fu_list);
+			struct dentry * dentry = filp->f_path.dentry;
+			struct inode * inode;
+			const struct file_operations *fops;
+
+			if (dentry->d_op != &proc_dentry_operations)
+				continue;
+			inode = dentry->d_inode;
+			if (PDE(inode) != de)
+				continue;
+			fops = filp->f_op;
+			filp->f_op = NULL;
+			fops_put(fops);
+		}
+		file_list_unlock();
 	}
-	file_list_unlock();
+	spin_unlock(&sb_lock);
 }
 
 static struct proc_dir_entry *proc_create(struct proc_dir_entry **parent,

fs/proc/internal.h +2 -0

index 1820eb2..1b2b6c6 100644
@@ -78,3 +78,5 @@ static inline int proc_fd(struct inode *inode)
 {
 	return PROC_I(inode)->fd;
 }
+
+extern struct file_system_type proc_fs_type;

fs/proc/root.c +1 -1

index ec9cb3b..1f86bb8 100644
@@ -98,7 +98,7 @@ static void proc_kill_sb(struct super_block *sb)
 	put_pid_ns(ns);
 }
 
-static struct file_system_type proc_fs_type = {
+struct file_system_type proc_fs_type = {
 	.name		= "proc",
 	.get_sb		= proc_get_sb,
 	.kill_sb	= proc_kill_sb,

Signature · 1 line

1.5.3.rc6.17.g1911

Pavel Emelyanov

Acked-by on v1 1/1

2007-11-01 15:48 UTC #1 lore raw

Quoted 5 lines · Eric W. Biederman

It appears we overlooked support for removing generic proc files when we added support for multiple proc super blocks. Handle that now.
Signed-off-by Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>

AFAIS this is just making the kill for all the super blocks we have.

Acked-by Pavel Emelyanov <[email protected]>

Quoted 83 lines

---
 fs/proc/generic.c  |   38 +++++++++++++++++++++-----------------
 fs/proc/internal.h |    2 ++
 fs/proc/root.c     |    2 +-
 3 files changed, 24 insertions(+), 18 deletions(-)

fs/proc/generic.c +21 ~~-17~~

index 1bdb624..3906770 100644
@@ -561,28 +561,32 @@ static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp
 static void proc_kill_inodes(struct proc_dir_entry *de)
 {
 	struct list_head *p;
-	struct super_block *sb = proc_mnt->mnt_sb;
+	struct super_block *sb;
 
 	/*
 	 * Actually it's a partial revoke().
 	 */
-	file_list_lock();
-	list_for_each(p, &sb->s_files) {
-		struct file * filp = list_entry(p, struct file, f_u.fu_list);
-		struct dentry * dentry = filp->f_path.dentry;
-		struct inode * inode;
-		const struct file_operations *fops;
-
-		if (dentry->d_op != &proc_dentry_operations)
-			continue;
-		inode = dentry->d_inode;
-		if (PDE(inode) != de)
-			continue;
-		fops = filp->f_op;
-		filp->f_op = NULL;
-		fops_put(fops);
+	spin_lock(&sb_lock);
+	list_for_each_entry(sb, &proc_fs_type.fs_supers, s_instances) {
+		file_list_lock();
+		list_for_each(p, &sb->s_files) {
+			struct file * filp = list_entry(p, struct file, f_u.fu_list);
+			struct dentry * dentry = filp->f_path.dentry;
+			struct inode * inode;
+			const struct file_operations *fops;
+
+			if (dentry->d_op != &proc_dentry_operations)
+				continue;
+			inode = dentry->d_inode;
+			if (PDE(inode) != de)
+				continue;
+			fops = filp->f_op;
+			filp->f_op = NULL;
+			fops_put(fops);
+		}
+		file_list_unlock();
 	}
-	file_list_unlock();
+	spin_unlock(&sb_lock);
 }
 
 static struct proc_dir_entry *proc_create(struct proc_dir_entry **parent,

fs/proc/internal.h +2 -0

index 1820eb2..1b2b6c6 100644
@@ -78,3 +78,5 @@ static inline int proc_fd(struct inode *inode)
 {
 	return PROC_I(inode)->fd;
 }
+
+extern struct file_system_type proc_fs_type;

fs/proc/root.c +1 -1

index ec9cb3b..1f86bb8 100644
@@ -98,7 +98,7 @@ static void proc_kill_sb(struct super_block *sb)
 	put_pid_ns(ns);
 }
 
-static struct file_system_type proc_fs_type = {
+struct file_system_type proc_fs_type = {
 	.name		= "proc",
 	.get_sb		= proc_get_sb,
 	.kill_sb	= proc_kill_sb,

Alexey Dobriyan

2007-11-01 15:59 UTC #2 lore raw

Quoted 11 lines · Pavel Emelyanov

Quoted 5 lines · Eric W. Biederman
It appears we overlooked support for removing generic proc files when we added support for multiple proc super blocks. Handle that now.
Signed-off-by Eric W. Biederman <[email protected]>
AFAIS this is just making the kill for all the super blocks we have.
Acked-by Pavel Emelyanov <[email protected]>

bzzrt, this function is executed only for directories and symlinks, obviously buggy (you can't flip ->f_op on live file) and is going away after readdir/rmmod races for directories will be dealt the same way as for regular file.

Quoted 42 lines

fs/proc/generic.c +16 ~~-16~~

@@ -561,28 +561,32 @@ static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp
 static void proc_kill_inodes(struct proc_dir_entry *de)
 {
 	struct list_head *p;
-	struct super_block *sb = proc_mnt->mnt_sb;
+	struct super_block *sb;
 
 	/*
 	 * Actually it's a partial revoke().
 	 */
-	file_list_lock();
-	list_for_each(p, &sb->s_files) {
-		struct file * filp = list_entry(p, struct file, f_u.fu_list);
-		struct dentry * dentry = filp->f_path.dentry;
-		struct inode * inode;
-		const struct file_operations *fops;
-
-		if (dentry->d_op != &proc_dentry_operations)
-			continue;
-		inode = dentry->d_inode;
-		if (PDE(inode) != de)
-			continue;
-		fops = filp->f_op;
-		filp->f_op = NULL;
-		fops_put(fops);
+	spin_lock(&sb_lock);
+	list_for_each_entry(sb, &proc_fs_type.fs_supers, s_instances) {
+		file_list_lock();
+		list_for_each(p, &sb->s_files) {
+			struct file * filp = list_entry(p, struct file, f_u.fu_list);
+			struct dentry * dentry = filp->f_path.dentry;
+			struct inode * inode;
+			const struct file_operations *fops;
+
+			if (dentry->d_op != &proc_dentry_operations)
+				continue;
+			inode = dentry->d_inode;
+			if (PDE(inode) != de)
+				continue;
+			fops = filp->f_op;

Quoted 8 lines

fs/proc/generic.c static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp · line 590
Quoted 8 lines
fs/proc/generic.c +5 -1
+			filp->f_op = NULL;
+			fops_put(fops);
+		}
+		file_list_unlock();
 	}
-	file_list_unlock();
+	spin_unlock(&sb_lock);
 }

Yup, all this bogo-revoke is going away RSN. you just kicked my lazy ass.

Eric W. Biederman

author

2007-11-01 17:07 UTC #3 lore raw

Quoted 1 line · Alexey Dobriyan

Yup, all this bogo-revoke is going away RSN. you just kicked my lazy ass.

As long as the issue is fixed I don't care.

Eric

[email protected]

Acked-by

2007-11-01 19:57 UTC #4 lore raw

In reply to Pavel Emelyanov

Pavel Emelianov [[email protected]] wrote:

| Eric W. Biederman wrote:
| > It appears we overlooked support for removing generic proc files
| > when we added support for multiple proc super blocks.  Handle
| > that now.
| > 
| > Signed-off-by: Eric W. Biederman <[email protected]>
| 
| AFAIS this is just making the kill for all the super blocks
| we have.
| 
| Acked-by: Pavel Emelyanov <[email protected]>

Acked-by Sukadev Bhattiprolu <[email protected]>

| 
| > ---
| >  fs/proc/generic.c  |   38 +++++++++++++++++++++-----------------
| >  fs/proc/internal.h |    2 ++
| >  fs/proc/root.c     |    2 +-
| >  3 files changed, 24 insertions(+), 18 deletions(-)
| > 
| > diff --git a/fs/proc/generic.c b/fs/proc/generic.c
| > index 1bdb624..3906770 100644
| > --- a/fs/proc/generic.c
| > +++ b/fs/proc/generic.c
| > @@ -561,28 +561,32 @@ static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry * dp
| >  static void proc_kill_inodes(struct proc_dir_entry *de)
| >  {
| >  	struct list_head *p;
| > -	struct super_block *sb = proc_mnt->mnt_sb;
| > +	struct super_block *sb;
| >  
| >  	/*
| >  	 * Actually it's a partial revoke().
| >  	 */
| > -	file_list_lock();
| > -	list_for_each(p, &sb->s_files) {
| > -		struct file * filp = list_entry(p, struct file, f_u.fu_list);
| > -		struct dentry * dentry = filp->f_path.dentry;
| > -		struct inode * inode;
| > -		const struct file_operations *fops;
| > -
| > -		if (dentry->d_op != &proc_dentry_operations)
| > -			continue;
| > -		inode = dentry->d_inode;
| > -		if (PDE(inode) != de)
| > -			continue;
| > -		fops = filp->f_op;
| > -		filp->f_op = NULL;
| > -		fops_put(fops);
| > +	spin_lock(&sb_lock);
| > +	list_for_each_entry(sb, &proc_fs_type.fs_supers, s_instances) {
| > +		file_list_lock();
| > +		list_for_each(p, &sb->s_files) {
| > +			struct file * filp = list_entry(p, struct file, f_u.fu_list);
| > +			struct dentry * dentry = filp->f_path.dentry;
| > +			struct inode * inode;
| > +			const struct file_operations *fops;
| > +
| > +			if (dentry->d_op != &proc_dentry_operations)
| > +				continue;
| > +			inode = dentry->d_inode;
| > +			if (PDE(inode) != de)
| > +				continue;
| > +			fops = filp->f_op;
| > +			filp->f_op = NULL;
| > +			fops_put(fops);
| > +		}
| > +		file_list_unlock();
| >  	}
| > -	file_list_unlock();
| > +	spin_unlock(&sb_lock);
| >  }
| >  
| >  static struct proc_dir_entry *proc_create(struct proc_dir_entry **parent,
| > diff --git a/fs/proc/internal.h b/fs/proc/internal.h
| > index 1820eb2..1b2b6c6 100644
| > --- a/fs/proc/internal.h
| > +++ b/fs/proc/internal.h
| > @@ -78,3 +78,5 @@ static inline int proc_fd(struct inode *inode)
| >  {
| >  	return PROC_I(inode)->fd;
| >  }
| > +
| > +extern struct file_system_type proc_fs_type;
| > diff --git a/fs/proc/root.c b/fs/proc/root.c
| > index ec9cb3b..1f86bb8 100644
| > --- a/fs/proc/root.c
| > +++ b/fs/proc/root.c
| > @@ -98,7 +98,7 @@ static void proc_kill_sb(struct super_block *sb)
| >  	put_pid_ns(ns);
| >  }
| >  
| > -static struct file_system_type proc_fs_type = {
| > +struct file_system_type proc_fs_type = {
| >  	.name		= "proc",
| >  	.get_sb		= proc_get_sb,
| >  	.kill_sb	= proc_kill_sb,