44 replies to this topic

[Wonko the Sane]

Maybe (just maybe) I found where the issue began.

 

The checksum generation is not in isolinux.asm, that contains the source code to compile isolinux.bin.

Isolinux.bin contains the code to check the checksum, but not the one to "self-write" it.

It is a post-modification of isolinux.bin, *something* modifies it after the isolinux.asm has been compiled to isolinux.bin.

 

It was not easy (provided that I actually found the "culprits") to find what (the heck) "populates" the "built-in" bootinfotable.

 

Seemingly:

 

In 3.86 the checksum is calculated and written by a checksumiso.pl. (which is readable - by me)

 

in 4.00 the checksum is (seemingly) calculated and written by a prepcore.c  (which is UNreadable - by me)

 

Given my ignorance of C and the lack of comments in the relevant part of prepcore.c (and provided I found the right file) I cannot say what actually is done there.

 

Wonko

[erwan.l]

A checksum function is usually quite simple.

Very simplistically, you go thru all your bytes and each compute a sum=sum+value.

 

Is your idea that since isolinux 4.x, the checksum is computed wrongly by prepcore.c and later on detected as wrong by isolinux.bin?

If so, this is quite a finding... and would explain why boot-table is NOT OPTIONAL since 4.x but actually MUST HAVE.

 

One note (and possibly a wrong track) but : my code uses all bytes (not only 2048) to generate the checksum.

And it does work i.E isolinux.bin is happy with it (and mkisofs generates the same code).

The code from prepcore.c uses the first 2048 bytes only.

I will update my code to see if it makes a difference.

EDIT : if i compute a checksum on 2048 bytes only (starting at position 64 like before) : different checksum and isolinux.bin reporting a wrong checksum.

 

Here below my delphi code.

type
  PSum32  = ^TSum32;
  TSum32   = LongWord;
  PSumA32 = ^TSumA32;
  TSumA32  = array[0..0] of TSum32;

function Sum32(const Data; const DataSize: LongWord; const BeginSum: TSum32 = 0): TSum32;
var ptr : PSumA32;
    N   : TSum32;
begin
  result := BeginSum;
  ptr := @Data;
  for N := 0 to (DataSize shr 2)-1 do
    result := TSum32((result + ptr^[N]));
end;

 

from prepcore.c

/* ISOLINUX padding and checksumming */
	uint32_t csum = 0;
	unsigned int ptr;
	outfile_len =
	    ((offset - start + out_len + 2047) & ~2047) - (offset - start);
	for (ptr = 64; ptr < offset; ptr += 4)
	    csum += get_32((uint32_t *) (infile + ptr));
	for (ptr = 0; ptr < outfile_len; ptr += 4)
	    csum += get_32((uint32_t *) (out + ptr));

[Wonko the Sane]

The checksum "by the book" is that of the whole isolinux.bin file minus the first 64 bytes.

 

This is how I calculated it (and verified it against the isolinux.bin 2.12-3.86) and veriified it agains "mkisofs massaged" isolinux.bin's and tested successfully in the 4.07 tests I made with the batch.

 

In a nutshell, there are two checksums:

1) is called in isolinux.asm "FirstSecSum" (and is the checksum of bytes 64-2048, and more or less is/should be a kind of verification of proper loading of the first sector, which is 2048 bytes and corresponds to the 4*512 bytes sectors that -boot-load-size 4 refers)

2) is called in isolinux.asm "bi_csum" (probably stands for binary image checksum) and is initially set, in isolinux.asm to 0xDEADBEEF (like the length of the file itself and the 10 following dwords while the LBA address of the binary image (in the .iso)  is set to 0x00000000),   

		; This table hopefully gets filled in by mkisofs using the
		; -boot-info-table option.  If not, the values in this
		; table are default values that we can use to get us what
		; we need, at least under a certain set of assumptions.
bi_pvd:		dd 16				; LBA of primary volume descriptor
bi_file:	dd 0				; LBA of boot file
bi_length:	dd 0xdeadbeef			; Length of boot file
bi_csum:	dd 0xdeadbeef			; Checksum of boot file
bi_reserved:	times 10 dd 0xdeadbeef		; Reserved

Now, after compilation of isolinux.asm to isolinux.bin, all these fields remain as above.

 

