arm: mvebu: kwbimage: inline function to fix use-after-free
PATCH 1/1 arm: mvebu: kwbimage: inline function to fix use-after-free
image_version_file()'s only use is to return the version number of the specified image, and it's only called by kwbimage_generate(). This version function mallocs "image_cfg" and reads the contents of the image into that buffer. Before return to its caller it frees the buffer.
After extracting the version, kwb_image_generate() tries to calculate the header size by calling image_headersz_v1(). This function now accesses "image_cfg", which has already been freed.
Since image_version_file() is only used by a single function, inline it into kwbimage_generate() and only free the buffer after it is no longer needed. This also improves code readability since the code is mostly equal to kwbimage_set_header().
--- tools/kwbimage.c | 93 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 48 insertions(+), 45 deletions(-)
tools/kwbimage.c +48 -45
@@ -1452,47 +1452,6 @@ static int image_get_version(void)return e->version; } -static int image_version_file (const char *input) -{ -FILE *fcfg; -int version; -int ret; - - fcfg =fopen (input,"r" ); -if (!fcfg) { -fprintf (stderr,"Could not open input file %s\n" , input); -return -1 ; - } - - image_cfg =malloc (IMAGE_CFG_ELEMENT_MAX * -sizeof (struct image_cfg_element )); -if (!image_cfg) { -fprintf (stderr,"Cannot allocate memory\n" ); -fclose (fcfg); -return -1 ; - } - -memset (image_cfg,0 , -IMAGE_CFG_ELEMENT_MAX *sizeof (struct image_cfg_element )); -rewind (fcfg); - - ret =image_create_config_parse (fcfg); -fclose (fcfg); -if (ret) { -free (image_cfg); -return -1 ; - } - - version =image_get_version (); -/* Fallback to version 0 is no version is provided in the cfg file */ -if (version ==-1 ) - version =0 ; - -free (image_cfg); - -return version; -} -static void kwbimage_set_header (void *ptr,struct stat *sbuf,int ifd,struct image_tool_params *params) {
@@ -1633,18 +1592,62 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size,static int kwbimage_generate (struct image_tool_params *params,struct image_type_params *tparams) { +FILE *fcfg;int alloc_len; +int version;void *hdr; -int version =0 ; +int ret; - version =image_version_file (params->imagename); -if (version ==0 ) { + fcfg =fopen (params->imagename,"r" ); +if (!fcfg) { +fprintf (stderr,"Could not open input file %s\n" , + params->imagename); +exit (EXIT_FAILURE ); + } + + image_cfg =malloc (IMAGE_CFG_ELEMENT_MAX * +sizeof (struct image_cfg_element )); +if (!image_cfg) { +fprintf (stderr,"Cannot allocate memory\n" ); +fclose (fcfg); +exit (EXIT_FAILURE ); + } + +memset (image_cfg,0 , +IMAGE_CFG_ELEMENT_MAX *sizeof (struct image_cfg_element )); +rewind (fcfg); + + ret =image_create_config_parse (fcfg); +fclose (fcfg); +if (ret) { +free (image_cfg); +exit (EXIT_FAILURE ); + } + + version =image_get_version (); +switch (version) { +/* +* Fallback to version 0 if no version is provided in the +* cfg file +*/ +case -1 : +case 0 : alloc_len =sizeof (struct main_hdr_v0 ) +sizeof (struct ext_hdr_v0 ); - }else { +break ; + +case 1 : alloc_len =image_headersz_v1 (NULL ); +break ; + +default : +fprintf (stderr,"Unsupported version %d\n" , version); +free (image_cfg); +exit (EXIT_FAILURE ); } +free (image_cfg); + hdr =malloc (alloc_len);if (!hdr) { fprintf(stderr, "%s: malloc return failure: %s\n",
Signature · 1 line
2.12.2
PATCH 1/1 arm: mvebu: kwbimage: inline function to fix use-after-free
Quoted 9 lines · Patrick Wildt
image_version_file()'s only use is to return the version number of the specified image, and it's only called by
kwbimage_generate(). This version function mallocs "image_cfg" and reads the contents of the image into that buffer. Before return to its caller it frees the buffer.After extracting the version,
kwb_image_generate()tries to calculate the header size by callingimage_headersz_v1(). This function now accesses "image_cfg", which has already been freed.
Quoted 6 lines
Since
image_version_file()is only used by a single function, inline it intokwbimage_generate()and only free the buffer after it is no longer needed. This also improves code readability since the code is mostly equal tokwbimage_set_header().Signed-off-by Patrick Wildt <[email protected]>
Applied to u-boot-marvell/master.
Thanks,
Stefan