[LinuxBIOS] [PATCH] LAR checksums

[Carl-Daniel Hailfinger]

On 06.09.2007 03:23, Alex Beregszaszi wrote:
> Hi,
> 
> checksum-fix.diff fixes a checksum calculation bug in the lar utility,
> where it would checksum less bytes then desired.
> 
> lar-check-sum.diff implements checksum checking in the runtime lar code.
> 
> I implemented the check inside the filename check, thus it wont be
> executed on unneeded entries. As my assumption was that I cant modify
> the memory, the code is skipping bytes between 20-24 of the header
> (checksum field) to generate a correct sum.

Byte 20-24? Our checksum does not have 5 bytes. It seems both your
comment and the implementation suffer from a off-by-one error.

> This is bad, as the numbers shall be updated at every lar structure

Try offsetof() and sizeof() to fix that.

> change. This is good, as we are not writing to memory.
> 
> --
> Alex

Carl-Daniel

> ------------------------------------------------------------------------
> 
> Signed-off-by: Alex Beregszaszi <alex at rtfs.hu>
> 
> Index: util/lar/stream.c
> ===================================================================
> --- util/lar/stream.c	(revision 494)
> +++ util/lar/stream.c	(working copy)
> @@ -815,7 +825,7 @@
>  
>  	csum = 0;
>  	for (walk = (u32 *) (lar->map + offset);
> -	     walk < (u32 *) (lar->map + complen + hlen);
> +	     walk < (u32 *) (lar->map + offset + complen + hlen);
>  	     walk++) {
>  		csum += ntohl(*walk);
>  	}
> 
> 
> ------------------------------------------------------------------------
> 
> Signed-off-by: Alex Beregszaszi <alex at rtfs.hu>
> 
> Index: lib/lar.c
> ===================================================================
> --- lib/lar.c	(revision 494)
> +++ lib/lar.c	(working copy)
> @@ -99,9 +99,21 @@
>  		fullname = walk + sizeof(struct lar_header);
>  
>  		printk(BIOS_SPEW, "LAR: search for %s\n", fullname);
> -		// FIXME: check checksum
>  
>  		if (strcmp(fullname, filename) == 0) {
> +			u32 csum = 0, *p = (u32 *)walk;
> +
> +			/* validate checksum */
> +			for (; p < (u32 *)(walk+ntohl(header->len)+ntohl(header->offset)); p++)
> +			    /* skip the checksum field itself */
> +			    if (((char*)p-walk) < 20 || ((char*)p - walk) > 24)

see above.

> +				csum += ntohl(*p);
> +			if (csum != ntohl(header->checksum)) {
> +			    printk(BIOS_SPEW, "LAR: checksum failed on %s, skipping (%x != %x)\n",
> +				fullname, csum, ntohl(header->checksum));
> +			    return 1;
> +			}
> +
>  			printk(BIOS_SPEW, "LAR: CHECK %s @ %p\n", fullname, header);
>  			result->start = walk + ntohl(header->offset);
>  			result->len = ntohl(header->len);
> 


-- 
http://www.hailfinger.org/