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 <[EMAIL PROTECTED]>
>
> 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 <[EMAIL PROTECTED]>
>
> 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/
--
linuxbios mailing list
linuxbios@linuxbios.org
http://www.linuxbios.org/mailman/listinfo/linuxbios
