diff mbox series

[U-Boot] efi: fix memory calculation overflow on 32-bit systems

Message ID 20190409205830.GA5818@nyx.fritz.box
State Accepted, archived
Commit 60fd8844af5c47f606680f287db4585d783c1964
Delegated to: Heinrich Schuchardt
Headers show
Series [U-Boot] efi: fix memory calculation overflow on 32-bit systems | expand

Commit Message

Patrick Wildt April 9, 2019, 8:58 p.m. UTC
Hi,

There are Cubox-i machines out there with nearly 4 GiB of RAM.  The
RAM starts at 0x10000000 with a size of 0xf0000000.  Thus the end
of RAM is at 0x100000000.  This overflows a 32-bit integer, which
should be fine since in the EFI memory code the variables used are
all 64-bit with a fixed size.  Unfortunately EFI_PAGE_MASK, which is
used in the EFI memory code to remove the lower bits, is based on
the EFI_PAGE_SIZE macro which, uses 1UL with a shift.  This means
the resulting mask is UL, which is only 32-bit on ARMv7.  Use ULL to
make sure that even on 32-bit platforms we use a 64-bit long mask.
Without this there will be no memory available in the EFI memory map
and bootefi will fail allocating pages.

Best regards,
Patrick

Comments

Heinrich Schuchardt April 10, 2019, 12:36 a.m. UTC | #1
On 4/9/19 10:58 PM, Patrick Wildt wrote:
> Hi,
>
> There are Cubox-i machines out there with nearly 4 GiB of RAM.  The
> RAM starts at 0x10000000 with a size of 0xf0000000.  Thus the end
> of RAM is at 0x100000000.  This overflows a 32-bit integer, which
> should be fine since in the EFI memory code the variables used are
> all 64-bit with a fixed size.  Unfortunately EFI_PAGE_MASK, which is
> used in the EFI memory code to remove the lower bits, is based on
> the EFI_PAGE_SIZE macro which, uses 1UL with a shift.  This means
> the resulting mask is UL, which is only 32-bit on ARMv7.  Use ULL to
> make sure that even on 32-bit platforms we use a 64-bit long mask.
> Without this there will be no memory available in the EFI memory map
> and bootefi will fail allocating pages.
>
> Best regards,
> Patrick

We are using `& ~EFI_PAGE_MASK` in multiplaces for 64bit operations. So
it makes sense to use a 64bit value.

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

>
> diff --git a/include/efi.h b/include/efi.h
> index d98441ab19d..3c9d20f8c0b 100644
> --- a/include/efi.h
> +++ b/include/efi.h
> @@ -190,7 +190,7 @@ enum efi_mem_type {
>  #define EFI_MEM_DESC_VERSION	1
>
>  #define EFI_PAGE_SHIFT		12
> -#define EFI_PAGE_SIZE		(1UL << EFI_PAGE_SHIFT)
> +#define EFI_PAGE_SIZE		(1ULL << EFI_PAGE_SHIFT)
>  #define EFI_PAGE_MASK		(EFI_PAGE_SIZE - 1)
>
>  struct efi_mem_desc {
>
Patrick DELAUNAY April 11, 2019, 8:38 a.m. UTC | #2
Hi Patrick,

> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Patrick Wildt
> 
> Hi,
> 
> There are Cubox-i machines out there with nearly 4 GiB of RAM.  The RAM starts
> at 0x10000000 with a size of 0xf0000000.  Thus the end of RAM is at
> 0x100000000.  This overflows a 32-bit integer, which should be fine since in the
> EFI memory code the variables used are all 64-bit with a fixed size.  Unfortunately
> EFI_PAGE_MASK, which is used in the EFI memory code to remove the lower
> bits, is based on the EFI_PAGE_SIZE macro which, uses 1UL with a shift.  This
> means the resulting mask is UL, which is only 32-bit on ARMv7.  Use ULL to
> make sure that even on 32-bit platforms we use a 64-bit long mask.
> Without this there will be no memory available in the EFI memory map and bootefi
> will fail allocating pages.
> 
> Best regards,
> Patrick
> 
> diff --git a/include/efi.h b/include/efi.h index d98441ab19d..3c9d20f8c0b 100644
> --- a/include/efi.h
> +++ b/include/efi.h
> @@ -190,7 +190,7 @@ enum efi_mem_type {
>  #define EFI_MEM_DESC_VERSION	1
> 
>  #define EFI_PAGE_SHIFT		12
> -#define EFI_PAGE_SIZE		(1UL << EFI_PAGE_SHIFT)
> +#define EFI_PAGE_SIZE		(1ULL << EFI_PAGE_SHIFT)
>  #define EFI_PAGE_MASK		(EFI_PAGE_SIZE - 1)
> 
>  struct efi_mem_desc {

Same issue for stm32mp157c-ev1 board (32bits platform with 1GB at 0xC0000000).

Patched tested on v2019.04 and my issue is solved and it is a better approach that my patch http://patchwork.ozlabs.org/patch/1083262/

So I will abandon my path and 

Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com>


> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot


Regards

Patrick
diff mbox series

Patch

diff --git a/include/efi.h b/include/efi.h
index d98441ab19d..3c9d20f8c0b 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -190,7 +190,7 @@  enum efi_mem_type {
 #define EFI_MEM_DESC_VERSION	1
 
 #define EFI_PAGE_SHIFT		12
-#define EFI_PAGE_SIZE		(1UL << EFI_PAGE_SHIFT)
+#define EFI_PAGE_SIZE		(1ULL << EFI_PAGE_SHIFT)
 #define EFI_PAGE_MASK		(EFI_PAGE_SIZE - 1)
 
 struct efi_mem_desc {