[U-Boot] [PATCH v4 2/7] efi_loader: Fix memory map size check to avoid out-of-bounds access

[U-Boot] [PATCH v4 2/7] efi_loader: Fix memory map size check to avoid out-of-bounds access

[Stefan Brüns]

The current efi_get_memory_map() function overwrites the map_size
property before reading its value. That way the sanity check whether our
memory map fits into the given array always succeeds, potentially
overwriting arbitrary payload memory.

This patch moves the property update write after its sanity check, so
that the check actually verifies the correct value.

So far this has not triggered any known bugs, but we're better off safe
than sorry.

If the buffer is to small, the returned memory_map_size indicates the
required size to the caller.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 lib/efi_loader/efi_memory.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index ebe8e94..1d23783 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -336,6 +336,7 @@ efi_status_t efi_get_memory_map(unsigned long *memory_map_size,
 	ulong map_size = 0;
 	int map_entries = 0;
 	struct list_head *lhandle;
+	unsigned long provided_map_size = *memory_map_size;
 
 	list_for_each(lhandle, &efi_mem)
 		map_entries++;
@@ -350,7 +351,7 @@ efi_status_t efi_get_memory_map(unsigned long *memory_map_size,
 	if (descriptor_version)
 		*descriptor_version = EFI_MEMORY_DESCRIPTOR_VERSION;
 
-	if (*memory_map_size < map_size)
+	if (provided_map_size < map_size)
 		return EFI_BUFFER_TOO_SMALL;
 
 	/* Copy list into array */
-- 
2.10.0

[U-Boot] [PATCH v4 2/7] efi_loader: Fix memory map size check to avoid out-of-bounds access

[Alexander Graf]

On 10/09/2016 10:17 PM, Stefan Br?ns wrote:
> The current efi_get_memory_map() function overwrites the map_size
> property before reading its value. That way the sanity check whether our
> memory map fits into the given array always succeeds, potentially
> overwriting arbitrary payload memory.
>
> This patch moves the property update write after its sanity check, so
> that the check actually verifies the correct value.
>
> So far this has not triggered any known bugs, but we're better off safe
> than sorry.
>
> If the buffer is to small, the returned memory_map_size indicates the
> required size to the caller.
>
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>

Reviewed-by: Alexander Graf <agraf@suse.de>


Alex

[U-Boot] [U-Boot, v4, 2/7] efi_loader: Fix memory map size check to avoid out-of-bounds access

[Alexander Graf]

> The current efi_get_memory_map() function overwrites the map_size
> property before reading its value. That way the sanity check whether our
> memory map fits into the given array always succeeds, potentially
> overwriting arbitrary payload memory.
> 
> This patch moves the property update write after its sanity check, so
> that the check actually verifies the correct value.
> 
> So far this has not triggered any known bugs, but we're better off safe
> than sorry.
> 
> If the buffer is to small, the returned memory_map_size indicates the
> required size to the caller.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Alexander Graf <agraf@suse.de>

Thanks, applied to