possible memory leak in commands/nand.c?

once again, perhaps i'm just misreading this but consider this code
from commands/nand.c, noting the two early calls to asprintf():

===== begin =====

        bb = xzalloc(sizeof(*bb));
        bb->devname = asprintf("/dev/%s", basename(path));
        if (name)
                bb->cdev.name = strdup(name);
        else
                bb->cdev.name = asprintf("%s.bb", basename(path));

        ret = stat(bb->devname, &s);
        if (ret)
                goto free_out;

        bb->raw_size = s.st_size;

        bb->fd = open(bb->devname, O_RDWR);
        if (bb->fd < 0) {
                ret = -ENODEV;
                goto free_out;
        }

        ret = ioctl(bb->fd, MEMGETINFO, &bb->info);
        if (ret)
                goto free_out;

        nand_bb_calc_size(bb);
        bb->cdev.ops = &nand_bb_ops;
        bb->cdev.priv = bb;

        devfs_create(&bb->cdev);

        return 0;

free_out:

        free(bb);
        return ret;

===== end =====

if something in the latter part of that code fails and control jumps
to the label "free_out", won't this code fail to free() the memory allocated in the two asprintf() calls? isn't the programmer explicitly required to free memory allocated with asprintf() or vasprintf() calls?

Yes, indeed, that's a memory hole here. The following should fix this. Thanks for noting.

Sascha

Quoted 1 line

From 4e4b03cd61808383a98cb1d10a47025e1909e0bd Mon Sep 17 00:00:00 2001

From: Sascha Hauer <[email protected]>
Date: Mon, 21 Dec 2009 09:41:52 +0100
Subject: [PATCH] commands/nand.c: Fix memory hole

Signed-off-by Sascha Hauer <[email protected]>

---
 commands/nand.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

commands/nand.c +10 -5

index cbf1058..55b89af 100644
@@ -224,31 +224,37 @@ static struct file_operations nand_bb_ops = {
 int dev_add_bb_dev(char *path, const char *name)
 {
 	struct nand_bb *bb;
-	int ret;
+	int ret = -ENOMEM;
 	struct stat s;
 
 	bb = xzalloc(sizeof(*bb));
 	bb->devname = asprintf("/dev/%s", basename(path));
+	if (!bb->devname)
+		goto out1;
+
 	if (name)
 		bb->cdev.name = strdup(name);
 	else
 		bb->cdev.name = asprintf("%s.bb", basename(path));
 
+	if (!bb->cdev.name)
+		goto out2;
+
 	ret = stat(bb->devname, &s);
 	if (ret)
-		goto free_out;
+		goto out3;
 
 	bb->raw_size = s.st_size;
 
 	bb->fd = open(bb->devname, O_RDWR);
 	if (bb->fd < 0) {
 		ret = -ENODEV;
-		goto free_out;
+		goto out3;
 	}
 
 	ret = ioctl(bb->fd, MEMGETINFO, &bb->info);
 	if (ret)
-		goto free_out;
+		goto out4;
 
 	nand_bb_calc_size(bb);
 	bb->cdev.ops = &nand_bb_ops;

@@ -258,7 +264,13 @@ int dev_add_bb_dev(char *path, const char *name)
 
 	return 0;
 
-free_out:

On Mon, 21 Dec 2009, Sascha Hauer wrote:

... snip ...

Quoted 67 lines

Yes, indeed, that's a memory hole here. The following should fix this. Thanks for noting.

Sascha

Quoted 1 line

From 4e4b03cd61808383a98cb1d10a47025e1909e0bd Mon Sep 17 00:00:00 2001

From: Sascha Hauer <[email protected]>
Date: Mon, 21 Dec 2009 09:41:52 +0100
Subject: [PATCH] commands/nand.c: Fix memory hole

Signed-off-by Sascha Hauer <[email protected]>

---
 commands/nand.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

commands/nand.c +10 -5

index cbf1058..55b89af 100644
@@ -224,31 +224,37 @@ static struct file_operations nand_bb_ops = {
 int dev_add_bb_dev(char *path, const char *name)
 {
 	struct nand_bb *bb;
-	int ret;
+	int ret = -ENOMEM;
 	struct stat s;
 
 	bb = xzalloc(sizeof(*bb));
 	bb->devname = asprintf("/dev/%s", basename(path));
+	if (!bb->devname)
+		goto out1;
+
 	if (name)
 		bb->cdev.name = strdup(name);
 	else
 		bb->cdev.name = asprintf("%s.bb", basename(path));
 
+	if (!bb->cdev.name)
+		goto out2;
+
 	ret = stat(bb->devname, &s);
 	if (ret)
-		goto free_out;
+		goto out3;
 
 	bb->raw_size = s.st_size;
 
 	bb->fd = open(bb->devname, O_RDWR);
 	if (bb->fd < 0) {
 		ret = -ENODEV;
-		goto free_out;
+		goto out3;
 	}
 
 	ret = ioctl(bb->fd, MEMGETINFO, &bb->info);
 	if (ret)
-		goto free_out;
+		goto out4;
 
 	nand_bb_calc_size(bb);
 	bb->cdev.ops = &nand_bb_ops;

@@ -258,7 +264,13 @@ int dev_add_bb_dev(char *path, const char *name)
 
 	return 0;
 
-free_out:

commands/nand.c int dev_add_bb_dev(char *path, const char *name) · line 276

Quoted 10 lines

commands/nand.c +7 -0

+out4:
+	close(bb->fd);
+out3:
+	free(bb->cdev.name);
+out2:
+	free(bb->devname);
+out1:
 	free(bb);
 	return ret;
 }

i'm not sure this required distinguishing between every one of those
cases since the initial space was allocated with xzalloc(), guaranteeing it would be zero-filled, and freeing a NULL pointer is supposed to be a no-op.

so it would have been simpler to just

free(bb->devname); # might be NULL, no problem
free(bb->cdev.name); # same here
free(bb);