[PATCH] ext4: Avoid memory leak of ext4_groupinfo_caches in ext4_groupinfo_create_slab.

[PATCH] ext4: Avoid memory leak of ext4_groupinfo_caches in ext4_groupinfo_create_slab.

[Tao Ma]

From: Tao Ma <boyu.mt@taobao.com>

In ext4_groupinfo_create_slab, we create ext4_groupinfo_caches within
ext4_grpinfo_slab_create_mutex, but set it outside the lock, and there
does exist some case that we may create it twice and causes a memory
leak. So set it before we call mutex_unlock.

Signed-off-by: Tao Ma <boyu.mt@taobao.com>
---
 fs/ext4/mballoc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 9a35263..c680641 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2404,14 +2404,14 @@ static int ext4_groupinfo_create_slab(size_t size)
 					slab_size, 0, SLAB_RECLAIM_ACCOUNT,
 					NULL);
 
+	ext4_groupinfo_caches[cache_index] = cachep;
+
 	mutex_unlock(&ext4_grpinfo_slab_create_mutex);
 	if (!cachep) {
 		printk(KERN_EMERG "EXT4: no memory for groupinfo slab cache\n");
 		return -ENOMEM;
 	}
 
-	ext4_groupinfo_caches[cache_index] = cachep;

[PATCH] ext4: Fix a double free of sbi->s_group_info in ext4_mb_init_backend

[Tao Ma]

From: Tao Ma <boyu.mt@taobao.com>

If we meet with an error in ext4_mb_add_groupinfo, we kfree
sbi->s_group_info[group >> EXT4_DESC_PER_BLOCK_BITS(sb)], but fail to
reset it to NULL. So the caller ext4_mb_init_backend will try to kfree
it again and causes a double free. So fix it by resetting it to NULL.

Some typo in comments of mballoc.c are also changed.

Signed-off-by: Tao Ma <boyu.mt@taobao.com>
---
 fs/ext4/mballoc.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index c680641..4e7d055 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -75,8 +75,8 @@
  *
  * The inode preallocation space is used looking at the _logical_ start
  * block. If only the logical file block falls within the range of prealloc
- * space we will consume the particular prealloc space. This make sure that
- * that the we have contiguous physical blocks representing the file blocks
+ * space we will consume the particular prealloc space. This makes sure that
+ * we have contiguous physical blocks representing the file blocks
  *
  * The important thing to be noted in case of inode prealloc space is that
  * we don't modify the values associated to inode prealloc space except
@@ -84,7 +84,7 @@
  *
  * If we are not able to find blocks in the inode prealloc space and if we
  * have the group allocation flag set then we look at the locality group
- * prealloc space. These are per CPU prealloc list repreasented as
+ * prealloc space. These are per CPU prealloc list represented as
  *
  * ext4_sb_info.s_locality_groups[smp_processor_id()]
  *
@@ -152,7 +152,7 @@
  * best extent in the found extents. Searching for the blocks starts with
  * the group specified as the goal value in allocation context via
  * ac_g_ex. Each group is first checked based on the criteria whether it
- * can used for allocation. ext4_mb_good_group explains how the groups are
+ * can be used for allocation. ext4_mb_good_group explains how the groups are
  * checked.
  *
  * Both the prealloc space are getting populated as above. So for the first
@@ -2279,8 +2279,10 @@ int ext4_mb_add_groupinfo(struct super_block *sb, ext4_group_t group,
 
 exit_group_info:
 	/* If a meta_group_info table has been allocated, release it now */
-	if (group % EXT4_DESC_PER_BLOCK(sb) == 0)
+	if (group % EXT4_DESC_PER_BLOCK(sb) == 0) {
 		kfree(sbi->s_group_info[group >> EXT4_DESC_PER_BLOCK_BITS(sb)]);
+		sbi->s_group_info[group >> EXT4_DESC_PER_BLOCK_BITS(sb)] = NULL;
+	}
 exit_meta_group_info:
 	return -ENOMEM;
 } /* ext4_mb_add_groupinfo */
-- 
1.7.4

Re: [PATCH] ext4: Fix a double free of sbi->s_group_info in ext4_mb_init_backend

[Ted Ts'o]

On Mon, Jul 11, 2011 at 11:26:56PM +0800, Tao Ma wrote:
> From: Tao Ma <boyu.mt@taobao.com>
> 
> If we meet with an error in ext4_mb_add_groupinfo, we kfree
> sbi->s_group_info[group >> EXT4_DESC_PER_BLOCK_BITS(sb)], but fail to
> reset it to NULL. So the caller ext4_mb_init_backend will try to kfree
> it again and causes a double free. So fix it by resetting it to NULL.
> 
> Some typo in comments of mballoc.c are also changed.
> 
> Signed-off-by: Tao Ma <boyu.mt@taobao.com>

Added to the ext4 tree, thanks.

						- Ted

Re: [PATCH] ext4: Avoid memory leak of ext4_groupinfo_caches in ext4_groupinfo_create_slab.

[Ted Ts'o]

On Mon, Jul 11, 2011 at 11:26:55PM +0800, Tao Ma wrote:
> From: Tao Ma <boyu.mt@taobao.com>
> 
> In ext4_groupinfo_create_slab, we create ext4_groupinfo_caches within
> ext4_grpinfo_slab_create_mutex, but set it outside the lock, and there
> does exist some case that we may create it twice and causes a memory
> leak. So set it before we call mutex_unlock.
> 
> Signed-off-by: Tao Ma <boyu.mt@taobao.com>

Applied, with a slightly reworded one-line summary:

    ext4: fix a race which could leak memory in ext4_groupinfo_create_slab()

    	      	     	   	      	     	- Ted