[PATCH v2 3/4] cmd: gpt: fix calc_parts_list_len()

[Simon Glass]

Hi Heinrich,
On Sat, 2 Sept 2023 at 01:34, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> * Avoid incrementing by moving comma into strlen("uuid_disk=,") and
>   considering NUL byte.
> * Appending a UUID only adds UUID_STR_LEN bytes.
>   Don't count the terminating NUL.
> * The length of the hexadecimal representation of lba_int is
>   2 * sizeof(lba_int).
> * We don't use a 'MiB' postfix but a '0x' prefix.

Why do we want the 0x prefix?

> * The uuid field is only needed if configured.
>
> Fixes: 2fcaa413b3f6 ("gpt: harden set_gpt_info() against non NULL-terminated strings")
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
> v2:
>         new patch
> ---
>  cmd/gpt.c | 33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> index 092f7defff..e7a53747fc 100644
> --- a/cmd/gpt.c
> +++ b/cmd/gpt.c
> @@ -162,22 +162,29 @@ static bool found_key(const char *str, const char *key)
>         return result;
>  }
>
> +/**
> + * calc_parts_list_len() - get size of partition table description
> + *
> + * @numparts:  number of partitions
> + * Return:     string size including terminating NUL
> + */
>  static int calc_parts_list_len(int numparts)
>  {
> -       int partlistlen = UUID_STR_LEN + 1 + strlen("uuid_disk=");
> -       /* for the comma */
> -       partlistlen++;
> -
> -       /* per-partition additions; numparts starts at 1, so this should be correct */
> -       partlistlen += numparts * (strlen("name=,") + PART_NAME_LEN + 1);
> +       /* number of hexadecimal digits of the lbaint_t representation */
> +       const int lbaint_size = 2 * sizeof(lbaint_t);
> +       int partlistlen;
> +
> +       /* media description including terminating NUL */
> +       partlistlen = strlen("uuid_disk=;") + UUID_STR_LEN + 1;
> +       /* per-partition descriptions; numparts */
> +       partlistlen += numparts * (strlen("name=,") + PART_NAME_LEN);
>         /* see part.h for definition of struct disk_partition */
> -       partlistlen += numparts * (strlen("start=MiB,") + sizeof(lbaint_t) + 1);
> -       partlistlen += numparts * (strlen("size=MiB,") + sizeof(lbaint_t) + 1);
> -       partlistlen += numparts * (strlen("uuid=;") + UUID_STR_LEN + 1);
> -       /* for the terminating null */
> -       partlistlen++;
> -       debug("Length of partitions_list is %d for %d partitions\n", partlistlen,
> -             numparts);
> +       partlistlen += numparts * (strlen("start=0x,") + lbaint_size);
> +       partlistlen += numparts * (strlen("size=0x,") + lbaint_size);
> +       if (IS_ENABLED(CONFIG_PARTITION_UUIDS))
> +               partlistlen += numparts * (strlen("uuid=;") + UUID_STR_LEN);
> +       debug("Length of partitions_list is %d for %d partitions\n",
> +             partlistlen, numparts);
>         return partlistlen;
>  }
>
> --
> 2.40.1
>

Regards,
Simon