Just caught up with all the patches and the final one indeed looks good to me
too!
Kind Regards,
Jordy
> On January 22, 2020 2:20 AM Simon Goldschmidt
> <simon.k.r.goldschm...@gmail.com> wrote:
>
>
> On Tue, Jan 21, 2020 at 5:53 PM Tom Rini <tr...@konsulko.com> wrote:
> >
> > New analysis by the tool has shown that we have some cases where we
> > weren't handling the error exit condition correctly. When we ran into
> > the ENOMEM case we wouldn't exit the function and thus incorrect things
> > could happen. Rework the unwinding such that we don't need a helper
> > function now and free what we may have allocated.
> >
> > Fixes: 18030d04d25d ("GPT: fix memory leaks identified by Coverity")
> > Reported-by: Coverity (CID: 275475, 275476)
> > Cc: Alison Chaiken <ali...@she-devel.com>
> > Cc: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com>
> > Cc: Jordy <jo...@simplyhacker.com>
> > Signed-off-by: Tom Rini <tr...@konsulko.com>
>
> Looks good to me.
>
> Reviewed-by: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com>
>
> > ---
> > Changes in v3:
> > - Move del_gpt_info() call into the unwind as set_gpt_info() will have
> > been called at that point and we need to clear it. (Simon)
> > Changes in v2:
> > - Initialize str_disk_guid to NULL to be sure we can test that it has
> > been set later on (Simon, Jordy).
> > ---
> > cmd/gpt.c | 47 ++++++++++++-----------------------------------
> > 1 file changed, 12 insertions(+), 35 deletions(-)
> >
> > diff --git a/cmd/gpt.c b/cmd/gpt.c
> > index 0c4349f4b249..964702bad43e 100644
> > --- a/cmd/gpt.c
> > +++ b/cmd/gpt.c
> > @@ -633,21 +633,6 @@ static int do_disk_guid(struct blk_desc *dev_desc,
> > char * const namestr)
> > }
> >
> > #ifdef CONFIG_CMD_GPT_RENAME
> > -/*
> > - * There are 3 malloc() calls in set_gpt_info() and there is no info about
> > which
> > - * failed.
> > - */
> > -static void set_gpt_cleanup(char **str_disk_guid,
> > - disk_partition_t **partitions)
> > -{
> > -#ifdef CONFIG_RANDOM_UUID
> > - if (str_disk_guid)
> > - free(str_disk_guid);
> > -#endif
> > - if (partitions)
> > - free(partitions);
> > -}
> > -
> > static int do_rename_gpt_parts(struct blk_desc *dev_desc, char *subcomm,
> > char *name1, char *name2)
> > {
> > @@ -655,7 +640,7 @@ static int do_rename_gpt_parts(struct blk_desc
> > *dev_desc, char *subcomm,
> > struct disk_part *curr;
> > disk_partition_t *new_partitions = NULL;
> > char disk_guid[UUID_STR_LEN + 1];
> > - char *partitions_list, *str_disk_guid;
> > + char *partitions_list, *str_disk_guid = NULL;
> > u8 part_count = 0;
> > int partlistlen, ret, numparts = 0, partnum, i = 1, ctr1 = 0, ctr2
> > = 0;
> >
> > @@ -697,14 +682,8 @@ static int do_rename_gpt_parts(struct blk_desc
> > *dev_desc, char *subcomm,
> > /* set_gpt_info allocates new_partitions and str_disk_guid */
> > ret = set_gpt_info(dev_desc, partitions_list, &str_disk_guid,
> > &new_partitions, &part_count);
> > - if (ret < 0) {
> > - del_gpt_info();
> > - free(partitions_list);
> > - if (ret == -ENOMEM)
> > - set_gpt_cleanup(&str_disk_guid, &new_partitions);
> > - else
> > - goto out;
> > - }
> > + if (ret < 0)
> > + goto out;
> >
> > if (!strcmp(subcomm, "swap")) {
> > if ((strlen(name1) > PART_NAME_LEN) || (strlen(name2) >
> > PART_NAME_LEN)) {
> > @@ -766,14 +745,8 @@ static int do_rename_gpt_parts(struct blk_desc
> > *dev_desc, char *subcomm,
> > * Even though valid pointers are here passed into set_gpt_info(),
> > * it mallocs again, and there's no way to tell which failed.
> > */
> > - if (ret < 0) {
> > - del_gpt_info();
> > - free(partitions_list);
> > - if (ret == -ENOMEM)
> > - set_gpt_cleanup(&str_disk_guid, &new_partitions);
> > - else
> > - goto out;
> > - }
> > + if (ret < 0)
> > + goto out;
> >
> > debug("Writing new partition table\n");
> > ret = gpt_restore(dev_desc, disk_guid, new_partitions, numparts);
> > @@ -795,10 +768,14 @@ static int do_rename_gpt_parts(struct blk_desc
> > *dev_desc, char *subcomm,
> > }
> > printf("new partition table with %d partitions is:\n", numparts);
> > print_gpt_info();
> > - del_gpt_info();
> > out:
> > - free(new_partitions);
> > - free(str_disk_guid);
> > + del_gpt_info();
> > +#ifdef CONFIG_RANDOM_UUID
> > + if (str_disk_guid)
> > + free(str_disk_guid);
> > +#endif
> > + if (new_partitions)
> > + free(new_partitions);
> > free(partitions_list);
> > return ret;
> > }
> > --
> > 2.17.1
> >
