diff mbox series

[1/1] diskpart: fix createtable memory leak

Message ID 20210624164827.32171-1-james.hilliard1@gmail.com
State Changes Requested
Headers show
Series [1/1] diskpart: fix createtable memory leak | expand

Commit Message

James Hilliard June 24, 2021, 4:48 p.m. UTC
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 handlers/diskpart_handler.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Comments

Stefano Babic June 24, 2021, 4:57 p.m. UTC | #1
On 24.06.21 18:48, James Hilliard wrote:
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>   handlers/diskpart_handler.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c
> index acd21d4..dfdb55a 100644
> --- a/handlers/diskpart_handler.c
> +++ b/handlers/diskpart_handler.c
> @@ -212,7 +212,7 @@ static int diskpart_assign_context(struct fdisk_context **cxt,struct img_type *i
>   		 * This also lets us access the parent context.
>   		 */
>   		*cxt = fdisk_new_nested_context(parent, "dos");
> -		if (!cxt) {
> +		if (!*cxt) {
>   			ERROR("Failed to allocate libfdisk nested context");
>   			return -ENOMEM;
>   		}
> @@ -691,7 +691,7 @@ static int diskpart(struct img_type *img,
>   	unsigned long i;
>   	unsigned long hybrid = 0;
>   	struct hnd_priv priv =  {FDISK_DISKLABEL_DOS};
> -	struct create_table *createtable = (struct create_table *)calloc(1, sizeof(struct create_table));
> +	struct create_table *createtable = NULL;
>   
>   	if (!lbtype || (strcmp(lbtype, "gpt") && strcmp(lbtype, "dos"))) {
>   		ERROR("Just GPT or DOS partition table are supported");
> @@ -807,6 +807,13 @@ static int diskpart(struct img_type *img,
>   		goto handler_release;
>   	}
>   
> +	createtable = calloc(1, sizeof(*createtable));
> +	if (!createtable) {
> +		ERROR("OOM allocating createtable !");
> +		ret = -ENOMEM;
> +		goto handler_release;
> +	}
> +
>   	ret = diskpart_assign_context(&cxt, img, priv, hybrid, createtable);
>   	if (ret == -EACCES)
>   		goto handler_release;
> @@ -827,7 +834,8 @@ static int diskpart(struct img_type *img,
>   	oldtb = calloc(1, sizeof(*oldtb));
>   	if (!oldtb) {
>   		ERROR("OOM loading partitions !");
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto handler_exit;
>   	}
>   
>   	/*
> @@ -912,6 +920,9 @@ handler_release:
>   		free(part);
>   	}
>   
> +	if (createtable)
> +		free(createtable);
> +
>   	return ret;
>   }
>   
> 

Thanks for fixing coverity's issues !

Acked-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano
James Hilliard June 24, 2021, 5:02 p.m. UTC | #2
On Thu, Jun 24, 2021 at 10:57 AM Stefano Babic <sbabic@denx.de> wrote:
>
> On 24.06.21 18:48, James Hilliard wrote:
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> >   handlers/diskpart_handler.c | 17 ++++++++++++++---
> >   1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c
> > index acd21d4..dfdb55a 100644
> > --- a/handlers/diskpart_handler.c
> > +++ b/handlers/diskpart_handler.c
> > @@ -212,7 +212,7 @@ static int diskpart_assign_context(struct fdisk_context **cxt,struct img_type *i
> >                * This also lets us access the parent context.
> >                */
> >               *cxt = fdisk_new_nested_context(parent, "dos");
> > -             if (!cxt) {
> > +             if (!*cxt) {
> >                       ERROR("Failed to allocate libfdisk nested context");
> >                       return -ENOMEM;
> >               }
> > @@ -691,7 +691,7 @@ static int diskpart(struct img_type *img,
> >       unsigned long i;
> >       unsigned long hybrid = 0;
> >       struct hnd_priv priv =  {FDISK_DISKLABEL_DOS};
> > -     struct create_table *createtable = (struct create_table *)calloc(1, sizeof(struct create_table));
> > +     struct create_table *createtable = NULL;
> >
> >       if (!lbtype || (strcmp(lbtype, "gpt") && strcmp(lbtype, "dos"))) {
> >               ERROR("Just GPT or DOS partition table are supported");
> > @@ -807,6 +807,13 @@ static int diskpart(struct img_type *img,
> >               goto handler_release;
> >       }
> >
> > +     createtable = calloc(1, sizeof(*createtable));
> > +     if (!createtable) {
> > +             ERROR("OOM allocating createtable !");
> > +             ret = -ENOMEM;
> > +             goto handler_release;
> > +     }
> > +
> >       ret = diskpart_assign_context(&cxt, img, priv, hybrid, createtable);
> >       if (ret == -EACCES)
> >               goto handler_release;
> > @@ -827,7 +834,8 @@ static int diskpart(struct img_type *img,
> >       oldtb = calloc(1, sizeof(*oldtb));
> >       if (!oldtb) {
> >               ERROR("OOM loading partitions !");
> > -             return -ENOMEM;
> > +             ret = -ENOMEM;
> > +             goto handler_exit;
> >       }
> >
> >       /*
> > @@ -912,6 +920,9 @@ handler_release:
> >               free(part);
> >       }
> >
> > +     if (createtable)
> > +             free(createtable);
> > +
> >       return ret;
> >   }
> >
> >
>
> Thanks for fixing coverity's issues !

I think this might have had a bug still, sent a v2 just now.

>
> Acked-by: Stefano Babic <sbabic@denx.de>
>
> Best regards,
> Stefano
>
>
> --
> =====================================================================
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
> =====================================================================
diff mbox series

Patch

diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c
index acd21d4..dfdb55a 100644
--- a/handlers/diskpart_handler.c
+++ b/handlers/diskpart_handler.c
@@ -212,7 +212,7 @@  static int diskpart_assign_context(struct fdisk_context **cxt,struct img_type *i
 		 * This also lets us access the parent context.
 		 */
 		*cxt = fdisk_new_nested_context(parent, "dos");
-		if (!cxt) {
+		if (!*cxt) {
 			ERROR("Failed to allocate libfdisk nested context");
 			return -ENOMEM;
 		}
@@ -691,7 +691,7 @@  static int diskpart(struct img_type *img,
 	unsigned long i;
 	unsigned long hybrid = 0;
 	struct hnd_priv priv =  {FDISK_DISKLABEL_DOS};
-	struct create_table *createtable = (struct create_table *)calloc(1, sizeof(struct create_table));
+	struct create_table *createtable = NULL;
 
 	if (!lbtype || (strcmp(lbtype, "gpt") && strcmp(lbtype, "dos"))) {
 		ERROR("Just GPT or DOS partition table are supported");
@@ -807,6 +807,13 @@  static int diskpart(struct img_type *img,
 		goto handler_release;
 	}
 
+	createtable = calloc(1, sizeof(*createtable));
+	if (!createtable) {
+		ERROR("OOM allocating createtable !");
+		ret = -ENOMEM;
+		goto handler_release;
+	}
+
 	ret = diskpart_assign_context(&cxt, img, priv, hybrid, createtable);
 	if (ret == -EACCES)
 		goto handler_release;
@@ -827,7 +834,8 @@  static int diskpart(struct img_type *img,
 	oldtb = calloc(1, sizeof(*oldtb));
 	if (!oldtb) {
 		ERROR("OOM loading partitions !");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto handler_exit;
 	}
 
 	/*
@@ -912,6 +920,9 @@  handler_release:
 		free(part);
 	}
 
+	if (createtable)
+		free(createtable);
+
 	return ret;
 }