Syslinux - Apr 2017 - Patch: Fix isohybrid.c tool under Windows platforms

Hi all,

As part of a bigger effort to port the ISOLINUX hybrid mode to ReactOS,
I've encountered a bug in isohybrid under Windows platforms. Attached
comes a patch to fix it.

Precisely, if isohybrid currently finds a 0x0A (LF) byte in the MBR code
(which exists in the strings), it adds a 0x0D (CR) before, thereby
shifting the MBR bytes to the right and making it invalid.
This can be fixed by opening the file in binary mode with the "b"
flag.

Cheers,

Colin
-------------- next part --------------

Hi,

Colin Finck wrote:
> Precisely, if isohybrid currently finds a 0x0A (LF) byte in the MBR code
> (which exists in the strings), it adds a 0x0D (CR) before, thereby
> shifting the MBR bytes to the right and making it invalid.
> This can be fixed by opening the file in binary mode with the "b"
flag.
This is a very reasonable proposal.
If one wants the same behavior of fread(3) and fwrite(3) on descendants of
Unix and DOS, then one should in any case add "b" to the fopen(3) mode
string.

Not only MBR writing but also reading of the Primary Volume Descriptor
and of the El Torito catalog by isohybrid.c are currently prone to
unwanted CR-insertion. Miscalculations can be caused by this.

The fopen() call for reading external MBR code (option -m) already has
"b".

The old perl code contains a call to function binmode, which does about
the same as C fopen(3) with "b" in the mode string.


Have a nice day :)

Thomas

> Hi,
> 
> Colin Finck wrote:
> > Precisely, if isohybrid currently finds a 0x0A (LF) byte in the MBR
code
> > (which exists in the strings), it adds a 0x0D (CR) before, thereby
> > shifting the MBR bytes to the right and making it invalid.
> > This can be fixed by opening the file in binary mode with the
"b" flag.
> 
> This is a very reasonable proposal.
> If one wants the same behavior of fread(3) and fwrite(3) on descendants of
> Unix and DOS, then one should in any case add "b" to the fopen(3)
mode
> string.
> 
> Not only MBR writing but also reading of the Primary Volume Descriptor
> and of the El Torito catalog by isohybrid.c are currently prone to
> unwanted CR-insertion. Miscalculations can be caused by this.
 
 
Is there anything that can be done in order to reduce such chances of 
miscalculation and/or unwanted insertion of (CR) characters?

> 
> The fopen() call for reading external MBR code (option -m) already has
"b".
> 
> The old perl code contains a call to function binmode, which does about
> the same as C fopen(3) with "b" in the mode string.

So, is there any reason not to apply the suggested patch? Any potential 
compatibility issues? Backward compatibility between different versions 
of isohybrid and isolinux.bin? Anything?

> 
> 
> Have a nice day :)
> 
> Thomas
> 
Regards,
Ady.

On Mon, Apr 10, 2017 at 6:21 AM, Colin Finck via Syslinux
<syslinux at zytor.com> wrote:
> Hi all,
>
> As part of a bigger effort to port the ISOLINUX hybrid mode to ReactOS,
> I've encountered a bug in isohybrid under Windows platforms. Attached
> comes a patch to fix it.
>
> Precisely, if isohybrid currently finds a 0x0A (LF) byte in the MBR code
> (which exists in the strings), it adds a 0x0D (CR) before, thereby
> shifting the MBR bytes to the right and making it invalid.
> This can be fixed by opening the file in binary mode with the "b"
flag.
Thanks for catching this.  It has been committed.

-- 
-Gene