On Wed, 2016-01-20 at 09:15 +0100, Uwe Kleine-König wrote:
> The images that can be sent to a Marvell CPU have a fixed format. Do
> some sanity checks before actually sending an image for easier diagnosis
> of broken files.
>
> Signed-off-by: Uwe Kleine-König <uwe@xxxxxxxxxxxxxxxxx>
> ---
> Changes since (implicit) v1, sent with
> Message-Id: 1453276010-4669-1-git-send-email-uwe@xxxxxxxxxxxxxxxxx:
>
> - whitespace fix
> - error out if a problem is detected
> - add a commit log
>
> scripts/kwboot.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/scripts/kwboot.c b/scripts/kwboot.c
> index 46328d8ed006..06e58f6a7e3b 100644
> --- a/scripts/kwboot.c
> +++ b/scripts/kwboot.c
> @@ -546,6 +546,59 @@ out:
> return rc;
> }
>
> +static int
> +kwboot_check_image(unsigned char *img, size_t size)
Make it const unsigned char?
> +{
> + size_t i;
> + size_t header_size, image_size;
> + unsigned char csum = 0;
> +
> + switch (img[0x0]) {
> + case 0x5a: /* SPI/NOR */
> + case 0x69: /* UART0 */
> + case 0x78: /* SATA */
> + case 0x8b: /* NAND */
> + case 0x9c: /* PCIe */
> + break;
> + default:
> + printf("Unknown boot source: 0x%hhx\n", img[0x0]);
> + goto err;
> + }
> +
> + if (img[0x8] != 1) {
> + printf("Unknown version: 0x%hhx\n", img[0x8]);
> + goto err;
> + }
If you're verifying the image, maybe also check that size > 8 before
reading img[0x8]? Otherwise you could crash instead of rejecting a too
small image.
And having compared the version tag to 1, couldn't you then cast img
into a struct main_hdr_v1 *? That would avoid all the hard coded magic
offsets in the rest of the code. Unless img isn't aligned?
> +
> + image_size = img[0x4] | (img[0x5] << 8) |
> + (img[0x6] << 16) | (img[0x7] << 24);
struct main_hdr_v1 *hdr = (const struct main_hdr
image_size = le32_to_cpu(hdr->blocksize);
or if unaligned:
image_size = get_unaligned_le32(&img[0x4]);
> +
> + header_size = (img[0x9] << 16) | img[0xa] | (img[0xb] << 8);
header_size = hdr->headersz_msb << 16 | le16_to_cpu(hdr->headersz_lsb);
> +
> + if (header_size + image_size != size) {
> + printf("Size mismatch (%zu + %zu != %zu)\n",
> + header_size, image_size, size);
> + goto err;
> + } else {
Don't really need the else block here since the failure above exits,
just like the two failure checks before this one.
> + for (i = 0; i < header_size; ++i)
> + csum += img[i];
csum = image_checksum8(img, header_size)
> +
> + csum -= img[0x1f];
> +
> + if (csum != img[0x1f]) {
> + printf("Checksum mismatch: header: 0x%02hhx, calculated: 0x%02hhx\n",
> + img[0x1f], csum);
> + goto err;
> + }
> + }
> +
> + return 0;
> +
> +err:
> + errno = EINVAL;
> + return 1;
> +}
> +
> static void *
> kwboot_mmap_image(const char *path, size_t *size, int prot)
> {
> @@ -574,6 +627,8 @@ kwboot_mmap_image(const char *path, size_t *size, int prot)
>
> rc = 0;
> *size = st.st_size;
> +
> + rc = kwboot_check_image(img, *size);
> out:
> if (rc && img) {
> munmap(img, st.st_size);
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
