[U-Boot] mkimage (kwbimage) unmapping more bytes than it maps

[Chris Packham]

Hi,

Mark and I have been looking at a problem we've been experiencing on
our build system.

We're running a number of similarly configured makers and seemingly
randomly one will fail to build one of our kirkwood based targets
(specifically the SBx81LIFXCAT that was recently added) with a
segfault in mkimage. When I tried to run the same build it would work
fine on my PC (in theory the same base install as the makers).

I'd been blaming specific makers as having bad RAM (these things get
hammered so we've had them die before) but Mark managed to track it
down to mmap and munmap disagreeing on the size of the area mapped.
You can see this by running strace on mkimage

strace -y ./tools/mkimage -n ./arch/arm/mach-mvebu/kwbimage.cfg -T
kwbimage -a 0x00800000 -e 0x00800000 -d u-boot.img u-boot-spl.kwb
...
mmap(NULL, 641024, PROT_READ|PROT_WRITE, MAP_SHARED, 3, 0) = 0x7f1b5ce3e000
...
munmap(0x7f1b5ce3e000, 641028) = 0

Note that the size has been incremented by 4. Most of the time this is
harmless but on our build system the size of the image worked out to
an exact multiple of the page size so the munmap ended up spilling
over and unmapping another page which depending on the in-memory
layout of the process could result in a segfault.

This extra 4 bytes comes from kwbimage.c. Towards the end of
kwbimage_set_header() we have

checksum = cpu_to_le32(image_checksum32((uint32_t *)ptr, sbuf->st_size));
size = write(ifd, &checksum, sizeof(uint32_t));
...
sbuf->st_size += sizeof(uint32_t);

The st_size increment seems like a leftover from commit 4acd2d24b651
("tools: kwbimage: Add image version 1 support for Armada XP / 370")
as aside from the munmap in mkimage nothing else uses it.

This is where Mark and I disagree. A minimal change would simply to
remove the increment of st_size from kwbimage.c. But a more robust fix
would be to have mkimage not trust sbuf after it's been passed to
tparams->set_header.

Is there are preference for how to handle this? I'll follow up this
mail with Marks proposed fix.