This would be OK, as long as mkisofs (or however a program that fixes the bootinfotable) is used with the -boot-info-table switch that:

1) writes in bi_file the LBA address (in the .iso)

2) writes in bi_length the actual length of the boot file

3) writes in bi_csum the actual checksum from offset 64 to the end of the boot file

4) for *whatever* reasons writes all 00's to bi_reserved

 

The news in Isolinux 2.13 is that the good Peter Anvin introduced in the code a smart way to attempt to boot even if the user forgot the -boot-info-table switch.

 

So *something* at build time post processes the just assembled isolinux.bin as follows:

1) leaves in bi_file the address 0x00000000 (which value is later used to check whether -boot-info-table has been used or not, as that value won't ever be 0 if -boot-info-table is used)

2) writes in bi_length the actual length of the boot file (after having padded it with 00's to the next multiple of the CD sector, i.e. 2048 bytes)

3) writes in bi_csum the actual checksum from offset 64 to the end of the boot file just padded

4) for *whatever* reasons leaves the bi_reserved to 0XDEADBEEF

 

In checksumiso.pl the process is very clear.

 

Since version 4.00 the checksumiso.pl is no more and as said seemingly the same chores are done by the prepcore.c.

 

But it is not like re-posting it you make me understand the (stupid) C syntax, I can stare at those lines all day long, but I will never be able to actually understand what (the heck) they do, I take for good that you can understand that code and that it actually (wrongly) computes only the range 64-2048, though I am not convinced at all that the checksum there is calculated for bytes 64-2048, simply because if I recalculate it it doesn't match (while if I recalculate the right range checksum it matches, meaning that my half-@§§ed and blatantly copied/pasted/adapted algorithm in Tiny Hexer works just fine).

 

In any case it is to be confirmed that it is actually that prepcore.c file the culprit and that the code in it is wrong (there could be another zillion things in the build procedure that changes those fields).

 

 

 

may be I should split this thread to a isolinux one?

 

Yep, why not?

 

Wonko

[Wonko the Sane]

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.

 

@Ady

I guess we are now ready to report to "mainstream".

Could you do that ?

 

Wonko

[erwan.l]

So, making sure I understand correctly : issue would come from the checksum code in prepcode.c introduced in version 4.x starting at offset 0 instead of offiset 64?

 

Hence explaining why the checksum in is systematically wrong since then and hence explaining that the only way to boot it is to re-compute the checksum (starting at offset 64) ?

 

Quite a finding.

If MakeIso was useful at least for one thing only, then I am glad it led to that discussion

[Wonko the Sane]

So, making sure I understand correctly : issue would come from the checksum code in prepcode.c introduced in version 4.x starting at offset 0 instead of offiset 64?

 

Hence explaining why the checksum in is systematically wrong since then and hence explaining that the only way to boot it is to re-compute the checksum (starting at offset 64) ?

 

Quite a finding.

If MakeIso was useful at least for one thing only, then I am glad it led to that discussion

Yep , that's it.

 

The code, the one that you just stated calculates the checksum of first 2048 bytes only :

 

 

 

 

The code from prepcore.c uses the first 2048 bytes only.

 

 

actually computes the checksum of the WHOLE file AND it does so at a time when both bi_length and bi_csum are 0xDEADBEEF.
 
Clearly a small glitch in the matrix. 
 
Since the whole stuff is essentially a "failback provision", it would only come into play if the user making the .iso did not use the -boot-info-table switch (i.e. ignoring the recommendations on the command line to use with mkisofs).

 What probably happened all these years (i.e. since release of Syslinux 4.00, 28/06/2010) is the following:

1) a final user attempted using the Isolinux.bin with mkisofs WITHOUT specifying the -boot-info-table parameter

2) the resulting .iso won't boot

3) when asking for assistance, was told to use the -boot-info-table, as per recommendation of the Author

4) the new .iso with -boot-info-table worked, problem solved

5) wrong conclusion: -boot-info-table is needed

 

It is an exceptionally good example of how a (very minor in this case) bug can survive in a rarely (almost never) used code "branch" after years of use of the software by - I believe - millions of users.

 

Wonko

[ady]

The report is already "in transit" to upstream, and there is even one possible (untested) potential suggested patch (which might or might not have additional consequences?).

