diff mbox

btrfs-progs: mkfs seg fault for wrong free

Message ID 1366892142-21920-1-git-send-email-anand.jain@oracle.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Anand Jain April 25, 2013, 12:15 p.m. UTC
With commit
        87c09f7 Btrfs-progs: fix memory leaks on cleanup

mkfs on multiple dev is ending with segfault at
close_all_devices() during kfree(device->name)

because mkfs calls btrfs_add_to_fsid, which does not initialize
name when dev is added to the list.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 disk-io.c | 6 ++++--
 utils.c   | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

David Sterba April 25, 2013, 12:51 p.m. UTC | #1
On Thu, Apr 25, 2013 at 08:15:42PM +0800, Anand Jain wrote:
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -1290,8 +1290,10 @@ static int close_all_devices(struct btrfs_fs_info *fs_info)
>  				fprintf(stderr, "Warning, could not drop caches\n");
>  		}
>  		close(device->fd);
> -		kfree(device->name);
> -		kfree(device->label);
> +		if (device->name)
> +			kfree(device->name);
> +		if (device->label)
> +			kfree(device->label);

As we're going to use the kernel sources directly, we should also keep
the semantics of the helpers. The in-kernel kfree accepts NULL and does
nothing in that case, so should the userspace replacement which is now
implemented as:

#define kfree(x) free(x)

>  		kfree(device);
>  	}
>  	kfree(fs_info->fs_devices);
> diff --git a/utils.c b/utils.c
> index 7b028a7..8104465 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -484,6 +484,8 @@ int btrfs_add_to_fsid(struct btrfs_trans_handle *trans,
>  	device = kmalloc(sizeof(*device), GFP_NOFS);
>  	if (!device)
>  		return -ENOMEM;
> +	memset(device, 0, sizeof(*device));

Please replace the above kmalloc with kzalloc instead.

> +	
>  	buf = kmalloc(sectorsize, GFP_NOFS);
>  	if (!buf) {
>  		kfree(device);
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/disk-io.c b/disk-io.c
index be4abb8..15c90fe 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1290,8 +1290,10 @@  static int close_all_devices(struct btrfs_fs_info *fs_info)
 				fprintf(stderr, "Warning, could not drop caches\n");
 		}
 		close(device->fd);
-		kfree(device->name);
-		kfree(device->label);
+		if (device->name)
+			kfree(device->name);
+		if (device->label)
+			kfree(device->label);
 		kfree(device);
 	}
 	kfree(fs_info->fs_devices);
diff --git a/utils.c b/utils.c
index 7b028a7..8104465 100644
--- a/utils.c
+++ b/utils.c
@@ -484,6 +484,8 @@  int btrfs_add_to_fsid(struct btrfs_trans_handle *trans,
 	device = kmalloc(sizeof(*device), GFP_NOFS);
 	if (!device)
 		return -ENOMEM;
+	memset(device, 0, sizeof(*device));
+	
 	buf = kmalloc(sectorsize, GFP_NOFS);
 	if (!buf) {
 		kfree(device);