Hi Patrick,
From: Patrick Oppenlander patrick.oppenlander@gmail.com
Previously, mkimage -F could be run multiple times causing already ciphered image data to be ciphered again.
Good catch, mkimage -F cipher data that are already ciphered, generating a broken FIT image.
Signed-off-by: Patrick Oppenlander patrick.oppenlander@gmail.com
tools/image-host.c | 47 +++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 15 deletions(-)
diff --git a/tools/image-host.c b/tools/image-host.c index 87ef79ef53..12de9b5ec0 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -397,33 +397,43 @@ int fit_image_write_cipher(void *fit, int image_noffset, int noffset, const void *data, size_t size, unsigned char *data_ciphered, int data_ciphered_len) {
- int ret = -1;
- /*
- fit_image_cipher_data() uses the presence of the data-size-unciphered
- property as a sentinel to detect whether the data for this image is
- already encrypted. This is important as:
- 'mkimage -F' can be run multiple times on a FIT image
- This function is in a retry loop to handle ENOSPC
- */
- /* add non ciphered data size */
- int ret;
- /* Add unciphered data size */
ret = fdt_setprop_u32(fit, image_noffset, "data-size-unciphered", size);
- if (ret == -FDT_ERR_NOSPACE) {
- ret = -ENOSPC;
- goto out;
- }
- if (ret == -FDT_ERR_NOSPACE)
- return -ENOSPC;
if (ret) { printf("Can't add unciphered data size (err = %d)\n", ret);
- goto out;
- return -EIO;
}
- /* Add ciphered data */
- /* Replace contents of data property with data_ciphered */
ret = fdt_setprop(fit, image_noffset, FIT_DATA_PROP, data_ciphered, data_ciphered_len); if (ret == -FDT_ERR_NOSPACE) {
- ret = -ENOSPC;
- goto out;
- /* Remove data-size-unciphered; data is not ciphered */
- ret = fdt_delprop(fit, image_noffset, "data-size-unciphered");
- if (ret) {
- printf("Can't remove unciphered data size (err = %d)\n", ret);
- return -EIO;
- }
- return -ENOSPC;
} if (ret) {
- printf("Can't add ciphered data (err = %d)\n", ret);
- goto out;
- printf("Can't replace data with ciphered data (err = %d)\n", ret);
- return -EIO;
}
- out:
- return ret;
- return 0;
}
As for the second patch, I think that the loop is not an issue because it always start with "fresh/clean" value (using a backup file).
So I am not sure that changes in this function are needed.
static int @@ -482,7 +492,7 @@ int fit_image_cipher_data(const char *keydir, void *keydest, const char *image_name; const void *data; size_t size;
- int cipher_node_offset;
- int cipher_node_offset, len;
/* Get image name */ image_name = fit_get_name(fit, image_noffset, NULL); @@ -497,6 +507,13 @@ int fit_image_cipher_data(const char *keydir, void *keydest, return -1; }
- /* Don't cipher ciphered data */
- if (fdt_getprop(fit, image_noffset, "data-size-unciphered", &len))
- return 0;
- if (len != -FDT_ERR_NOTFOUND) {
- printf("Failure testing for data-size-unciphered\n");
- return -1;
- }
From my point of view, it fixes an issue. But I see this solution more
as "workaround" than a clean solution.
As it fixes a real issue, we may start with it and then try to found a "clean" solution.
/* Process cipher node if present */ cipher_node_offset = fdt_subnode_offset(fit, image_noffset, "cipher"); -- 2.27.0
Regards, Philippe