Syslinux - May 2014 - [PATCH] isohybrid: fix overflow on 32 bit system

When call isohybrid with option '-u', it overflows on a 32 bits host. It
seeks to 512 bytes before the end of the image to install gpt header. If
the size of image is larger than LONG_MAX, it overflows fseek() and
cause error:

isohybrid: wrlinux-image-x86-64-20140505110100.iso: seek error - 8: Invalid
argument

Check the offset and call fseek() multi-times if offset is too large.

Signed-off-by: Kai Kang <kai.kang at windriver.com>
---
 utils/isohybrid.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/utils/isohybrid.c b/utils/isohybrid.c
index 410bb60..4047287 100644
--- a/utils/isohybrid.c
+++ b/utils/isohybrid.c
@@ -34,6 +34,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <limits.h>
 #include <sys/stat.h>
 #include <inttypes.h>
 #include <uuid/uuid.h>
@@ -888,7 +889,7 @@ main(int argc, char *argv[])
     FILE *fp = NULL;
     uint8_t *buf = NULL, *bufz = NULL;
     int cylsize = 0, frac = 0;
-    size_t orig_gpt_size, free_space, gpt_size;
+    size_t orig_gpt_size, free_space, gpt_size, real_offset;
     struct iso_primary_descriptor descriptor;
 
     prog = strcpy(alloca(strlen(argv[0]) + 1), argv[0]);
@@ -1125,9 +1126,16 @@ main(int argc, char *argv[])
 	 * Seek far enough back that the gpt header is 512 bytes before the
 	 * end of the image
 	 */
+	real_offset = (isostat.st_size + padding) - orig_gpt_size - 512;
 
-	if (fseek(fp, (isostat.st_size + padding) - orig_gpt_size - 512,
-		  SEEK_SET))
+	fseek(fp, 0, SEEK_SET);
+	while (real_offset > LONG_MAX) {
+		if (fseek(fp, LONG_MAX, SEEK_CUR))
+			err(1, "%s: seek error - 8", argv[0]);
+		real_offset -= LONG_MAX;
+	}
+
+	if (fseek(fp, real_offset, SEEK_CUR))
 	    err(1, "%s: seek error - 8", argv[0]);
 
 	if (fwrite(buf, sizeof(char), orig_gpt_size, fp) != orig_gpt_size)
-- 
1.9.1

On 05/11/2014 06:56 PM, Kai Kang wrote:
> When call isohybrid with option '-u', it overflows on a 32 bits
host. It
> seeks to 512 bytes before the end of the image to install gpt header. If
> the size of image is larger than LONG_MAX, it overflows fseek() and
> cause error:
> 
> isohybrid: wrlinux-image-x86-64-20140505110100.iso: seek error - 8: Invalid
argument
> 
> Check the offset and call fseek() multi-times if offset is too large.
> 
> Signed-off-by: Kai Kang <kai.kang at windriver.com>
NAK.

The right thing to do is compile it with #define _FILE_OFFSET_BITS 64
and change fseek to fseeko with the appropriate type being off_t.

	-hpa

Hi,

hpa:
> The right thing to do is compile it with #define _FILE_OFFSET_BITS 64
> and change fseek to fseeko with the appropriate type being off_t.
And for GPT names it should really use 16-bit UTF-16LE characters
and not 8-bit characters padded up by the same number of
random bytes from the program memory.
See e.g.:
 
http://git.kernel.org/cgit/boot/syslinux/syslinux.git/tree/utils/isohybrid.c#n795
This problem and more are reported in
  http://www.syslinux.org/archives/2012-May/017843.html
  http://www.syslinux.org/archives/2012-May/017871.html

Fedora image (by isohybrid.c):
  GPT partition name :   1 
49534f4879627269642049534f0049534f487962726964004170706c
Archlinux (by libisofs):
  GPT partition name :   1  490053004f00480079006200720069006400
  GPT partname local :   1  ISOHybrid
