Patchwork [U-Boot] disk: Fix possible out-of-bounds access in part_efi.c

login
register
mail settings
Submitter Marek Vasut
Date May 19, 2013, 10:53 p.m.
Message ID <1369004014-11395-1-git-send-email-marex@denx.de>
Download mbox | patch
Permalink /patch/244826/
State Accepted
Delegated to: Tom Rini
Headers show

Comments

Marek Vasut - May 19, 2013, 10:53 p.m.
Make sure to never access beyond bounds of either EFI partition name
or DOS partition name. This situation is happening:

part.h:     disk_partition_t->name is 32-byte long
part_efi.h: gpt_entry->partition_name is 36-bytes long

The loop in part_efi.c copies over 36 bytes and thus accesses beyond
the disk_partition_t->name .

Fix this by picking the shortest of source and destination arrays and
make sure the destination array is cleared so the trailing bytes are
zeroed-out and don't cause issues with string manipulation.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@ti.com>
Cc: Simon Glass <sjg@chromium.org>
---
 disk/part_efi.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

NOTE: I didn't test this patch on real hardware! Please review and test
      if possible.
Tom Rini - June 5, 2013, 12:34 p.m.
On Sun, May 19, 2013 at 12:53:34PM -0000, Marek Vasut wrote:

> Make sure to never access beyond bounds of either EFI partition name
> or DOS partition name. This situation is happening:
> 
> part.h:     disk_partition_t->name is 32-byte long
> part_efi.h: gpt_entry->partition_name is 36-bytes long
> 
> The loop in part_efi.c copies over 36 bytes and thus accesses beyond
> the disk_partition_t->name .
> 
> Fix this by picking the shortest of source and destination arrays and
> make sure the destination array is cleared so the trailing bytes are
> zeroed-out and don't cause issues with string manipulation.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Tom Rini <trini@ti.com>
> Cc: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

Patch

diff --git a/disk/part_efi.c b/disk/part_efi.c
index e9987f0..8753aea 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -373,7 +373,7 @@  int gpt_fill_pte(gpt_header *gpt_h, gpt_entry *gpt_e,
 	u32 offset = (u32)le32_to_cpu(gpt_h->first_usable_lba);
 	ulong start;
 	int i, k;
-	size_t name_len;
+	size_t efiname_len, dosname_len;
 #ifdef CONFIG_PARTITION_UUIDS
 	char *str_uuid;
 #endif
@@ -421,9 +421,14 @@  int gpt_fill_pte(gpt_header *gpt_h, gpt_entry *gpt_e,
 		       sizeof(gpt_entry_attributes));
 
 		/* partition name */
-		name_len = sizeof(gpt_e[i].partition_name)
+		efiname_len = sizeof(gpt_e[i].partition_name)
 			/ sizeof(efi_char16_t);
-		for (k = 0; k < name_len; k++)
+		dosname_len = sizeof(partitions[i].name);
+
+		memset(gpt_e[i].partition_name, 0,
+		       sizeof(gpt_e[i].partition_name));
+
+		for (k = 0; k < min(dosname_len, efiname_len); k++)
 			gpt_e[i].partition_name[k] =
 				(efi_char16_t)(partitions[i].name[k]);