Skip to contentSkip to conversation

aoe: use after free in aoedev_by_aoeaddr()

PATCH In review Dan Carpenter 2 participants 3 messages Dec 7, 2012 – Dec 10, 2012

Subsystems (3)
Block layer Kernel janitors ATA over Ethernet (aoe) driver
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 aoe: use after free in aoedev_by_aoeaddr()

We should return NULL on failure instead of returning a freed pointer.

Signed-off-by Dan Carpenter <[email protected]>
drivers/block/aoe/aoedev.c +1 -0
index e66ccbf..98f2965 100644
@@ -461,6 +461,7 @@ aoedev_by_aoeaddr(ulong maj, int min, int do_alloc)
 	d->targets = kcalloc(NTARGETS, sizeof(*d->targets), GFP_ATOMIC);
 	if (!d->targets) {
 		kfree(d);
+		d = NULL;
 		goto out;
 	}
 	d->ntargets = NTARGETS;
#1 lore raw
Quoted 9 lines · Dan Carpenter

We should return NULL on failure instead of returning a freed pointer.

Signed-off-by Dan Carpenter <[email protected]>
drivers/block/aoe/aoedev.c +0 -0
index e66ccbf..98f2965 100644

drivers/block/aoe/aoedev.c aoedev_by_aoeaddr(ulong maj, int min, int do_alloc) · line 467

Quoted 7 lines
drivers/block/aoe/aoedev.c +1 -0
@@ -461,6 +461,7 @@ aoedev_by_aoeaddr(ulong maj, int min, int do_alloc)
 	d->targets = kcalloc(NTARGETS, sizeof(*d->targets), GFP_ATOMIC);
 	if (!d->targets) {
 		kfree(d);
+		d = NULL;
 		goto out;
 	}
 	d->ntargets = NTARGETS;

That looks good, thanks.

If smatch found that, then I have to figure out what I'm doing wrong, because I ran sparse and smatch on the latest patch series until they were clean. :/

Signature · 2 lines
  Ed Cashin
  [email protected]
author
#2 lore raw
Quoted 22 lines · Ed Cashin
Quoted 9 lines · Dan Carpenter

We should return NULL on failure instead of returning a freed pointer.

Signed-off-by Dan Carpenter <[email protected]>
drivers/block/aoe/aoedev.c +0 -0
index e66ccbf..98f2965 100644

drivers/block/aoe/aoedev.c aoedev_by_aoeaddr(ulong maj, int min, int do_alloc) · line 467

Quoted 7 lines
drivers/block/aoe/aoedev.c +1 -0
@@ -461,6 +461,7 @@ aoedev_by_aoeaddr(ulong maj, int min, int do_alloc)
 	d->targets = kcalloc(NTARGETS, sizeof(*d->targets), GFP_ATOMIC);
 	if (!d->targets) {
 		kfree(d);
+		d = NULL;
 		goto out;
 	}
 	d->ntargets = NTARGETS;

That looks good, thanks.

Quoted 2 lines

If smatch found that, then I have to figure out what I'm doing wrong, because I ran sparse and smatch on the latest patch series until they were clean. :/

It's one of the checks which is only enabled with the --spammy option.

regards,
dan carpenter