On 7/22/23 22:37, Heinrich Schuchardt wrote:
Hi,
diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
index 095443c63d..9246f555e3 100644
--- a/common/spl/spl_legacy.c
+++ b/common/spl/spl_legacy.c
@@ -22,10 +22,7 @@ static void spl_parse_legacy_validate(uintptr_t
start, uintptr_t size)
uintptr_t spl_end = (uintptr_t)_image_binary_end;
uintptr_t end = start + size;
- if ((start >= spl_start && start < spl_end) ||
- (end > spl_start && end <= spl_end) ||
- (start < spl_start && end >= spl_end) ||
- (start > end && end > spl_start))
+ if (end > spl_start && start < spl_end)
panic("SPL: Image overlaps SPL\n");
if (size > CONFIG_SYS_BOOTM_LEN)
Does this handle address space wrap around ? This:
=====blob=====|..............|=====blob=====
.....|==spl==|..............................
Hint: it does not.
The replaced code does not handle wrap around where end is below
spl_start. The following code would cover all cases of image wrap around:
+ if (end > spl_start && start < spl_end
+ panic("SPL: Image overlaps SPL\n");
+ if (start >= end)
+ panic("SPL: Image wraps around\n");
I guess we don't have to test for spl_start >= spl_end.
I would really like someone to double-check this piece of code.
+CC Rasmus.
Look at all the other checks in U-Boot which do, and first factor them
Which other instances do you refer to?
See e.g. bootm_find_images() in boot/bootm.c .
Basically git grep -i overlap and it shows a few more sites.
The lmb library does not to check for wrap around but the wrap around
checks would have to be in other places than lmb_addrs_overlap().
I am kind-of tempted to suggest we should add LMB to SPL to fix this
properly, since that way LMB would cover all vital parts of U-Boot and
assure they won't be overwritten, not just code.
