[PATCH 1/4] tools: kwbimage: Verify supported image version

Only image versions 0 and 1 are supported. Verify it in kwbimage_verify_header() function.
Signed-off-by: Pali Rohár pali@kernel.org --- tools/kwbimage.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 4bff02bb3fb5..80aae8a6b619 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -1690,9 +1690,7 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, if (checksum != ext_hdr->checksum) return -FDT_ERR_BADSTRUCTURE; } - } - - if (image_version((void *)ptr) == 1) { + } else if (image_version((void *)ptr) == 1) { struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr; uint32_t offset; uint32_t size; @@ -1762,6 +1760,8 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, if (image_checksum32(ptr + offset, size - 4) != *(uint32_t *)(ptr + offset + size - 4)) return -FDT_ERR_BADSTRUCTURE; + } else { + return -FDT_ERR_BADSTRUCTURE; }
return 0;

Check that extended image header size is not larger than file size.
Signed-off-by: Pali Rohár pali@kernel.org --- tools/kwbimage.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 80aae8a6b619..84c41210e39e 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -1682,6 +1682,9 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, if (mhdr->ext & 0x1) { struct ext_hdr_v0 *ext_hdr;
+ if (header_size + sizeof(*ext_hdr) > image_size) + return -FDT_ERR_BADSTRUCTURE; + ext_hdr = (struct ext_hdr_v0 *) (ptr + sizeof(struct main_hdr_v0)); checksum = image_checksum8(ext_hdr,

On 11.08.21 10:14, Pali Rohár wrote:
Check that extended image header size is not larger than file size.
Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
tools/kwbimage.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 80aae8a6b619..84c41210e39e 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -1682,6 +1682,9 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, if (mhdr->ext & 0x1) { struct ext_hdr_v0 *ext_hdr;
if (header_size + sizeof(*ext_hdr) > image_size)
return -FDT_ERR_BADSTRUCTURE;
ext_hdr = (struct ext_hdr_v0 *) (ptr + sizeof(struct main_hdr_v0)); checksum = image_checksum8(ext_hdr,
Viele Grüße, Stefan

On 11.08.21 10:14, Pali Rohár wrote:
Check that extended image header size is not larger than file size.
Signed-off-by: Pali Rohár pali@kernel.org
Applied to u-boot-marvell/master
Thanks, Stefan
tools/kwbimage.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 80aae8a6b619..84c41210e39e 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -1682,6 +1682,9 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, if (mhdr->ext & 0x1) { struct ext_hdr_v0 *ext_hdr;
if (header_size + sizeof(*ext_hdr) > image_size)
return -FDT_ERR_BADSTRUCTURE;
ext_hdr = (struct ext_hdr_v0 *) (ptr + sizeof(struct main_hdr_v0)); checksum = image_checksum8(ext_hdr,
Viele Grüße, Stefan

Part of image data is 4 byte checksum, so every image must contain at least 4 bytes. Verify it to prevent memory corruptions.
Signed-off-by: Pali Rohár pali@kernel.org --- tools/kwbimage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 84c41210e39e..5ac34ac5e9c8 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -1757,7 +1757,7 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, return -FDT_ERR_BADSTRUCTURE;
size = le32_to_cpu(mhdr->blocksize); - if (offset + size > image_size || size % 4 != 0) + if (size < 4 || offset + size > image_size || size % 4 != 0) return -FDT_ERR_BADSTRUCTURE;
if (image_checksum32(ptr + offset, size - 4) !=

On 11.08.21 10:14, Pali Rohár wrote:
Part of image data is 4 byte checksum, so every image must contain at least 4 bytes. Verify it to prevent memory corruptions.
Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
tools/kwbimage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 84c41210e39e..5ac34ac5e9c8 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -1757,7 +1757,7 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, return -FDT_ERR_BADSTRUCTURE;
size = le32_to_cpu(mhdr->blocksize);
if (offset + size > image_size || size % 4 != 0)
if (size < 4 || offset + size > image_size || size % 4 != 0) return -FDT_ERR_BADSTRUCTURE;
if (image_checksum32(ptr + offset, size - 4) !=
Viele Grüße, Stefan

On 11.08.21 10:14, Pali Rohár wrote:
Part of image data is 4 byte checksum, so every image must contain at least 4 bytes. Verify it to prevent memory corruptions.
Signed-off-by: Pali Rohár pali@kernel.org
Applied to u-boot-marvell/master
Thanks, Stefan
tools/kwbimage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 84c41210e39e..5ac34ac5e9c8 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -1757,7 +1757,7 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, return -FDT_ERR_BADSTRUCTURE;
size = le32_to_cpu(mhdr->blocksize);
if (offset + size > image_size || size % 4 != 0)
if (size < 4 || offset + size > image_size || size % 4 != 0) return -FDT_ERR_BADSTRUCTURE;
if (image_checksum32(ptr + offset, size - 4) !=
Viele Grüße, Stefan

There are already IBR_HDR_* constants for these numbers, so use them.
Signed-off-by: Pali Rohár pali@kernel.org --- tools/kwbimage.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 5ac34ac5e9c8..c1269fb6633a 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -59,13 +59,13 @@ struct hash_v1 { };
struct boot_mode boot_modes[] = { - { 0x4D, "i2c" }, - { 0x5A, "spi" }, - { 0x8B, "nand" }, - { 0x78, "sata" }, - { 0x9C, "pex" }, - { 0x69, "uart" }, - { 0xAE, "sdio" }, + { IBR_HDR_I2C_ID, "i2c" }, + { IBR_HDR_SPI_ID, "spi" }, + { IBR_HDR_NAND_ID, "nand" }, + { IBR_HDR_SATA_ID, "sata" }, + { IBR_HDR_PEX_ID, "pex" }, + { IBR_HDR_UART_ID, "uart" }, + { IBR_HDR_SDIO_ID, "sdio" }, {}, };
@@ -75,10 +75,10 @@ struct nand_ecc_mode { };
struct nand_ecc_mode nand_ecc_modes[] = { - { 0x00, "default" }, - { 0x01, "hamming" }, - { 0x02, "rs" }, - { 0x03, "disabled" }, + { IBR_HDR_ECC_DEFAULT, "default" }, + { IBR_HDR_ECC_FORCED_HAMMING, "hamming" }, + { IBR_HDR_ECC_FORCED_RS, "rs" }, + { IBR_HDR_ECC_DISABLED, "disabled" }, {}, };

On 11.08.21 10:14, Pali Rohár wrote:
There are already IBR_HDR_* constants for these numbers, so use them.
Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
tools/kwbimage.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 5ac34ac5e9c8..c1269fb6633a 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -59,13 +59,13 @@ struct hash_v1 { };
struct boot_mode boot_modes[] = {
- { 0x4D, "i2c" },
- { 0x5A, "spi" },
- { 0x8B, "nand" },
- { 0x78, "sata" },
- { 0x9C, "pex" },
- { 0x69, "uart" },
- { 0xAE, "sdio" },
- { IBR_HDR_I2C_ID, "i2c" },
- { IBR_HDR_SPI_ID, "spi" },
- { IBR_HDR_NAND_ID, "nand" },
- { IBR_HDR_SATA_ID, "sata" },
- { IBR_HDR_PEX_ID, "pex" },
- { IBR_HDR_UART_ID, "uart" },
- { IBR_HDR_SDIO_ID, "sdio" }, {}, };
@@ -75,10 +75,10 @@ struct nand_ecc_mode { };
struct nand_ecc_mode nand_ecc_modes[] = {
- { 0x00, "default" },
- { 0x01, "hamming" },
- { 0x02, "rs" },
- { 0x03, "disabled" },
- { IBR_HDR_ECC_DEFAULT, "default" },
- { IBR_HDR_ECC_FORCED_HAMMING, "hamming" },
- { IBR_HDR_ECC_FORCED_RS, "rs" },
- { IBR_HDR_ECC_DISABLED, "disabled" }, {}, };
Viele Grüße, Stefan

On 11.08.21 10:14, Pali Rohár wrote:
There are already IBR_HDR_* constants for these numbers, so use them.
Signed-off-by: Pali Rohár pali@kernel.org
Applied to u-boot-marvell/master
Thanks, Stefan
tools/kwbimage.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 5ac34ac5e9c8..c1269fb6633a 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -59,13 +59,13 @@ struct hash_v1 { };
struct boot_mode boot_modes[] = {
- { 0x4D, "i2c" },
- { 0x5A, "spi" },
- { 0x8B, "nand" },
- { 0x78, "sata" },
- { 0x9C, "pex" },
- { 0x69, "uart" },
- { 0xAE, "sdio" },
- { IBR_HDR_I2C_ID, "i2c" },
- { IBR_HDR_SPI_ID, "spi" },
- { IBR_HDR_NAND_ID, "nand" },
- { IBR_HDR_SATA_ID, "sata" },
- { IBR_HDR_PEX_ID, "pex" },
- { IBR_HDR_UART_ID, "uart" },
- { IBR_HDR_SDIO_ID, "sdio" }, {}, };
@@ -75,10 +75,10 @@ struct nand_ecc_mode { };
struct nand_ecc_mode nand_ecc_modes[] = {
- { 0x00, "default" },
- { 0x01, "hamming" },
- { 0x02, "rs" },
- { 0x03, "disabled" },
- { IBR_HDR_ECC_DEFAULT, "default" },
- { IBR_HDR_ECC_FORCED_HAMMING, "hamming" },
- { IBR_HDR_ECC_FORCED_RS, "rs" },
- { IBR_HDR_ECC_DISABLED, "disabled" }, {}, };
Viele Grüße, Stefan

On 11.08.21 10:14, Pali Rohár wrote:
Only image versions 0 and 1 are supported. Verify it in kwbimage_verify_header() function.
Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
tools/kwbimage.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 4bff02bb3fb5..80aae8a6b619 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -1690,9 +1690,7 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, if (checksum != ext_hdr->checksum) return -FDT_ERR_BADSTRUCTURE; }
- }
- if (image_version((void *)ptr) == 1) {
- } else if (image_version((void *)ptr) == 1) { struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr; uint32_t offset; uint32_t size;
@@ -1762,6 +1760,8 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, if (image_checksum32(ptr + offset, size - 4) != *(uint32_t *)(ptr + offset + size - 4)) return -FDT_ERR_BADSTRUCTURE;
} else {
return -FDT_ERR_BADSTRUCTURE;
}
return 0;
Viele Grüße, Stefan

On 11.08.21 10:14, Pali Rohár wrote:
Only image versions 0 and 1 are supported. Verify it in kwbimage_verify_header() function.
Signed-off-by: Pali Rohár pali@kernel.org
Applied to u-boot-marvell/master
Thanks, Stefan
tools/kwbimage.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 4bff02bb3fb5..80aae8a6b619 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -1690,9 +1690,7 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, if (checksum != ext_hdr->checksum) return -FDT_ERR_BADSTRUCTURE; }
- }
- if (image_version((void *)ptr) == 1) {
- } else if (image_version((void *)ptr) == 1) { struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr; uint32_t offset; uint32_t size;
@@ -1762,6 +1760,8 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, if (image_checksum32(ptr + offset, size - 4) != *(uint32_t *)(ptr + offset + size - 4)) return -FDT_ERR_BADSTRUCTURE;
} else {
return -FDT_ERR_BADSTRUCTURE;
}
return 0;
Viele Grüße, Stefan
participants (2)
-
Pali Rohár
-
Stefan Roese