LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [GIT PULL -mm] Unionfs updates
@ 2007-03-05  1:54 Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 01/13] fs/unionfs: Fix a memory leak & null pointer dereference Josef 'Jeff' Sipek
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:54 UTC (permalink / raw)
  To: linux-kernel, akpm

The following patches (also available though the git tree) address a number
of code cleanliness issues with Unionfs.

You can pull from 'master' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jsipek/unionfs.git

to receive the following:

Erez Zadok (6):
      fs/unionfs: Fix a memory leak & null pointer dereference
      fs/unionfs/: Fix a memory leak in unionfs_read_super
      fs/unionfs/: Fix copyup_deleted_file dentry leak
      fs/unionfs/: mntput in __cleanup_dentry
      fs/unionfs/: Fix dentry leak in copyup_named_dentry
      fs/unionfs/: Fix unlocking in error paths

Josef 'Jeff' Sipek (7):
      fs/unionfs/: Don't grab dentry private data mutex in unionfs_d_release
      fs/unionfs/: Several small cleanups in unionfs_interpose
      fs/unionfs/: Rename unionfs_d_revalidate_wrap
      fs/unionfs/: Remove alloc_filldir_node
      fs/unionfs/: Use SEEK_{SET,CUR} instead of hardcoded values
      fs/unionfs/: Check return value of d_path
      fs/unionfs/: Miscellaneous coding style fixes

 fs/unionfs/commonfops.c |   22 ++++++++++++++++++----
 fs/unionfs/copyup.c     |   17 +++++++++++++----
 fs/unionfs/dentry.c     |   15 ++++-----------
 fs/unionfs/dirfops.c    |    6 +++---
 fs/unionfs/lookup.c     |   11 +++++++++--
 fs/unionfs/main.c       |   18 ++++++------------
 fs/unionfs/rdstate.c    |    8 +-------
 fs/unionfs/super.c      |    8 ++++++--
 fs/unionfs/union.h      |    2 +-
 9 files changed, 61 insertions(+), 46 deletions(-)

Josef 'Jeff' Sipek.

