Re: [PATCH 1/2] image-host: add a check of the return value of snprintf.

Hi Hugo,

On Fri, Dec 29, 2023 at 2:07 PM Hugo Cornelis hugo.cornelis@essensium.com wrote:

This patch allows to generate a sensible error message when generating binary images using very long filenames.

This can happen with Buildroot and Yocto builds.

Signed-off-by: Hugo Cornelis hugo.cornelis@essensium.com

tools/image-host.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-)

Good to see this, but I would like a refactoring, please see below.

diff --git a/tools/image-host.c b/tools/image-host.c index ca4950312f..3719f36117 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -378,6 +378,7 @@ static int fit_image_setup_cipher(struct image_cipher_info *info, char *algo_name; char filename[128]; int ret = -1;

•   int snprintf_return;
  

sret ? We try to use 'ret' for return values. But really I would prefer if you could split this function in two, since it is too long. Then you probably can use 'ret' in both functions.

    if (fit_image_cipher_get_algo(fit, noffset, &algo_name)) {
            fprintf(stderr, "Can't get algo name for cipher in image '%s'\n",

@@ -414,8 +415,18 @@ static int fit_image_setup_cipher(struct image_cipher_info *info, }

    /* Read the key in the file */

•   snprintf(filename, sizeof(filename), "%s/%s%s",
  

•            info->keydir, info->keyname, ".bin");
  

•   snprintf_return = snprintf(filename, sizeof(filename), "%s/%s%s",
  

•                              info->keydir, info->keyname, ".bin");
  

•   if (snprintf_return >= sizeof(filename)) {
  

•           printf("Can't format the key filename when setting up the cipher: insufficient buffer space\n");
  

•           ret = -1;
  

•           goto out;
  

•   }
  

•   if (snprintf_return < 0) {
  

•           printf("Can't format the key filename when setting up the cipher: snprintf error\n");
  

•           ret = -1;
  

•           goto out;
  

•   }
    info->key = malloc(info->cipher->key_len);
    if (!info->key) {
            fprintf(stderr, "Can't allocate memory for key\n");
  

@@ -436,8 +447,18 @@ static int fit_image_setup_cipher(struct image_cipher_info *info,

    if (info->ivname) {
            /* Read the IV in the file */

•           snprintf(filename, sizeof(filename), "%s/%s%s",
  

•                    info->keydir, info->ivname, ".bin");
  

•           snprintf_return = snprintf(filename, sizeof(filename), "%s/%s%s",
  

•                                      info->keydir, info->ivname, ".bin");
  

•           if (snprintf_return >= sizeof(filename)) {
  

•                   printf("Can't format the IV filename when setting up the cipher: insufficient buffer space\n");
  

•                   ret = -1;
  

•                   goto out;
  

•           }
  

•           if (snprintf_return < 0) {
  

•                   printf("Can't format the IV filename when setting up the cipher: snprintf error\n");
  

•                   ret = -1;
  

•                   goto out;
  

•           }
            ret = fit_image_read_data(filename, (unsigned char *)info->iv,
                                      info->cipher->iv_len);
    } else {
  

-- 2.34.1

Regards, Simon