[PATCH v2 1/1] spl: spl_legacy: clean up spl_parse_legacy_validate

Simplify the check for an overlap of the loaded image and SPL.
Detect all cases of wrap around.
Use the SPL_TPL_NAME prefix to avoid printing 'SPL' in TPL (both spl_parse_legacy_header and spl_parse_legacy_validate).
Fixes: 77aed22b48ab ("spl: spl_legacy: Add extra address checks") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v2: consider wrap around fix TPL prefix
@Marek:
You suggested to carve out a function for memory region overlaps. A function call for two comparisons would increase code size.
Introducing LMB for SPL would run into code size limitations and is beyond the scope of a simple fix. --- common/spl/spl_legacy.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c index 095443c63d..dd91e1077b 100644 --- a/common/spl/spl_legacy.c +++ b/common/spl/spl_legacy.c @@ -22,14 +22,14 @@ 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)) - panic("SPL: Image overlaps SPL\n"); + if (end > spl_start && start < spl_end) + panic(SPL_TPL_NAME ": Image overlaps SPL\n"); + + if (start >= end) + panic(SPL_TPL_NAME ": Image wraps around\n");
if (size > CONFIG_SYS_BOOTM_LEN) - panic("SPL: Image too large\n"); + panic(SPL_TPL_NAME ": Image too large\n"); }
int spl_parse_legacy_header(struct spl_image_info *spl_image, @@ -40,7 +40,7 @@ int spl_parse_legacy_header(struct spl_image_info *spl_image, /* check uImage header CRC */ if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_CRC_CHECK) && !image_check_hcrc(header)) { - puts("SPL: Image header CRC check failed!\n"); + puts(SPL_TPL_NAME ": Image header CRC check failed!\n"); return -EINVAL; }
@@ -76,7 +76,7 @@ int spl_parse_legacy_header(struct spl_image_info *spl_image, spl_image->name, spl_image->load_addr, spl_image->size);
spl_parse_legacy_validate(spl_image->load_addr, spl_image->size); - spl_parse_legacy_validate(spl_image->entry_point, 0); + spl_parse_legacy_validate(spl_image->entry_point, 1);
return 0; }

On 7/25/23 10:30, Heinrich Schuchardt wrote:
Simplify the check for an overlap of the loaded image and SPL.
Detect all cases of wrap around.
Use the SPL_TPL_NAME prefix to avoid printing 'SPL' in TPL (both spl_parse_legacy_header and spl_parse_legacy_validate).
Fixes: 77aed22b48ab ("spl: spl_legacy: Add extra address checks") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v2: consider wrap around fix TPL prefix
@Marek:
You suggested to carve out a function for memory region overlaps. A function call for two comparisons would increase code size.
Even if the function is inlined ?
Introducing LMB for SPL would run into code size limitations and is beyond the scope of a simple fix.
common/spl/spl_legacy.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c index 095443c63d..dd91e1077b 100644 --- a/common/spl/spl_legacy.c +++ b/common/spl/spl_legacy.c @@ -22,14 +22,14 @@ 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))
panic("SPL: Image overlaps SPL\n");
- if (end > spl_start && start < spl_end)
panic(SPL_TPL_NAME ": Image overlaps SPL\n");
- if (start >= end)
Really >= ? start == end means zero-size payload, no ?
Btw the extra string that is being printed here also increases code size, which might blow on at91.
participants (2)
-
Heinrich Schuchardt
-
Marek Vasut