jsipek@cs.sunysb.edu



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/13] fs/unionfs: Fix a memory leak & null pointer dereference
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
@ 2007-03-05  1:54 ` Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 02/13] fs/unionfs/: Fix a memory leak in unionfs_read_super Josef 'Jeff' Sipek
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:54 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Erez Zadok, Josef 'Jeff' Sipek

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/commonfops.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 379c525..66d6ce9 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -230,7 +230,7 @@ int unionfs_file_revalidate(struct file *file, int willwrite)
 	unionfs_read_lock(sb);
 	if (!unionfs_d_revalidate(dentry, NULL) && !d_deleted(dentry)) {
 		err = -ESTALE;
-		goto out;
+		goto out_nofree;
 	}
 
 	sbgen = atomic_read(&UNIONFS_SB(sb)->generation);
@@ -286,6 +286,9 @@ int unionfs_file_revalidate(struct file *file, int willwrite)
 	}
 
 out:
+	if (err)
+		kfree(UNIONFS_F(file)->lower_files);
+out_nofree:
 	unionfs_unlock_dentry(dentry);
 	unionfs_read_unlock(dentry->d_sb);
 	return err;
@@ -391,7 +394,7 @@ int unionfs_open(struct inode *inode, struct file *file)
 	file->private_data = kzalloc(sizeof(struct unionfs_file_info), GFP_KERNEL);
 	if (!UNIONFS_F(file)) {
 		err = -ENOMEM;
-		goto out;
+		goto out_nofree;
 	}
 	fbstart(file) = -1;
 	fbend(file) = -1;
@@ -444,7 +447,7 @@ out:
 		kfree(UNIONFS_F(file)->lower_files);
 		kfree(UNIONFS_F(file));
 	}
-
+out_nofree:
 	return err;
 }
 
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 02/13] fs/unionfs/: Fix a memory leak in unionfs_read_super
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 01/13] fs/unionfs: Fix a memory leak & null pointer dereference Josef 'Jeff' Sipek
@ 2007-03-05  1:54 ` Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 03/13] fs/unionfs/: Don't grab dentry private data mutex in unionfs_d_release Josef 'Jeff' Sipek
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:54 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Erez Zadok, Josef 'Jeff' Sipek

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/main.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index ca7ee26..bd64242 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -584,10 +584,11 @@ static int unionfs_read_super(struct super_block *sb, void *raw_data,
 	atomic_set(&UNIONFS_D(sb->s_root)->generation, 1);
 
 	/* call interpose to create the upper level inode */
-	if ((err = unionfs_interpose(sb->s_root, sb, 0)))
-		goto out_freedpd;
+	err = unionfs_interpose(sb->s_root, sb, 0);
 	unionfs_unlock_dentry(sb->s_root);
-	goto out;
+	if (!err)
+		goto out;
+	/* else fall through */
 
 out_freedpd:
 	if (UNIONFS_D(sb->s_root)) {
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 03/13] fs/unionfs/: Don't grab dentry private data mutex in unionfs_d_release
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 01/13] fs/unionfs: Fix a memory leak & null pointer dereference Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 02/13] fs/unionfs/: Fix a memory leak in unionfs_read_super Josef 'Jeff' Sipek
@ 2007-03-05  1:54 ` Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 04/13] fs/unionfs/: Several small cleanups in unionfs_interpose Josef 'Jeff' Sipek
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:54 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Josef 'Jeff' Sipek, Erez Zadok

Grabbing the UNIONFS_D(dentry)->lock is completely unnecessary and there are
no other references; we are about to free the object anyway. Additionally,
grabbing the mutex produces warning when the slab object is reused - as it
was freed while there still was a reference to it.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/dentry.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 3721409..ac4bf0e 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -190,12 +190,6 @@ static void unionfs_d_release(struct dentry *dentry)
 {
 	int bindex, bstart, bend;
 
-	/* There is no reason to lock the dentry, because we have the only
-	 * reference, but the printing functions verify that we have a lock
-	 * on the dentry before calling dbstart, etc.
-	 */
-	unionfs_lock_dentry(dentry);
-
 	/* this could be a negative dentry, so check first */
 	if (!UNIONFS_D(dentry)) {
 		printk(KERN_DEBUG "dentry without private data: %.*s",
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 04/13] fs/unionfs/: Several small cleanups in unionfs_interpose
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
                   ` (2 preceding siblings ...)
  2007-03-05  1:54 ` [PATCH 03/13] fs/unionfs/: Don't grab dentry private data mutex in unionfs_d_release Josef 'Jeff' Sipek
@ 2007-03-05  1:54 ` Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 05/13] fs/unionfs/: Rename unionfs_d_revalidate_wrap Josef 'Jeff' Sipek
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:54 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Josef 'Jeff' Sipek

1) No need to lock the inode - lockdep was complaining about potential
circular dependency

2) No need to use temporary variable for iunique() inode number

3) Removed unneeded comment

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/main.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index bd64242..a37916d 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -66,19 +66,14 @@ int unionfs_interpose(struct dentry *dentry, struct super_block *sb, int flag)
 			err = -ENOMEM;
 			goto out;
 		}
-		mutex_lock(&inode->i_mutex);
 	} else {
-		ino_t ino;
 		/* get unique inode number for unionfs */
-		ino = iunique(sb, UNIONFS_ROOT_INO);
-
-		inode = iget(sb, ino);
+		inode = iget(sb, iunique(sb, UNIONFS_ROOT_INO));
 		if (!inode) {
-			err = -EACCES;	/* should be impossible??? */
+			err = -EACCES;
 			goto out;
 		}
 
-		mutex_lock(&inode->i_mutex);
 		if (atomic_read(&inode->i_count) > 1)
 			goto skip;
 	}
