> Hi,
>
> i think i found a suspect in lzo/prepcore.c and it would indeed be a
> wrong range of checksumming (speculative congratulations to Ady).
Thank you Thomas for your replies and for looking into this issue.
My part on the initial investigation that triggered this email thread
is relatively small. Others deserve much more credit. I was/am
providing not just my own report, but their's too.
This is a quote from Wonko/Jaclaz, who has also been investigation this
issue and deserves credit for it:
[quote]
Ok, not that I understand the code, but what *somehow* happens is that
the "embedded" checksum in Isolinux.bin (starting from 4.00) is the
checksum of the WHOLE file (i.e. starting from offset 0 instead of 64)
at a time when:
1) the dword at offset 0x10 (bi_length) is still 0xDEAFBEEF
2) the dword at offset 0x14 (bi_csum) is still 0xDEADBEEF
Whether the file length is already changed or not at the time the
checksum is calculated doesn't matter (since the file is being padded
with 00's, and this doesn't change the checksum), I tried resetting the
two 32 bit values to 0xDEADBEEF and re-calculate the 32 wide sum of the
whole file on a couple of versions and the result matched the (wrong)
embedded checksum.
[/quote]
Credits also go to Erwan.L.
> Looking at
>
http://repo.or.cz/syslinux.git/blob/0d82b71304d596d80f3c4520f9dcf90048ca50b7:/lzo/prepcore.c
> it seems that this change in line 374 could yield correct checksums:
>
> unsigned int ptr;
> - for (ptr = 64; ptr < offset; ptr += 4)
> + for (ptr = start+64; ptr < offset; ptr += 4)
> csum += get_32((uint32_t *)(infile+ptr));
>
>
> A test whether it works would be to produce isolinux.bin, read the 4
checksum
> bytes from it at offset 20, put isolinux.bin into an ISO with mkisofs
option
> -boot-info-table, and check whether the 4 bytes at offset 20 are still the
> same.
> (If you use mkisofs or genisoimage, isolinux.bin on hard disk will change.
> With xorrisofs the change will only appear in the file isolinux.bin in the
> ISO filesystem.)
Unfortunately, for various reasons I cannot build Syslinux myself these
days/months. If someone builds isolinux.bin + ldlinux.c32 with Thomas'
patch (for prepcore.c), I am very willing to help with several tests.
>
> --------------------------------------------------------------------------
> Reasoning:
>
> - The prescription for Boot Info Table says that checksumming begins
> at byte 64 of isolinux.bin. (See for example man mkisofs paragraph
> "EL TORITO BOOT INFORMATION TABLE".)
>
> - prepcore.c constructs isolinux.bin from an unencrypted piece out of
> the input file isolinux.raw (which stems from an objcopy -S run) and
> from a compressed part.
> So it begins to compute the checksum on the input file bytes
>
> for (ptr = 64; ptr < offset; ptr += 4)
> csum += get_32((uint32_t *)(infile+ptr));
>
> and then continues on the compressed part
>
> for (ptr = 0; ptr < outfile_len; ptr += 4)
> csum += get_32((uint32_t *)(out+ptr));
>
> - But prepcore.c copies infile data to isolinux.bin starting with an
> offset named "start" which i strongly believe is not zero:
>
> if (fwrite(infile+start,1,offset-start,f) != offset-start ||
>
> If "start" is not zero, then this is not the same byte range as
with
> the checksum loop over (infile+ptr).
>
> - "start" obviously is used to skip at least the 32 byte header
which
> prepcore.c interprets at the beginning of the input file:
>
> struct prefix {
> uint32_t pfx_start;
> uint32_t pfx_compressed;
> uint32_t pfx_cdatalen;
> uint32_t pfx_checksum;
> };
> ...
> struct prefix *prefix;
> ...
> infile_len = (lzo_uint) fread(infile,1,infile_len,f);
> ...
> prefix = (struct prefix *)infile;
> start = get_32(&prefix->pfx_start);
> offset = get_32(&prefix->pfx_compressed);
>
> Such header bytes with plausible values are not to see at the beginning
of
> isolinux.bin:
>
> fa ea 6c 7c 00 00 90 90 10 00 00 00 00 00 00 00
>
> The struct members pfx_start and pfx_compressed are read as little endian
> unsigned integers and used as offsets in the isolinux.raw file.
> 0x7c6ceafa and 0x90900000 are much too large to have served for that
> purpose.
> So the original prefix header was skipped, and thus "start" was
not zero.
>
>
> Have a nice day :)
>
> Thomas
>
Since I am not a developer, I cannot avoid wondering whether patching
prepcore.c would have additional consequences.
Considering that LZO was brought into Syslinux by Peter, and he is also
the author of the commit that replaced checksumiso.pl, I really wish
Peter would show up and comment on this matter.
Hpa?
Any Syslinux developer (past or present)?
TIA,
Ady.