IMHO, one way of improving the chances of seeing some real progress would be to have a dedicated topic within the relevant forum's area ([reboot.pro > Boot methods & tools > Boot from USB / Boot anywhere > The Syslinux Project]). I don't think that splitting this "MakeIso" topic is really needed at this time, but just having a minimal "intro" or basic explanation of the issue, perhaps with a link to this topic, for reference.

Then, the next step would be to actively invite forum members that have been developers/contributors for The Syslinux Project at some point (e.g. Sha0 , Icecube) to participate / help / provide background/historical info / propose patches.

[Wonko the Sane]

The report is already "in transit" to upstream, and there is even one possible (untested) potential suggested patch (which might or might not have additional consequences?).

Good .

 

As a matter of fact, no actual "patch" is needed.

The "patch" consists IMHO in a correction to the code in next *whatever* release of Syslinux.

 

What would be useful (and I leave that to the programmers) would be a "patcher", a small program (that should be developed for both Linux and Windows) that takes *any* isolinux.bin and corrects the checksum.

 

This can already be done manually by *anyone*:

1) putting the whatever isolinux.bin in a directory by itself

2) create an "expendable" .iso with mkisofs with the -boot-info-table switch

3) once the mkisofs will have thus corrected the checksum (and overwitten with the same value the file size bi_length) and written the LBA offset bi_file, simply hex-edit the bi_file setting it to 0x00000000

4) totally optionally, as they never come into play, re-write the 10*0xDEADBEEF values (that mkisofs will have re-written as 0x00000000)

 

A dedicated "patcher" would do the same without the need to create the directory ad the .iso. 

 

I don't think that it would be historically and philosophically correct to go back in the archives and patch/correct all the already released versions , but it could be an option 

 

 

 

IMHO, one way of improving the chances of seeing some real progress would be to have a dedicated topic within the relevant forum's area ([reboot.pro > Boot methods & tools > Boot from USB / Boot anywhere > The Syslinux Project]). I don't think that splitting this "MakeIso" topic is really needed at this time, but just having a minimal "intro" or basic explanation of the issue, perhaps with a link to this topic, for reference.

Then, the next step would be to actively invite forum members that have been developers/contributors for The Syslinux Project at some point (e.g. Sha0 , Icecube) to participate / help / provide background/historical info / propose patches.

 

I don't know.

 

I believe that the Author (H. Peter Anvin) should be made aware of the issue, and he should be the one that decides how (and if) to correct it.

 

 With all due respect to the way some Open Source projects are "managed by the crowd" I think that when the Author (or Maintainer) is active he should just be made aware and he is the one that - if needed - should ask for help, patches, etc.

 

Wonko

[erwan.l]

I could write a patcher, for both platforms.
Lets see how it go with Ady's report.

[erwan.l]

a quick boot table iso pacther.

windows only.

linux version will come this week end.

 

should work for isolinux but also grub2.

 

source code included.

can be compiled with delphi or freepascal.

same code used in MakeIso.

patcher isolinux.bin
Boot table ISO patchere 0.1 by erwan2212@gmail.com
usage : patcher my.iso
checksump updated: 0x9E9EBA3C

Attached Files

•  patcher.zip   48.87KB   7 downloads

[Wonko the Sane]

I have no idea if GRUB2 actually *needs* a boot info table (I mean whether it works or not without one).

 

Grub4dos surely needs not one, it works just fine without one (and with -boot-load-size 4), the (tiny) issue at the time was only about the opportunity of including one (actually what would have been strictly needed was only of course bi_length field) to make easier the extraction of the boot loader from an existing .iso created with -boot-load-size 4 (or however, in the context of Isobuster and similar software have an accurate alternate method to calculate its size).

 

Personally I would actually "restrict" by default (and maybe actually add an explicit override) to files named isolinux.bin and isolinux-debug.bin or - maybe even better - now that we know how the "wrong" checksum is calculated, re-calculate the "wrong" one, check that it is there and only patch if the "wrong" checksum is present.

Or - easier - check for the presence of the 10*0xDEADBEEF and only attempt to patch in that case.

 

And - besides the google ones - you might be interested in "creative naming" lessons.

 

"patcher" ?

After MakeIso and Iso_info?

 

Come on , what will be your next program name, "program"?

 