@@ -147,8 +142,6 @@ skip:
 		BUG();
 	}
 
-	mutex_unlock(&inode->i_mutex);
-
 out:
 	return err;
 }
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 05/13] fs/unionfs/: Rename unionfs_d_revalidate_wrap
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
                   ` (3 preceding siblings ...)
  2007-03-05  1:54 ` [PATCH 04/13] fs/unionfs/: Several small cleanups in unionfs_interpose Josef 'Jeff' Sipek
@ 2007-03-05  1:54 ` Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 06/13] fs/unionfs/: Remove alloc_filldir_node Josef 'Jeff' Sipek
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:54 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Josef 'Jeff' Sipek

Follow the convention of "foo" calling "__foo".

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/commonfops.c |    2 +-
 fs/unionfs/dentry.c     |    9 ++++-----
 fs/unionfs/union.h      |    2 +-
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 66d6ce9..2664be9 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -228,7 +228,7 @@ int unionfs_file_revalidate(struct file *file, int willwrite)
 	unionfs_lock_dentry(dentry);
 	sb = dentry->d_sb;
 	unionfs_read_lock(sb);
-	if (!unionfs_d_revalidate(dentry, NULL) && !d_deleted(dentry)) {
+	if (!__unionfs_d_revalidate(dentry, NULL) && !d_deleted(dentry)) {
 		err = -ESTALE;
 		goto out_nofree;
 	}
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index ac4bf0e..cae4897 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -21,7 +21,7 @@
 /*
  * returns 1 if valid, 0 otherwise.
  */
-int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
+int __unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 {
 	int valid = 1;		/* default is valid (1); invalid is 0. */
 	struct dentry *hidden_dentry;
@@ -174,13 +174,12 @@ out:
 	return valid;
 }
 
-static int unionfs_d_revalidate_wrap(struct dentry *dentry,
-				     struct nameidata *nd)
+static int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 {
 	int err;
 
 	unionfs_lock_dentry(dentry);
-	err = unionfs_d_revalidate(dentry, nd);
+	err = __unionfs_d_revalidate(dentry, nd);
 	unionfs_unlock_dentry(dentry);
 
 	return err;
@@ -226,7 +225,7 @@ out:
 }
 
 struct dentry_operations unionfs_dops = {
-	.d_revalidate	= unionfs_d_revalidate_wrap,
+	.d_revalidate	= unionfs_d_revalidate,
 	.d_release	= unionfs_d_release,
 };
 
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 60276c7..bae3c76 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -296,7 +296,7 @@ extern int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 int unionfs_unlink(struct inode *dir, struct dentry *dentry);
 int unionfs_rmdir(struct inode *dir, struct dentry *dentry);
 
-int unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd);
+int __unionfs_d_revalidate(struct dentry *dentry, struct nameidata *nd);
 
 /* The values for unionfs_interpose's flag. */
 #define INTERPOSE_DEFAULT	0
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 06/13] fs/unionfs/: Remove alloc_filldir_node
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
                   ` (4 preceding siblings ...)
  2007-03-05  1:54 ` [PATCH 05/13] fs/unionfs/: Rename unionfs_d_revalidate_wrap Josef 'Jeff' Sipek
@ 2007-03-05  1:54 ` Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 07/13] fs/unionfs/: Use SEEK_{SET,CUR} instead of hardcoded values Josef 'Jeff' Sipek
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:54 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Josef 'Jeff' Sipek

alloc_filldir_node was used only once. Additionally, all the arguments
passed to it were ignored wasting stack space for no reason whatsoever.

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/rdstate.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/fs/unionfs/rdstate.c b/fs/unionfs/rdstate.c
index e240285..b67a86a 100644
--- a/fs/unionfs/rdstate.c
+++ b/fs/unionfs/rdstate.c
@@ -225,12 +225,6 @@ struct filldir_node *find_filldir_node(struct unionfs_dir_state *rdstate,
 	return cursor;
 }
 
