[PATCH] partitions: efi: fix overflow issues while allocating gpt entries

[PATCH] partitions: efi: fix overflow issues while allocating gpt entries

[Abdelrahman Youssef]

while parsting the GPT header in alloc_read_gpt_entries() the number
of partitions can be large that multiplying it with the size of a single
partition overflows 32-bit multiplication.

we already enforce a MAX_PARTITION limit of 128 partitions in efi_partition(),
so allowing any bigger value in alloc_read_gpt_entries() would fail,
even if we fix the overflow.

Therefore, we can enforce the limit strictly and treat any overflow as
a failing condition.

Signed-off-by: Abdelrahman Youssef <abdelrahmanyossef12@gmail.com>
---
 common/partitions/efi.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/common/partitions/efi.c b/common/partitions/efi.c
index 9a04b7014d..8014579b67 100644
--- a/common/partitions/efi.c
+++ b/common/partitions/efi.c
@@ -35,6 +35,25 @@ struct efi_partition {
 
 static const int force_gpt = IS_ENABLED(CONFIG_PARTITION_DISK_EFI_GPT_NO_FORCE);
 
+/**
+* compute_partitions_entries_size() - return the size of all partitions
+* @gpt: GPT header
+*
+* Description: return size of all partitions, 0 on error
+*
+* This is a helper function that compute the size of all partitions
+* by multiplying the size of a single partition by the number of partitions
+*/
+static u32 compute_partitions_entries_size(const gpt_header *gpt) {
+	u32 nb_parts, sz_parts, total_size;
+
+	nb_parts = min(MAX_PARTITION, le32_to_cpu(gpt->num_partition_entries));
+	sz_parts = le32_to_cpu(gpt->sizeof_partition_entry);
+	if (check_mul_overflow(nb_parts, sz_parts, &total_size))
+		return 0;
+	return total_size;
+}
+
 /**
  * efi_crc32() - EFI version of crc32 function
  * @buf: buffer to calculate crc32 of
@@ -81,14 +100,12 @@ static u64 last_lba(struct block_device *bdev)
 static gpt_entry *alloc_read_gpt_entries(struct block_device *blk,
 					 gpt_header * pgpt_head)
 {
-	size_t count = 0;
+	u32 count = 0;
 	gpt_entry *pte = NULL;
 	unsigned long from, size;
 	int ret;
 
-	count = le32_to_cpu(pgpt_head->num_partition_entries) *
-		le32_to_cpu(pgpt_head->sizeof_partition_entry);
-
+	count = compute_partitions_entries_size(pgpt_head);
 	if (!count)
 		return NULL;
 
@@ -156,7 +173,7 @@ static gpt_header *alloc_read_gpt_header(struct block_device *blk,
 static int is_gpt_valid(struct block_device *blk, u64 lba,
 			gpt_header **gpt, gpt_entry **ptes)
 {
-	u32 crc, origcrc;
+	u32 crc, origcrc, count;
 	u64 lastlba;
 
 	if (!ptes)
@@ -215,10 +232,13 @@ static int is_gpt_valid(struct block_device *blk, u64 lba,
 	if (!(*ptes = alloc_read_gpt_entries(blk, *gpt)))
 		goto fail;
 
+	/* Check the size of all partitions */
+	count = compute_partitions_entries_size(*gpt);
+	if (!count)
+		goto fail_ptes;
+
 	/* Check the GUID Partition Table Entry Array CRC */
-	crc = efi_crc32((const unsigned char *)*ptes,
-		le32_to_cpu((*gpt)->num_partition_entries) *
-		le32_to_cpu((*gpt)->sizeof_partition_entry));
+	crc = efi_crc32((const unsigned char *)*ptes, count);
 
 	if (crc != le32_to_cpu((*gpt)->partition_entry_array_crc32)) {
 		dev_dbg(blk->dev, "GUID Partitition Entry Array CRC check failed: 0x%08x 0x%08x\n",
-- 
2.43.0

Re: [PATCH] partitions: efi: fix overflow issues while allocating gpt entries

[AbdelRahman Yossef]

Hi,

This is just a quick reminder if the patch got missed for some reason.
Please let me know if there is anything that needs to get fixed.

Cheers,
Abdelrahman


On Thu, Oct 31, 2024 at 3:49 PM Abdelrahman Youssef
<abdelrahmanyossef12@gmail.com> wrote:
>
> while parsting the GPT header in alloc_read_gpt_entries() the number
> of partitions can be large that multiplying it with the size of a single
> partition overflows 32-bit multiplication.
>
> we already enforce a MAX_PARTITION limit of 128 partitions in efi_partition(),
> so allowing any bigger value in alloc_read_gpt_entries() would fail,
> even if we fix the overflow.
>
> Therefore, we can enforce the limit strictly and treat any overflow as
> a failing condition.
>
> Signed-off-by: Abdelrahman Youssef <abdelrahmanyossef12@gmail.com>
> ---
>  common/partitions/efi.c | 36 ++++++++++++++++++++++++++++--------
>  1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/common/partitions/efi.c b/common/partitions/efi.c
> index 9a04b7014d..8014579b67 100644
> --- a/common/partitions/efi.c
> +++ b/common/partitions/efi.c
> @@ -35,6 +35,25 @@ struct efi_partition {
>
>  static const int force_gpt = IS_ENABLED(CONFIG_PARTITION_DISK_EFI_GPT_NO_FORCE);
>
> +/**
> +* compute_partitions_entries_size() - return the size of all partitions
> +* @gpt: GPT header
> +*
> +* Description: return size of all partitions, 0 on error
> +*
> +* This is a helper function that compute the size of all partitions
> +* by multiplying the size of a single partition by the number of partitions
> +*/
> +static u32 compute_partitions_entries_size(const gpt_header *gpt) {
> +       u32 nb_parts, sz_parts, total_size;
> +
> +       nb_parts = min(MAX_PARTITION, le32_to_cpu(gpt->num_partition_entries));
> +       sz_parts = le32_to_cpu(gpt->sizeof_partition_entry);
> +       if (check_mul_overflow(nb_parts, sz_parts, &total_size))
> +               return 0;
> +       return total_size;
> +}
> +
>  /**
>   * efi_crc32() - EFI version of crc32 function
>   * @buf: buffer to calculate crc32 of
> @@ -81,14 +100,12 @@ static u64 last_lba(struct block_device *bdev)
>  static gpt_entry *alloc_read_gpt_entries(struct block_device *blk,
>                                          gpt_header * pgpt_head)
>  {
> -       size_t count = 0;
> +       u32 count = 0;
>         gpt_entry *pte = NULL;
>         unsigned long from, size;
>         int ret;
>
> -       count = le32_to_cpu(pgpt_head->num_partition_entries) *
> -               le32_to_cpu(pgpt_head->sizeof_partition_entry);
> -
> +       count = compute_partitions_entries_size(pgpt_head);
>         if (!count)
>                 return NULL;
>
> @@ -156,7 +173,7 @@ static gpt_header *alloc_read_gpt_header(struct block_device *blk,
>  static int is_gpt_valid(struct block_device *blk, u64 lba,
>                         gpt_header **gpt, gpt_entry **ptes)
>  {
> -       u32 crc, origcrc;
> +       u32 crc, origcrc, count;
>         u64 lastlba;
>
>         if (!ptes)
> @@ -215,10 +232,13 @@ static int is_gpt_valid(struct block_device *blk, u64 lba,
>         if (!(*ptes = alloc_read_gpt_entries(blk, *gpt)))
>                 goto fail;
>
> +       /* Check the size of all partitions */
> +       count = compute_partitions_entries_size(*gpt);
> +       if (!count)
> +               goto fail_ptes;
> +
>         /* Check the GUID Partition Table Entry Array CRC */
> -       crc = efi_crc32((const unsigned char *)*ptes,
> -               le32_to_cpu((*gpt)->num_partition_entries) *
> -               le32_to_cpu((*gpt)->sizeof_partition_entry));
> +       crc = efi_crc32((const unsigned char *)*ptes, count);
>
>         if (crc != le32_to_cpu((*gpt)->partition_entry_array_crc32)) {
>                 dev_dbg(blk->dev, "GUID Partitition Entry Array CRC check failed: 0x%08x 0x%08x\n",
> --
> 2.43.0
>

Re: [PATCH] partitions: efi: fix overflow issues while allocating gpt entries

[Sascha Hauer]

On Thu, 31 Oct 2024 15:48:52 +0300, Abdelrahman Youssef wrote:
> while parsting the GPT header in alloc_read_gpt_entries() the number
> of partitions can be large that multiplying it with the size of a single
> partition overflows 32-bit multiplication.
> 
> we already enforce a MAX_PARTITION limit of 128 partitions in efi_partition(),
> so allowing any bigger value in alloc_read_gpt_entries() would fail,
> even if we fix the overflow.
> 
> [...]

Applied, thanks!

[1/1] partitions: efi: fix overflow issues while allocating gpt entries
      https://git.pengutronix.de/cgit/barebox/commit/?id=ae8ec8dc9d6e (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>