diff mbox

lib: fwts_gpe: free original gpe buffer on failed realloc

Message ID 20170413101406.13534-1-colin.king@canonical.com
State Accepted
Headers show

Commit Message

Colin Ian King April 13, 2017, 10:14 a.m. UTC
From: Colin Ian King <colin.king@canonical.com>

A common bug is where realloc fails to allocate and we assume that
the memory being realloc'd was freed. This is not the case, the
NULL return means we need to free the original gpe buffer to avoid
a memory leak.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/lib/src/fwts_gpe.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Alex Hung April 17, 2017, 5:28 p.m. UTC | #1
On 2017-04-13 03:14 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> A common bug is where realloc fails to allocate and we assume that
> the memory being realloc'd was freed. This is not the case, the
> NULL return means we need to free the original gpe buffer to avoid
> a memory leak.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_gpe.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/src/lib/src/fwts_gpe.c b/src/lib/src/fwts_gpe.c
> index b09232f1..a16fee86 100644
> --- a/src/lib/src/fwts_gpe.c
> +++ b/src/lib/src/fwts_gpe.c
> @@ -62,12 +62,17 @@ int fwts_gpe_read(fwts_gpe **gpes)
>  	while ((entry = readdir(dir)) != NULL) {
>  		if ((strncmp(entry->d_name, "gpe", 3) == 0) ||
>  		    (strncmp(entry->d_name, "sci",3) == 0)) {
> -			if ((*gpes = realloc(*gpes, sizeof(fwts_gpe) * (n+1))) == NULL)
> +			fwts_gpe *tmp;
> +
> +			tmp = realloc(*gpes, sizeof(fwts_gpe) * (n+1));
> +			if (!tmp) {
> +				free(*gpes);
>  				goto error;
> -			else {
> +			} else {
>  				char path[PATH_MAX];
>  				char *data;
>
> +				*gpes = tmp;
>  				if (((*gpes)[n].name  = strdup(entry->d_name)) == NULL)
>  					goto error;
>
>


Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu April 21, 2017, 2:43 a.m. UTC | #2
On 04/13/2017 06:14 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> A common bug is where realloc fails to allocate and we assume that
> the memory being realloc'd was freed. This is not the case, the
> NULL return means we need to free the original gpe buffer to avoid
> a memory leak.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_gpe.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/src/lib/src/fwts_gpe.c b/src/lib/src/fwts_gpe.c
> index b09232f1..a16fee86 100644
> --- a/src/lib/src/fwts_gpe.c
> +++ b/src/lib/src/fwts_gpe.c
> @@ -62,12 +62,17 @@ int fwts_gpe_read(fwts_gpe **gpes)
>  	while ((entry = readdir(dir)) != NULL) {
>  		if ((strncmp(entry->d_name, "gpe", 3) == 0) ||
>  		    (strncmp(entry->d_name, "sci",3) == 0)) {
> -			if ((*gpes = realloc(*gpes, sizeof(fwts_gpe) * (n+1))) == NULL)
> +			fwts_gpe *tmp;
> +
> +			tmp = realloc(*gpes, sizeof(fwts_gpe) * (n+1));
> +			if (!tmp) {
> +				free(*gpes);
>  				goto error;
> -			else {
> +			} else {
>  				char path[PATH_MAX];
>  				char *data;
>
> +				*gpes = tmp;
>  				if (((*gpes)[n].name  = strdup(entry->d_name)) == NULL)
>  					goto error;
>
>

Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff mbox

Patch

diff --git a/src/lib/src/fwts_gpe.c b/src/lib/src/fwts_gpe.c
index b09232f1..a16fee86 100644
--- a/src/lib/src/fwts_gpe.c
+++ b/src/lib/src/fwts_gpe.c
@@ -62,12 +62,17 @@  int fwts_gpe_read(fwts_gpe **gpes)
 	while ((entry = readdir(dir)) != NULL) {
 		if ((strncmp(entry->d_name, "gpe", 3) == 0) ||
 		    (strncmp(entry->d_name, "sci",3) == 0)) {
-			if ((*gpes = realloc(*gpes, sizeof(fwts_gpe) * (n+1))) == NULL)
+			fwts_gpe *tmp;
+
+			tmp = realloc(*gpes, sizeof(fwts_gpe) * (n+1));
+			if (!tmp) {
+				free(*gpes);
 				goto error;
-			else {
+			} else {
 				char path[PATH_MAX];
 				char *data;
 
+				*gpes = tmp;
 				if (((*gpes)[n].name  = strdup(entry->d_name)) == NULL)
 					goto error;