(Lines from the upcomming boot equipment inspector of libisofs. See
  http://libburnia-project.org/wiki/ReportSystemArea
)


To Kai Kang:

I am willing to help with isohybrid.c but do not want to
become its maintainer. I have my own implementation of isohybrid
functionality in libisofs, and deem it better if both stay
code-wise independent.


Have a nice day :)

Thomas

On 2014?05?12? 10:49, H. Peter Anvin wrote:
> On 05/11/2014 06:56 PM, Kai Kang wrote:
>> When call isohybrid with option '-u', it overflows on a 32 bits
host. It
>> seeks to 512 bytes before the end of the image to install gpt header.
If
>> the size of image is larger than LONG_MAX, it overflows fseek() and
>> cause error:
>>
>> isohybrid: wrlinux-image-x86-64-20140505110100.iso: seek error - 8:
Invalid argument
>>
>> Check the offset and call fseek() multi-times if offset is too large.
>>
>> Signed-off-by: Kai Kang <kai.kang at windriver.com>
> NAK.
>
> The right thing to do is compile it with #define _FILE_OFFSET_BITS 64
> and change fseek to fseeko with the appropriate type being off_t.
>
> 	-hpa
Hi hpa,

Thanks for your reply.

In utils/Makefile, -D_FILE_OFFSET_BITS=64 is added to CFLAGS. isohybrid 
is compiled with -D_FILE_OFFSET_BITS=64 but it still fails to handle the 
large offset.

make[3]: Entering directory 
`/home/neil/wrlinux/builds/install-x86-64/bitbake_build/tmp/work/i686-linux/syslinux-native/6.01-r0/syslinux-6.01/bios/utils'
gcc 
-isystem/home/neil/wrlinux/builds/install-x86-64/bitbake_build/tmp/sysroots/i686-linux/usr/include
-O2 -pipe -Wp,-MT,isohybrid.o,-MMD,./.isohybrid.o.d -W -Wall 
-Wstrict-prototypes -Os -fomit-frame-pointer *-D_FILE_OFFSET_BITS=64* 
-I/home/neil/wrlinux/builds/install-x86-64/bitbake_build/tmp/work/i686-linux/syslinux-native/6.01-r0/syslinux-6.01/utils
-c -o isohybrid.o 
/home/neil/wrlinux/builds/install-x86-64/bitbake_build/tmp/work/i686-linux/syslinux-native/6.01-r0/syslinux-6.01/utils/isohybrid.c

I build it on RedHat Enterprise Linux 6.2 with glibc 2.12.

Regards,
Kai
>

-- 
Regards,
Neil | Kai Kang

Hi,

Kang Kai wrote:
> Do you mean make_isohybrid_mbr.c in libisofs? I'll check it.
Yes. The "new" implementation, make_isolinux_mbr() et.al.,
is what gets triggered by xorrisofs option -isohybrid-mbr.

But First:
I do not want to pull away users from isohybrid.c.
I rather deem it worthwhile to have two implementations which
users can both try in case of trouble.

Second:
My source will only be of limited help because it is not derived
from isohybrid.c but rather based on the specs which i derived
from isohybrid.pl:
 
http://bazaar.launchpad.net/~libburnia-team/libisofs/scdbackup/view/head:/doc/boot_sectors.txt
I advise you to look there if you want to learn about my doings.

------------------------------------------------------------------

But the known bugs of isohybrid.c should get fixed.

The UTF-16 thing is a clear C programming error, regardless of
my interpretation of GPT specs.
You do not need iconv(3) to produce UTF-16LE. Just interleave
ASCII with 0-bytes:
  char name[28] =  {'I', 0, 'S', 0, 'O', 0, 'H',
0, 'y', 0, ...};
  ...
  memcpy(part->name, name, 28);
  ...
(This works of course only with ASCII names, not with exotic
 names expressed in UTF-8 or ISO-8859.) 


Have a nice day :)

Thomas