Re: [PATCH] mkimage: zero fill alignment space

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 30.10.2015 16:49, Andrei Borzenkov wrote:
> This did not cause real problem but is good for reproducible builds. I hit
> it with recent bootinfoscript that displays embedded config; I was puzzled
> by random garbage at the end.
> 
> Also remove redundant zeroing code where we fill in the whole memory block
> anyway.
> 
Could we just zero-out the whole block when we allocate it to have an
easier code and avoid it happening again?
> ---
>  util/mkimage.c | 29 +++++++++++++----------------
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/util/mkimage.c b/util/mkimage.c
> index 35df998..1c522fe 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -992,7 +992,7 @@ grub_install_generate_image (const char *dir, const char 
> *prefix,
>  {
>    char *kernel_img, *core_img;
>    size_t kernel_size, total_module_size, core_size, exec_size;
> -  size_t memdisk_size = 0, config_size = 0, config_size_pure = 0;
> +  size_t memdisk_size = 0, memdisk_size_pure = 0, config_size = 0, 
> config_size_pure = 0;
>    size_t prefix_size = 0;
>    char *kernel_path;
>    size_t offset;
> @@ -1035,7 +1035,8 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>  
>    if (memdisk_path)
>      {
> -      memdisk_size = ALIGN_UP(grub_util_get_image_size (memdisk_path), 512);
> +      memdisk_size_pure = grub_util_get_image_size (memdisk_path);
> +      memdisk_size = ALIGN_UP(memdisk_size_pure, 512);
>        grub_util_info ("the size of memory disk is 0x%" 
> GRUB_HOST_PRIxLONG_LONG,
>                     (unsigned long long) memdisk_size);
>        total_module_size += memdisk_size + sizeof (struct grub_module_header);
> @@ -1043,8 +1044,8 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>  
>    if (config_path)
>      {
> -      config_size_pure = grub_util_get_image_size (config_path) + 1;
> -      config_size = ALIGN_ADDR (config_size_pure);
> +      config_size_pure = grub_util_get_image_size (config_path);
> +      config_size = ALIGN_ADDR (config_size_pure + 1);
>        grub_util_info ("the size of config file is 0x%" 
> GRUB_HOST_PRIxLONG_LONG,
>                     (unsigned long long) config_size);
>        total_module_size += config_size + sizeof (struct grub_module_header);
> @@ -1140,19 +1141,21 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>      size_t i;
>      for (i = 0; i < npubkeys; i++)
>        {
> -     size_t curs;
>       struct grub_module_header *header;
> +     size_t key_size, orig_size;
>  
> -     curs = grub_util_get_image_size (pubkey_paths[i]);
> +     orig_size = grub_util_get_image_size (pubkey_paths[i]);
> +     key_size = ALIGN_ADDR (orig_size);
>  
>       header = (struct grub_module_header *) (kernel_img + offset);
>       memset (header, 0, sizeof (struct grub_module_header));
>       header->type = grub_host_to_target32 (OBJ_TYPE_PUBKEY);
> -     header->size = grub_host_to_target32 (curs + sizeof (*header));
> +     header->size = grub_host_to_target32 (key_size + sizeof (*header));
>       offset += sizeof (*header);
>  
>       grub_util_load_image (pubkey_paths[i], kernel_img + offset);
> -     offset += ALIGN_ADDR (curs);
> +     memset (kernel_img + offset + orig_size, 0, key_size - orig_size);
> +     offset += key_size;
>        }
>    }
>  
> @@ -1167,6 +1170,7 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>        offset += sizeof (*header);
>  
>        grub_util_load_image (memdisk_path, kernel_img + offset);
> +      memset (kernel_img + offset + memdisk_size_pure, 0, memdisk_size - 
> memdisk_size_pure);
>        offset += memdisk_size;
>      }
>  
> @@ -1181,7 +1185,7 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>        offset += sizeof (*header);
>  
>        grub_util_load_image (config_path, kernel_img + offset);
> -      *(kernel_img + offset + config_size_pure - 1) = 0;
> +      memset (kernel_img + offset + config_size_pure, 0, config_size - 
> config_size_pure);
>        offset += config_size;
>      }
>  
> @@ -1267,17 +1271,10 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>             = grub_host_to_target_addr (image_target->link_addr);
>       }
>        full_size = core_size + decompress_size;
> -
>        full_img = xmalloc (full_size);
> -      memset (full_img, 0, full_size); 
> -
>        memcpy (full_img, decompress_img, decompress_size);
> -
>        memcpy (full_img + decompress_size, core_img, core_size);
>  
> -      memset (full_img + decompress_size + core_size, 0,
> -           full_size - (decompress_size + core_size));
> -
>        free (core_img);
>        core_img = full_img;
>        core_size = full_size;
>

signature.asc
Description: OpenPGP digital signature