[U-Boot] [PATCH] mkimage: SEGFAULT with imximage on 64 bit systems

[Mike Frysinger <vapier@gentoo.org>]

On Friday 29 January 2010 05:22:13 Stefano Babic wrote:
> Running mkimage to generate an imximage produces a SEGFAULT
> on 64 bit machines due to pointer arithmetic limited to 32 bit.

man this is terrible terrible code.  using 64bit casts may fix 64bit systems, 
but doesnt seem right on 32bit systems as you'd introduce more pointer/size 
mismatches.

just glancing through this file shows other random issues:
	- the structs seem to overlay the file on disk ?  but there's no code to 
do target endianness to host endianness translation ?
	- the structs are missing ((packed)) attributes

> -	ext_header = (flash_cfg_parms_t *) ((uint32_t)&imx_hdr->dcd_table +
> +	ext_header = (flash_cfg_parms_t *) ((uint64_t)&imx_hdr->dcd_table +
>  			sizeof(dcd_preamble_t) + size);

if the only thing you want is imx_hdr->ext_header, why not do it that way:
	ext_header = &imx_hdr->ext_header

hardcoding the layout of a struct into random places in the source is asking 
for unnecessary trouble.

>  	base_offset = fhdr->app_dest_ptr + hdr->flash_offset ;
> -	fhdr->dcd_ptr_ptr = (uint32_t) ((uint32_t)&fhdr->dcd_ptr -
> -		(uint32_t)&fhdr->app_code_jump_vector) + base_offset ;
> +	fhdr->dcd_ptr_ptr = (uint32_t) ((uint64_t)&fhdr->dcd_ptr -
> +		(uint64_t)&fhdr->app_code_jump_vector) + base_offset ;

you dont need casts to do simple pointer arithmetic:
fhdr->dcd_ptr_ptr = (uint32_t) (&fhdr->dcd_ptr - &fhdr->app_code_jump_vector)
	+ base_offset;
(there also shouldnt be a space before that semicolon)

then again, this looks like you're doing constant subtraction.  the distance 
between two struct members is always going to be the same, so why dont you use 
offsetof() to avoid the random pointer ugliness.

>  	/* The external flash header must be at the end of the DCD table */
> -	ext_header = (flash_cfg_parms_t *) ((uint32_t)&hdr->dcd_table +
> +	ext_header = (flash_cfg_parms_t *) ((uint64_t)&hdr->dcd_table +
>  			dcd_len +
>  			sizeof(dcd_preamble_t));

same issue as the first hunk
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100129/83bc403f/attachment.pgp