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

Hi Heinrich, On Sat, 2 Sept 2023 at 01:34, Heinrich Schuchardt heinrich.schuchardt@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@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