-static struct filldir_node *alloc_filldir_node(const char *name, int namelen,
-					       unsigned int hash, int bindex)
-{
-	return kmem_cache_alloc(unionfs_filldir_cachep, GFP_KERNEL);
-}
-
 int add_filldir_node(struct unionfs_dir_state *rdstate, const char *name,
 		     int namelen, int bindex, int whiteout)
 {
@@ -246,7 +240,7 @@ int add_filldir_node(struct unionfs_dir_state *rdstate, const char *name,
 	index = hash % rdstate->size;
 	head = &(rdstate->list[index]);
 
-	new = alloc_filldir_node(name, namelen, hash, bindex);
+	new = kmem_cache_alloc(unionfs_filldir_cachep, GFP_KERNEL);
 	if (!new) {
 		err = -ENOMEM;
 		goto out;
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 07/13] fs/unionfs/: Use SEEK_{SET,CUR} instead of hardcoded values
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
                   ` (5 preceding siblings ...)
  2007-03-05  1:54 ` [PATCH 06/13] fs/unionfs/: Remove alloc_filldir_node Josef 'Jeff' Sipek
@ 2007-03-05  1:54 ` Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 08/13] fs/unionfs/: Check return value of d_path Josef 'Jeff' Sipek
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:54 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Josef 'Jeff' Sipek

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/dirfops.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/unionfs/dirfops.c b/fs/unionfs/dirfops.c
index 2b77fa9..8f568c7 100644
--- a/fs/unionfs/dirfops.c
+++ b/fs/unionfs/dirfops.c
@@ -135,15 +135,15 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)
 		buf.sb = inode->i_sb;
 
 		/* Read starting from where we last left off. */
-		offset = vfs_llseek(hidden_file, uds->dirpos, 0);
+		offset = vfs_llseek(hidden_file, uds->dirpos, SEEK_SET);
 		if (offset < 0) {
 			err = offset;
 			goto out;
 		}
 		err = vfs_readdir(hidden_file, unionfs_filldir, &buf);
-		/* Save the position for when we continue. */
 
-		offset = vfs_llseek(hidden_file, 0, 1);
+		/* Save the position for when we continue. */
+		offset = vfs_llseek(hidden_file, 0, SEEK_CUR);
 		if (offset < 0) {
 			err = offset;
 			goto out;
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 08/13] fs/unionfs/: Check return value of d_path
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
                   ` (6 preceding siblings ...)
  2007-03-05  1:54 ` [PATCH 07/13] fs/unionfs/: Use SEEK_{SET,CUR} instead of hardcoded values Josef 'Jeff' Sipek
@ 2007-03-05  1:54 ` Josef 'Jeff' Sipek
  2007-03-05  1:54 ` [PATCH 09/13] fs/unionfs/: Miscellaneous coding style fixes Josef 'Jeff' Sipek
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:54 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Josef 'Jeff' Sipek

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/super.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 38443c7..176cfb6 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -311,6 +311,11 @@ static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt)
 		path = d_path(unionfs_lower_dentry_idx(sb->s_root, bindex),
 			   unionfs_lower_mnt_idx(sb->s_root, bindex), tmp_page,
 			   PAGE_SIZE);
+		if (IS_ERR(path)) {
+			ret = PTR_ERR(path);
+			goto out;
+		}
+
 		perms = branchperms(sb, bindex);
 
 		seq_printf(m, "%s=%s", path,
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 09/13] fs/unionfs/: Miscellaneous coding style fixes
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
                   ` (7 preceding siblings ...)
  2007-03-05  1:54 ` [PATCH 08/13] fs/unionfs/: Check return value of d_path Josef 'Jeff' Sipek
@ 2007-03-05  1:54 ` Josef 'Jeff' Sipek
  2007-03-05  1:55 ` [PATCH 10/13] fs/unionfs/: Fix copyup_deleted_file dentry leak Josef 'Jeff' Sipek
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:54 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Josef 'Jeff' Sipek

Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/super.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 176cfb6..571b589 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -320,9 +320,8 @@ static int unionfs_show_options(struct seq_file *m, struct vfsmount *mnt)
 
 		seq_printf(m, "%s=%s", path,
 			   perms & MAY_WRITE ? "rw" : "ro");
-		if (bindex != bend) {
+		if (bindex != bend)
 			seq_printf(m, ":");
-		}
 	}
 
 out:
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 10/13] fs/unionfs/: Fix copyup_deleted_file dentry leak
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
                   ` (8 preceding siblings ...)
  2007-03-05  1:54 ` [PATCH 09/13] fs/unionfs/: Miscellaneous coding style fixes Josef 'Jeff' Sipek
@ 2007-03-05  1:55 ` Josef 'Jeff' Sipek
  2007-03-05  1:55 ` [PATCH 11/13] fs/unionfs/: mntput in __cleanup_dentry Josef 'Jeff' Sipek
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:55 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Erez Zadok, Josef 'Jeff' Sipek

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/commonfops.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 2664be9..aa7c75d 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -41,6 +41,15 @@ static int copyup_deleted_file(struct file *file, struct dentry *dentry,
 	sprintf(name, ".unionfs%*.*lx",
 			i_inosize, i_inosize, hidden_dentry->d_inode->i_ino);
 
+	/*
+	 * Loop, looking for an unused temp name to copyup to.
+	 *
+	 * It's somewhat silly that we look for a free temp tmp name in the
+	 * source branch (bstart) instead of the dest branch (bindex), where
+	 * the final name will be created.  We _will_ catch it if somehow
+	 * the name exists in the dest branch, but it'd be nice to catch it
+	 * sooner than later.
+	 */
 	tmp_dentry = NULL;
 	do {
 		char *suffix = name + nlen - countersize;
@@ -58,7 +67,9 @@ static int copyup_deleted_file(struct file *file, struct dentry *dentry,
 			err = PTR_ERR(tmp_dentry);
 			goto out;
 		}
+		/* don't dput here because of do-while condition eval order */
 	} while (tmp_dentry->d_inode != NULL);	/* need negative dentry */
+	dput(tmp_dentry);
 
 	err = copyup_named_file(dentry->d_parent->d_inode, file, name, bstart,
 				bindex, file->f_dentry->d_inode->i_size);
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 11/13] fs/unionfs/: mntput in __cleanup_dentry
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
                   ` (9 preceding siblings ...)
  2007-03-05  1:55 ` [PATCH 10/13] fs/unionfs/: Fix copyup_deleted_file dentry leak Josef 'Jeff' Sipek
@ 2007-03-05  1:55 ` Josef 'Jeff' Sipek
  2007-03-05  1:55 ` [PATCH 12/13] fs/unionfs/: Fix dentry leak in copyup_named_dentry Josef 'Jeff' Sipek
  2007-03-05  1:55 ` [PATCH 13/13] fs/unionfs/: Fix unlocking in error paths Josef 'Jeff' Sipek
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:55 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Erez Zadok, Josef 'Jeff' Sipek

From: Erez Zadok <ezk@cs.sunysb.edu>

This fixes a mnt refleak which occured during copyup when directory
hierarchy was recreated on a writable branch.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/copyup.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 998cc69..e0075ca 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -548,6 +548,9 @@ static void __cleanup_dentry(struct dentry * dentry, int bindex,
 		if (!unionfs_lower_dentry_idx(dentry, i)->d_inode) {
 			dput(unionfs_lower_dentry_idx(dentry, i));
 			unionfs_set_lower_dentry_idx(dentry, i, NULL);
+
+			mntput(unionfs_lower_mnt_idx(dentry, i));
+			unionfs_set_lower_mnt_idx(dentry, i, NULL);
 		} else {
 			if (new_bstart < 0)
 				new_bstart = i;
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 12/13] fs/unionfs/: Fix dentry leak in copyup_named_dentry
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
                   ` (10 preceding siblings ...)
  2007-03-05  1:55 ` [PATCH 11/13] fs/unionfs/: mntput in __cleanup_dentry Josef 'Jeff' Sipek
@ 2007-03-05  1:55 ` Josef 'Jeff' Sipek
  2007-03-05  1:55 ` [PATCH 13/13] fs/unionfs/: Fix unlocking in error paths Josef 'Jeff' Sipek
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:55 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Erez Zadok, Josef 'Jeff' Sipek

From: Erez Zadok <ezk@cs.sunysb.edu>

When we chmod a directory on a readonly branch, and have to copy it up, we
forget to dput(). If this was a file, it gets dput indirectly through other
functions we call, but not if it was a directory.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/copyup.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index e0075ca..e24d940 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -352,20 +352,18 @@ static int copyup_named_dentry(struct inode *dir, struct dentry *dentry,
 
 	unionfs_read_lock(sb);
 
-	if ((err = is_robranch_super(sb, new_bindex))) {
-		dput(old_hidden_dentry);
+	if ((err = is_robranch_super(sb, new_bindex)))
 		goto out;
-	}
 
 	/* Create the directory structure above this dentry. */
 	new_hidden_dentry = create_parents_named(dir, dentry, name, new_bindex);
 	if (IS_ERR(new_hidden_dentry)) {
-		dput(old_hidden_dentry);
 		err = PTR_ERR(new_hidden_dentry);
 		goto out;
 	}
 
 	old_hidden_dentry = unionfs_lower_dentry_idx(dentry, old_bindex);
+	/* we conditionally dput this old_hidden_dentry at end of function */
 	dget(old_hidden_dentry);
 
 	/* For symlinks, we must read the link before we lock the directory. */
@@ -460,6 +458,14 @@ out_unlock:
 	unlock_dir(new_hidden_parent_dentry);
 
 out_free:
+	/*
+	 * If old_hidden_dentry was a directory, we need to dput it.  If it
+	 * was a file, then it was already dput indirectly by other
+	 * functions we call ablve which operate on regular files.
+	 */
+	if (old_hidden_dentry && old_hidden_dentry->d_inode &&
+	    S_ISDIR(old_hidden_dentry->d_inode->i_mode))
+		dput(old_hidden_dentry);
 	kfree(symbuf);
 
 out:
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 13/13] fs/unionfs/: Fix unlocking in error paths
  2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
                   ` (11 preceding siblings ...)
  2007-03-05  1:55 ` [PATCH 12/13] fs/unionfs/: Fix dentry leak in copyup_named_dentry Josef 'Jeff' Sipek
@ 2007-03-05  1:55 ` Josef 'Jeff' Sipek
  12 siblings, 0 replies; 14+ messages in thread
From: Josef 'Jeff' Sipek @ 2007-03-05  1:55 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Erez Zadok, Josef 'Jeff' Sipek

From: Erez Zadok <ezk@cs.sunysb.edu>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
Signed-off-by: Josef 'Jeff' Sipek <jsipek@cs.sunysb.edu>
---
 fs/unionfs/lookup.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index df929e9..967bb5b 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -84,6 +84,7 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *n
 	struct vfsmount *first_hidden_mnt = NULL;
 	int locked_parent = 0;
 	int locked_child = 0;
+	int allocated_new_info = 0;
 
 	int opaque;
 	char *whname = NULL;
@@ -101,9 +102,11 @@ struct dentry *unionfs_lookup_backend(struct dentry *dentry, struct nameidata *n
 		BUG_ON(UNIONFS_D(dentry) != NULL);
 		locked_child = 1;
 	}
-	if (lookupmode != INTERPOSE_PARTIAL)
+	if (lookupmode != INTERPOSE_PARTIAL) {
 		if ((err = new_dentry_private_data(dentry)))
 			goto out;
+		allocated_new_info = 1;
+	}
 	/* must initialize dentry operations */
 	dentry->d_op = &unionfs_dops;
 
@@ -380,7 +383,7 @@ out:
 	if (locked_parent)
 		unionfs_unlock_dentry(parent_dentry);
 	dput(parent_dentry);
-	if (locked_child)
+	if (locked_child || (err && allocated_new_info))
 		unionfs_unlock_dentry(dentry);
 	return ERR_PTR(err);
 }
@@ -431,6 +434,7 @@ int new_dentry_private_data(struct dentry *dentry)
 	int newsize;
 	int oldsize = 0;
 	struct unionfs_dentry_info *info = UNIONFS_D(dentry);
+	int unlock_on_err = 0;
 
 	spin_lock(&dentry->d_lock);
 	if (!info) {
@@ -443,6 +447,7 @@ int new_dentry_private_data(struct dentry *dentry)
 
 		mutex_init(&info->lock);
 		mutex_lock(&info->lock);
+		unlock_on_err = 1;
 
 		info->lower_paths = NULL;
 	} else
@@ -482,6 +487,8 @@ int new_dentry_private_data(struct dentry *dentry)
 
 out_free:
 	kfree(info->lower_paths);
+	if (unlock_on_err)
+		mutex_unlock(&info->lock);
 
 out:
 	free_dentry_private_data(info);
-- 
1.5.0.2.260.g2eb065


^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2007-03-05  1:59 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-05  1:54 [GIT PULL -mm] Unionfs updates Josef 'Jeff' Sipek
2007-03-05  1:54 ` [PATCH 01/13] fs/unionfs: Fix a memory leak & null pointer dereference Josef 'Jeff' Sipek
2007-03-05  1:54 ` [PATCH 02/13] fs/unionfs/: Fix a memory leak in unionfs_read_super Josef 'Jeff' Sipek
2007-03-05  1:54 ` [PATCH 03/13] fs/unionfs/: Don't grab dentry private data mutex in unionfs_d_release Josef 'Jeff' Sipek
2007-03-05  1:54 ` [PATCH 04/13] fs/unionfs/: Several small cleanups in unionfs_interpose Josef 'Jeff' Sipek
2007-03-05  1:54 ` [PATCH 05/13] fs/unionfs/: Rename unionfs_d_revalidate_wrap Josef 'Jeff' Sipek
2007-03-05  1:54 ` [PATCH 06/13] fs/unionfs/: Remove alloc_filldir_node Josef 'Jeff' Sipek
2007-03-05  1:54 ` [PATCH 07/13] fs/unionfs/: Use SEEK_{SET,CUR} instead of hardcoded values Josef 'Jeff' Sipek
2007-03-05  1:54 ` [PATCH 08/13] fs/unionfs/: Check return value of d_path Josef 'Jeff' Sipek
2007-03-05  1:54 ` [PATCH 09/13] fs/unionfs/: Miscellaneous coding style fixes Josef 'Jeff' Sipek
2007-03-05  1:55 ` [PATCH 10/13] fs/unionfs/: Fix copyup_deleted_file dentry leak Josef 'Jeff' Sipek
2007-03-05  1:55 ` [PATCH 11/13] fs/unionfs/: mntput in __cleanup_dentry Josef 'Jeff' Sipek
2007-03-05  1:55 ` [PATCH 12/13] fs/unionfs/: Fix dentry leak in copyup_named_dentry Josef 'Jeff' Sipek
2007-03-05  1:55 ` [PATCH 13/13] fs/unionfs/: Fix unlocking in error paths Josef 'Jeff' Sipek

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).