Re: [PATCH 11/17] target: plug memory leak

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 1 September 2014 01:07:01 +0300, Sagi Grimberg wrote:
> On 8/28/2014 4:01 AM, Joern Engel wrote:
> >Found by coverity.
> >
> >Signed-off-by: Joern Engel <joern@xxxxxxxxx>
> >---
> >  drivers/target/target_core_fabric_configfs.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
> >index 7de9f0475d05..ab8a27eff94c 100644
> >--- a/drivers/target/target_core_fabric_configfs.c
> >+++ b/drivers/target/target_core_fabric_configfs.c
> >@@ -381,7 +381,7 @@ static struct config_group *target_fabric_make_mappedlun(
> >  	if (!lacl_cg->default_groups) {
> >  		pr_err("Unable to allocate lacl_cg->default_groups\n");
> >  		ret = -ENOMEM;
> >-		goto out;
> >+		goto out2;
> >  	}
> >
> >  	config_group_init_type_name(&lacl->se_lun_group, name,
> >@@ -397,12 +397,14 @@ static struct config_group *target_fabric_make_mappedlun(
> >  	if (!ml_stat_grp->default_groups) {
> >  		pr_err("Unable to allocate ml_stat_grp->default_groups\n");
> >  		ret = -ENOMEM;
> >-		goto out;
> >+		goto out2;
> >  	}
> >  	target_stat_setup_mappedlun_default_groups(lacl);
> >
> >  	kfree(buf);
> >  	return &lacl->se_lun_group;
> >+out2:
> >+	kfree(lacl);
> 
> 1. Please use a meaningful tag name.

I'm not particularly creative nor do I care too much about meaningful
tag names.  Which makes me part of a 10% minority, according to git
grep.  Guess it might be time to change my ways and stick to majority
consensus.

> 2. Not sure you need a tag here, lacl is either NULL or should
> be freed - you can go ahead and free it under out is kfree is
> NULL safe (I even think Coverity complains about it, or is it smatch?)

That requires initializing lacl to NULL before the first goto, which
grows .text by 16 bytes on x86_64.  Remember there are "goto out"s
before the call to core_dev_init_initiator_node_lun_acl().

But I agree it makes the code a bit nicer and that's well worth it.

Jörn

--
With a PC, I always felt limited by the software available. On Unix,
I am limited only by my knowledge.
-- Peter J. Schoenster
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux