ext4: don't kfree uninitialized s_group_info members

PATCH In review Eric Sandeen 2 participants 2 messages Mar 10, 2011 – Mar 22, 2011 ext4 file system

Share & tools

Eric Sandeen

author

2011-03-10 23:03 UTC #0 lore raw

PATCH 1/1 ext4: don't kfree uninitialized s_group_info members

Per kernel.org bugzilla #30872 we may call kfree on uninitialized members of the s_group_info array. We can avoid this by kzalloc'ing the array, and only freeing them on the error path if they are non-zero.

This doesn't entirely solve the oops on mount if we fail down this path; failed_mount4: frees the sbi, for one, which gets referenced later in the failed mount paths - I haven't worked that out yet.

Reported-by Eugene A. Shatokhin <[email protected]>

Signed-off-by Eric Sandeen <[email protected]>

---

fs/ext4/mballoc.c +3 -2

index d1fe09a..e56befe 100644
@@ -2381,7 +2381,7 @@ static int ext4_mb_init_backend(struct super_block *sb)
 	/* An 8TB filesystem with 64-bit pointers requires a 4096 byte
 	 * kmalloc. A 128kb malloc should suffice for a 256TB filesystem.
 	 * So a two level scheme suffices for now. */
-	sbi->s_group_info = kmalloc(array_size, GFP_KERNEL);
+	sbi->s_group_info = kzalloc(array_size, GFP_KERNEL);
 	if (sbi->s_group_info == NULL) {
 		printk(KERN_ERR "EXT4-fs: can't allocate buddy meta group\n");
 		return -ENOMEM;

@@ -2412,7 +2412,8 @@ err_freebuddy:
 		kmem_cache_free(cachep, ext4_get_group_info(sb, i));
 	i = num_meta_group_infos;
 	while (i-- > 0)
-		kfree(sbi->s_group_info[i]);
+		if (sbi->s_group_info[i])
+			kfree(sbi->s_group_info[i]);
 	iput(sbi->s_buddy_cache);
 err_freesgi:
 	kfree(sbi->s_group_info);

Ted Ts'o

maintainer

2011-03-22 01:27 UTC #1 lore raw

Quoted 11 lines · Eric Sandeen

Per kernel.org bugzilla #30872 we may call kfree on uninitialized members of the s_group_info array. We can avoid this by kzalloc'ing the array, and only freeing them on the error path if they are non-zero.
This doesn't entirely solve the oops on mount if we fail down this path; failed_mount4: frees the sbi, for one, which gets referenced later in the failed mount paths - I haven't worked that out yet.
Reported-by Eugene A. Shatokhin <[email protected]>
Signed-off-by Eric Sandeen <[email protected]>

Added to the ext4 patch tree, although I've dropped the second patch hunk, since it's OK to call kfree on a NULL pointers:

fs/ext4/mballoc.c err_freebuddy: · line 2419

Quoted 10 lines

fs/ext4/mballoc.c +2 -1

@@ -2412,7 +2412,8 @@ err_freebuddy:
 		kmem_cache_free(cachep, ext4_get_group_info(sb, i));
 	i = num_meta_group_infos;
 	while (i-- > 0)
-		kfree(sbi->s_group_info[i]);
+		if (sbi->s_group_info[i])
+			kfree(sbi->s_group_info[i]);
 	iput(sbi->s_buddy_cache);
 err_freesgi:
 	kfree(sbi->s_group_info);

- Ted