Add a unit test that locks down the fix in imagemap_map(): a larger request against a region recorded by imagemap_map_to() (which the caller owns and is not LMB-reserved) falls through to a fresh LMB allocation instead of resizing the caller's memory.
The test reserves a caller-owned buffer inside the LMB-managed RAM, records a 64-byte region there via imagemap_map_to() with a guard pattern beyond the payload, then issues a 256-byte imagemap_map() at the same base. It verifies the returned pointer differs from the caller's buffer, the guard tail is unchanged, and the new region is LMB-reserved. Signed-off-by: Daniel Golle <[email protected]> --- test/boot/imagemap.c | 97 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/test/boot/imagemap.c b/test/boot/imagemap.c index c7289db3995..e038f5377cd 100644 --- a/test/boot/imagemap.c +++ b/test/boot/imagemap.c @@ -316,6 +316,103 @@ static int imagemap_test_map_to(struct unit_test_state *uts) IMAGEMAP_TEST(imagemap_test_map_to, UTF_DM); +/* + * Test: map() must NOT resize a region the caller recorded via map_to(). + * + * The caller reserves (owns) its own load address through the LMB, then + * asks imagemap_map_to() to load 64 bytes there and records it as a + * caller-owned region (lmb_reserved = false). A later, larger request at + * the same base must fall through to a fresh LMB allocation, leaving the + * caller's memory untouched. + * + * Without the guard in imagemap_map() this path would lmb_free() the + * caller's reservation, re-reserve the same address and re-read in place, + * clobbering whatever the caller kept beyond the 64 requested bytes. + * + * dst must live inside the LMB-managed RAM (here: a caller-reserved + * buffer) so the buggy in-place re-read actually happens; a plain stack + * buffer sits outside that region and would mask the bug. + * + * Mirrors the scenario Simon Glass asked to be locked down: a guard + * pattern in dst beyond the requested 64 bytes must survive, the returned + * pointer must differ from dst, and the new region must be LMB-reserved. + */ +static int imagemap_test_map_to_not_resized(struct unit_test_state *uts) +{ + struct udevice *dev; + struct imagemap_priv *priv; + struct mock_priv *mock; + u8 image[IMAGE_SIZE]; + phys_addr_t dst_phys; + u8 *dst; + const ulong dst_size = 512; + u8 guard[512 - 64]; + struct imagemap_region *r; + void *p; + + for (int i = 0; i < IMAGE_SIZE; i++) + image[i] = (u8)(i & 0xff); + + ut_assertok(create_mock_loader(image, IMAGE_SIZE, &dev)); + mock = get_mock_priv(dev); + priv = dev_get_uclass_priv(dev); + + /* Caller reserves its own load address and owns the memory */ + ut_assertok(lmb_alloc_mem(LMB_MEM_ALLOC_ANY, ARCH_DMA_MINALIGN, + &dst_phys, dst_size, LMB_NONE)); + dst = map_sysmem(dst_phys, dst_size); + + /* Distinct guard pattern beyond the 64 requested payload bytes */ + memset(guard, 0xa5, sizeof(guard)); + memcpy(dst + 64, guard, sizeof(guard)); + + /* + * Caller records a 64-byte region at its chosen address; imagemap + * must NOT claim an LMB reservation for it. + */ + p = imagemap_map_to(dev, 0, 64, dst); + ut_assert(!IS_ERR(p)); + ut_asserteq_ptr(dst, p); + ut_asserteq_mem(image, dst, 64); + r = alist_get(&priv->regions, 0, struct imagemap_region); + ut_assert(r && !r->lmb_reserved); + ut_asserteq(1, priv->regions.count); + + /* + * Larger request at the same base: must fall through to a fresh + * LMB allocation rather than resizing the caller's region. + */ + p = imagemap_map(dev, 0, 256); + ut_assert(!IS_ERR(p)); + /* Returned pointer must be a different buffer */ + ut_assert((char *)p != (char *)dst); + /* Caller's guard tail must remain intact */ + ut_asserteq_mem(guard, dst + 64, sizeof(guard)); + /* The new region is LMB-reserved and holds the full 256 bytes */ + ut_assert(lmb_is_reserved_flags(map_to_sysmem(p), LMB_NONE)); + ut_asserteq_mem(image, p, 256); + + /* + * The fall-through replaced the caller's record via + * imagemap_record() (same img_offset); the table keeps one entry, + * now LMB-reserved (the fresh allocation). + */ + ut_asserteq(1, priv->regions.count); + ut_assert(alist_get(&priv->regions, 0, struct imagemap_region)->lmb_reserved); + + imagemap_cleanup(dev); + + /* + * The caller's own reservation was never taken over by imagemap and + * is still ours to release. + */ + lmb_free(dst_phys, dst_size, LMB_NONE); + + return 0; +} + +IMAGEMAP_TEST(imagemap_test_map_to_not_resized, UTF_DM); + /* Test: lookup() returns NULL for unmapped ranges */ static int imagemap_test_lookup_miss(struct unit_test_state *uts) { -- 2.56.0