Not that they are in anyway creative or particularly smart but - say BTIpatcher or iSLpatcher sound already better...

 

Wonko

[erwan.l]

About grub2 I can confirm it needs the same patching.

 

About naming this is one of my weak points

I am off on business trip all week but will release a new version this week end. 

[ady]

The "patch" consists IMHO in a correction to the code in next *whatever* release of Syslinux.

Yes, sorry, apparently I wasn't clear enough. I was indeed referring to patching the source code for future Syslinux versions (not isolinux.bin itself, nor ISO images already in existence).
 

I believe that the Author (H. Peter Anvin) should be made aware of the issue, and he should be the one that decides how (and if) to correct it.

The Syslinux Project is "too big" for just one person. Asking for help from (current/past/new) contributors/developers for a patch that improves the relevant source code and solves this issue for future versions of Syslinux is not such a crazy idea, IMHO.
 

With all due respect to the way some Open Source projects are "managed by the crowd" I think that when the Author (or Maintainer) is active he should just be made aware and he is the one that - if needed - should ask for help, patches, etc.

If I had to wait for just/only one specific person to be active in The Syslinux Project before I do anything to help with Syslinux... Well, I'd rather not even think about that. I'd rather focus on getting things done, somehow.

Sha0 ? Icecube? Others?

[Wonko the Sane]

Only for the record/to keep everything as together as possible, relevant threads on the Syslinux mailing list:

http://www.syslinux....read.html#26064

http://www.syslinux....read.html#26077

 

Quick sum up, although the issue has been confirmed and a patch has been proposed/issued by Thomas Schmitt:

http://www.syslinux....ary/026079.html

the source has seemingly been not patched at the moment: 

http://repo.or.cz/sy.../lzo/prepcore.c

 

 

Wonko

[ady]

the source has seemingly been not patched at the moment

Sadly, that's correct, and that is also the reason I haven't posted an update here yet.

As I reported in the Syslinux Mailing List, binaries generated with the proposed patch (applied on the current git master head) have been tested with successful results.

FWIW, and for convenience, the patch:
 

--- a/lzo/prepcore.c
+++ b/lzo/prepcore.c
@@ -331,7 +331,7 @@ int main(int argc, char *argv[])
        unsigned int ptr;
        outfile_len =
            ((offset - start + out_len + 2047) & ~2047) - (offset - start);
-       for (ptr = 64; ptr < offset; ptr += 4)
+       for (ptr = start + 64; ptr < offset; ptr += 4)
            csum += get_32((uint32_t *) (infile + ptr));
        for (ptr = 0; ptr < outfile_len; ptr += 4)
            csum += get_32((uint32_t *) (out + ptr));

is valid for (and can be cleanly applied to ) Syslinux versions 4.xx and up to 6.04-pre1 included.
As soon as the official Syslinux code gets the patch, I'll post an update here too.

[erwan.l]

It can be a bit frustrative indeed

But on the other hand, it seems thet issue has been there for a while and now it is on a good way to get resolved sooner or later (or later ... ).

Worth saying (or repeating) : @Ady, thanks for carrying the request on syslinux ML.

[ady]

A proposed patch was sent to the Syslinux Mailing List... And 9 month later, a commit was born!

 https://repo.or.cz/s...63db071ba8ef7b0
 

https://repo.or.cz/syslinux.git/commitdiff/ac39cffd82d9f3ea4f243657c63db071ba8ef7b0 

The "lzo/prepcore.c" patch (authored by Thomas Schmitt) should apply (almost) cleanly to any version of Syslinux since 4.00 up to today's git master head.

Thanks to all that were involved.

[erwan.l]

[Wonko the Sane]

A proposed patch was sent to the Syslinux Mailing List... And 9 month later, a commit was born!

Good news .

 

@erwan.l

Besides that, JFYI in Italian there is a saying:

Tutto fa (disse quello che sputava in mare)

 

 

which translates roughly to:

Everything does (said the one who was spitting in the sea)

 

 

Wonko

[erwan.l]

@erwan.l

Besides that, JFYI in Italian there is a saying:

Quote

Tutto fa (disse quello che sputava in mare)

 

 

 

On a good day, I can be Yoda ... but on most days (and despite being 45...) I am either Luke Skywalker (whining) or worse, Anakin (angry)