diff mbox

[v2] lib/flash: fix resource leak in flash_setup_buffer() error paths

Message ID 1462324977-8936-1-git-send-email-andrew.donnellan@au1.ibm.com
State Accepted
Headers show

Commit Message

Andrew Donnellan May 4, 2016, 1:22 a.m. UTC
Some error paths in flash_setup_buffer() fail to free the flash_info struct
or close the open ffs before they return. Change them to goto the cleanup
code at the end. Separate the cleanup code into separate labels depending
on whether we need to call ffs_close(), arch_flash_close() and
talloc_free().

Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

---

Found by Coverity Scan. Build-tested only. sammj, please bikeshed :)

V1->V2:
	- Added missing ffs_close() call, suggested by Cyril Bur
---
 lib/flash/flash.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Sam Mendoza-Jonas May 6, 2016, 4:08 a.m. UTC | #1
On Wed, May 04, 2016 at 11:22:57AM +1000, Andrew Donnellan wrote:
> Some error paths in flash_setup_buffer() fail to free the flash_info struct
> or close the open ffs before they return. Change them to goto the cleanup
> code at the end. Separate the cleanup code into separate labels depending
> on whether we need to call ffs_close(), arch_flash_close() and
> talloc_free().
> 
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Thanks! Merged as 87fa488.

> 
> ---
> 
> Found by Coverity Scan. Build-tested only. sammj, please bikeshed :)
> 
> V1->V2:
> 	- Added missing ffs_close() call, suggested by Cyril Bur
> ---
>  lib/flash/flash.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/flash/flash.c b/lib/flash/flash.c
> index b6188a0..1384118 100644
> --- a/lib/flash/flash.c
> +++ b/lib/flash/flash.c
> @@ -82,26 +82,26 @@ static struct flash_info *flash_setup_buffer(void *ctx, const char *partition)
>  	rc = arch_flash_init(&info->bl, NULL, true);
>  	if (rc) {
>  		pb_log("Failed to init mtd device\n");
> -		return NULL;
> +		goto out;
>  	}
>  
>  	rc = blocklevel_get_info(info->bl, &info->path, &info->size,
>  				 &info->erase_granule);
>  	if (rc) {
>  		pb_log("Failed to retrieve blocklevel info\n");
> -		return NULL;
> +	        goto out_flash;
>  	}
>  
>  	rc = ffs_init(0, info->size, info->bl, &info->ffs, 1);
>  	if (rc) {
>  		pb_log("%s: Failed to init ffs\n", __func__);
> -		goto out;
> +		goto out_flash;
>  	}
>  
>  	rc = partition_info(info, partition);
>  	if (rc) {
>  		pb_log("Failed to retrieve partition info\n");
> -		goto out;
> +		goto out_ffs;
>  	}
>  
>  	/* Check if there is a second flash side. If there is not, or
> @@ -139,8 +139,11 @@ static struct flash_info *flash_setup_buffer(void *ctx, const char *partition)
>  	}
>  
>  	return info;
> -out:
> +out_ffs:
> +	ffs_close(info->ffs);
> +out_flash:
>  	arch_flash_close(info->bl, NULL);
> +out:
>  	talloc_free(info);
>  	return NULL;
>  }
> -- 
> Andrew Donnellan              OzLabs, ADL Canberra
> andrew.donnellan@au1.ibm.com  IBM Australia Limited
> 
> _______________________________________________
> Petitboot mailing list
> Petitboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/petitboot
diff mbox

Patch

diff --git a/lib/flash/flash.c b/lib/flash/flash.c
index b6188a0..1384118 100644
--- a/lib/flash/flash.c
+++ b/lib/flash/flash.c
@@ -82,26 +82,26 @@  static struct flash_info *flash_setup_buffer(void *ctx, const char *partition)
 	rc = arch_flash_init(&info->bl, NULL, true);
 	if (rc) {
 		pb_log("Failed to init mtd device\n");
-		return NULL;
+		goto out;
 	}
 
 	rc = blocklevel_get_info(info->bl, &info->path, &info->size,
 				 &info->erase_granule);
 	if (rc) {
 		pb_log("Failed to retrieve blocklevel info\n");
-		return NULL;
+	        goto out_flash;
 	}
 
 	rc = ffs_init(0, info->size, info->bl, &info->ffs, 1);
 	if (rc) {
 		pb_log("%s: Failed to init ffs\n", __func__);
-		goto out;
+		goto out_flash;
 	}
 
 	rc = partition_info(info, partition);
 	if (rc) {
 		pb_log("Failed to retrieve partition info\n");
-		goto out;
+		goto out_ffs;
 	}
 
 	/* Check if there is a second flash side. If there is not, or
@@ -139,8 +139,11 @@  static struct flash_info *flash_setup_buffer(void *ctx, const char *partition)
 	}
 
 	return info;
-out:
+out_ffs:
+	ffs_close(info->ffs);
+out_flash:
 	arch_flash_close(info->bl, NULL);
+out:
 	talloc_free(info);
 	return NULL;
 }