On Sun, Dec 20, 2009 at 02:33:11PM -0500, Robert P. J. Day wrote:
>
> 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
>From 4e4b03cd61808383a98cb1d10a47025e1909e0bd Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
Date: Mon, 21 Dec 2009 09:41:52 +0100
Subject: [PATCH] commands/nand.c: Fix memory hole
Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
---
commands/nand.c | 22 +++++++++++++++++-----
1 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/commands/nand.c b/commands/nand.c
index cbf1058..55b89af 100644
--- a/commands/nand.c
+++ b/commands/nand.c
@@ -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:
+out4:
+ close(bb->fd);
+out3:
+ free(bb->cdev.name);
+out2:
+ free(bb->devname);
+out1:
free(bb);
return ret;
}
--
1.6.5.2
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
