Skip to contentSkip to conversation

lib: lmb: fix overflow in __lmb_alloc_base w/ large RAM

PATCH In review Stephen Warren 2 participants 3 messages Jul 31, 2014 – Aug 10, 2014 Library code

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 lib: lmb: fix overflow in __lmb_alloc_base w/ large RAM

From: Stephen Warren <[email protected]>

If a 32-bit system has 2GB of RAM, and the base address of that RAM is 2GB, then start+size will overflow a 32-bit value (to a value of 0).

__lmb_alloc_base is affected by this; it calculates the minimum of (start+size of RAM) and max_addr. However, when start+size is 0, it is always less than max_addr, which causes the value of max_addr not to be taken into account when restricting the allocation's location.

Fix this by calculating start+size separately, and if that calculation underflows, using -1 (interpreted as the max unsigned value) as the value instead, and then taking the min of that and max_addr. Now that start+size doesn't overflow, it's typically large, and max_addr dominates the min() call, and is taken into account.

The user-visible symptom of this bug is that CONFIG_BOOTMAP_SZ is ignored on Tegra124 systems with 2GB of RAM, which in turn causes the DT to be relocated at the very end of RAM, which the ARM Linux kernel doesn't map during early boot, and which causes boot failures. With this fix, CONFIG_BOOTMAP_SZ correctly restricts the relocated DT to a much lower address, and everything works.

---
 lib/lmb.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
lib/lmb.c +4 -1
index 49a3c9e01e59..41a2be463565 100644
@@ -295,7 +295,10 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align, phy
 		if (max_addr == LMB_ALLOC_ANYWHERE)
 			base = lmb_align_down(lmbbase + lmbsize - size, align);
 		else if (lmbbase < max_addr) {
-			base = min(lmbbase + lmbsize, max_addr);
+			base = lmbbase + lmbsize;
+			if (base < lmbbase)
+				base = -1;
+			base = min(base, max_addr);
 			base = lmb_align_down(base - size, align);
 		} else
 			continue;
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 lib: lmb: fix overflow in __lmb_alloc_base w/ large RAM

Quoted 16 lines · Stephen Warren

From: Stephen Warren <[email protected]>

If a 32-bit system has 2GB of RAM, and the base address of that RAM is 2GB, then start+size will overflow a 32-bit value (to a value of 0).

__lmb_alloc_base is affected by this; it calculates the minimum of (start+size of RAM) and max_addr. However, when start+size is 0, it is always less than max_addr, which causes the value of max_addr not to be taken into account when restricting the allocation's location.

Fix this by calculating start+size separately, and if that calculation underflows, using -1 (interpreted as the max unsigned value) as the value instead, and then taking the min of that and max_addr. Now that start+size doesn't overflow, it's typically large, and max_addr dominates the min() call, and is taken into account.

Quoted 6 lines

The user-visible symptom of this bug is that CONFIG_BOOTMAP_SZ is ignored on Tegra124 systems with 2GB of RAM, which in turn causes the DT to be relocated at the very end of RAM, which the ARM Linux kernel doesn't map during early boot, and which causes boot failures. With this fix, CONFIG_BOOTMAP_SZ correctly restricts the relocated DT to a much lower address, and everything works.

TomR,

Does this patch look OK to you? I imagine that TomW is holding off applying "ARM: tegra: Use mem size from MC rather than ODMDATA" since it depends on this patch, which isn't applied yet.

Thanks.

#2 lore raw
In reply to Stephen Warren
Quoted 16 lines · Stephen Warren

From: Stephen Warren <[email protected]>

If a 32-bit system has 2GB of RAM, and the base address of that RAM is 2GB, then start+size will overflow a 32-bit value (to a value of 0).

__lmb_alloc_base is affected by this; it calculates the minimum of (start+size of RAM) and max_addr. However, when start+size is 0, it is always less than max_addr, which causes the value of max_addr not to be taken into account when restricting the allocation's location.

Fix this by calculating start+size separately, and if that calculation underflows, using -1 (interpreted as the max unsigned value) as the value instead, and then taking the min of that and max_addr. Now that start+size doesn't overflow, it's typically large, and max_addr dominates the min() call, and is taken into account.

Quoted 8 lines

The user-visible symptom of this bug is that CONFIG_BOOTMAP_SZ is ignored on Tegra124 systems with 2GB of RAM, which in turn causes the DT to be relocated at the very end of RAM, which the ARM Linux kernel doesn't map during early boot, and which causes boot failures. With this fix, CONFIG_BOOTMAP_SZ correctly restricts the relocated DT to a much lower address, and everything works.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140810/f61dc96b/attachment.pgp>