Xen devel - Oct 2011 - [PATCH] Add HybridISO support for PyGrub2

grub-mkrescue internally uses xorriso, which generates a
so-called "hybrid ISO": The ISO images also contains a DOS partition
table,
which allows the identical ISO file to be stored on an USB stick for booting
from it. This breaks PyGrub, since it (wrongly) detects the DOS partition 
table and uses the first partition instead of the complete ISO file.

Add a check to detech HybridISO files and fall back to unpartitioned 
operation.

Signed-off-by: Philipp Hahn <hahn@univention.de>
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -41,12 +41,16 @@ def enable_cursor(ison):
         pass
 
 def is_disk_image(file):
+    """Detect DOS partition table."""
     fd = os.open(file, os.O_RDONLY)
-    buf = os.read(fd, 512)
+    buf = os.read(fd, 0x8006)
     os.close(fd)
 
     if len(buf) >= 512 and \
            struct.unpack("H", buf[0x1fe: 0x200]) == (0xaa55,):
+        # HybridISO contains a DOS partition table for booting from USB 
devices, but really is an ISO image
+        if len(buf) >= 0x8006 and buf[0x8001:0x8006] ==
''CD001'':
+            return False
         return True
     return False
 
-- 
Philipp Hahn           Open Source Software Engineer      hahn@univention.de
Univention GmbH        Linux for Your Business        fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
                                                   http://www.univention.de/


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, 2011-10-25 at 09:56 +0100, Philipp Hahn wrote:
> grub-mkrescue internally uses xorriso, which generates a
> so-called "hybrid ISO": The ISO images also contains a DOS
partition table,
> which allows the identical ISO file to be stored on an USB stick for
booting
> from it. This breaks PyGrub, since it (wrongly) detects the DOS partition 
> table and uses the first partition instead of the complete ISO file.
Is the problem here that, having detected a DOS partition, pygrub is
then unwilling to accept that the partition contains an ISO9660 file
system? Or is it that the DOS partition table covers something other
than the ISO9660 data? Where does the kernel we want to boot actually
live?

Is there some reference to the layout of an hybridiso? i.e. what is the
significance on 0x8006? What are the chances of false positives?
> Add a check to detech HybridISO files and fall back to unpartitioned 
> operation.
> 
> Signed-off-by: Philipp Hahn <hahn@univention.de>
> --- a/tools/pygrub/src/pygrub
> +++ b/tools/pygrub/src/pygrub
> @@ -41,12 +41,16 @@ def enable_cursor(ison):
>          pass
>  
>  def is_disk_image(file):
> +    """Detect DOS partition table."""
>      fd = os.open(file, os.O_RDONLY)
> -    buf = os.read(fd, 512)
> +    buf = os.read(fd, 0x8006)
>      os.close(fd)
>  
>      if len(buf) >= 512 and \
>             struct.unpack("H", buf[0x1fe: 0x200]) == (0xaa55,):
> +        # HybridISO contains a DOS partition table for booting from USB 
> devices, but really is an ISO image
> +        if len(buf) >= 0x8006 and buf[0x8001:0x8006] ==
''CD001'':
> +            return False
>          return True
>      return False
>  


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

At 10:17 +0100 on 25 Oct (1319537858), Ian Campbell
wrote:
> On Tue, 2011-10-25 at 09:56 +0100, Philipp Hahn wrote:
> > grub-mkrescue internally uses xorriso, which generates a
> > so-called "hybrid ISO": The ISO images also contains a DOS
partition table,
> > which allows the identical ISO file to be stored on an USB stick for
booting
> > from it. This breaks PyGrub, since it (wrongly) detects the DOS
partition
> > table and uses the first partition instead of the complete ISO file.
> 
> Is the problem here that, having detected a DOS partition, pygrub is
> then unwilling to accept that the partition contains an ISO9660 file
> system?
The problem is that since the disk has a valid MBR partition table,
pygrub tries all the partitions in that but _doesn''t_ try the whole
device (as it would if it couldn''t find an MBR).
> Or is it that the DOS partition table covers something other
> than the ISO9660 data? Where does the kernel we want to boot actually
> live?
> 
> Is there some reference to the layout of an hybridiso? i.e. what is the
> significance on 0x8006? What are the chances of false positives?
Looking for "\001CD001" at offset 0x8000 is a pretty good way to
identify an ISO9660 image (e.g. I think that''s how magic(5) does it).
This patch doesn''t _quite_ do that, and in any case this is the wrong
place to do it.  The right fix to have get_partition_offsets() detect
ISO9660 images and add ''[0]'' to whatever else it comes up
with.

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hello Ian,

Am Dienstag 25 Oktober 2011 11:17:38 schrieb Ian
Campbell:
> On Tue, 2011-10-25 at 09:56 +0100, Philipp Hahn wrote:
> > grub-mkrescue internally uses xorriso, which generates a
> > so-called "hybrid ISO": The ISO images also contains a DOS
partition
> > table, which allows the identical ISO file to be stored on an USB
stick
> > for booting from it. This breaks PyGrub, since it (wrongly) detects
the
> > DOS partition table and uses the first partition instead of the
complete
> > ISO file.
>
> Is the problem here that, having detected a DOS partition, pygrub is
> then unwilling to accept that the partition contains an ISO9660 file
> system?
PyGrub chcks for the DOS partition table by reading the last two bytes of the 
first 512 byte sector. Since the HybridISO has a valid partition table, the 
first partition starting at offset 512 is used instead of the full image 
starting at offset 0.
> Or is it that the DOS partition table covers something other 
> than the ISO9660 data?
As far as I understood it, the partition covers the same area of the iso to 
protect it from accidently trashing the data on the stick. But since it has 
different sector sizes (512 vs 2048) and offsets (512 vs. 0), the data 
actually looks like garbage (at least "file" detects nothing)
> Where does the kernel we want to boot actually 
> live?
In the ISO starting at offset 0.
> Is there some reference to the layout of an hybridiso?
You might want to take a look at syslinux from hpa; his web-page still seems 
to be unaccessable.
<http://forum.avira.com/wbb/index.php?page=Thread&threadID=99493> has
a short
summary.
> i.e. what is the  significance on 0x8006?
It''s what "file" does to detect ISO-Images. At offset 0x8000
the ISO 9660
Primary Volume Descriptor starts. From offset [2..6[ is the "id",
which most
often is "CD001". See 
<http://users.telenet.be/it3.consultants.bvba/handouts/ISO9960.html> for a
complete spec.
> What are the chances of false positives? 
Existing, but small: 1:2^40.
As fas as I know many Linux distributions use the HybridISO trick. I''ve
read
from Debian, Ubuntu, Arch Linux,Gentoo. But they don''t seem to use
PyGrub
with Xen for PV installations ;-)

Sincerely
Philipp
-- 
Philipp Hahn           Open Source Software Engineer      hahn@univention.de
Univention GmbH        Linux for Your Business        fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
                                                   http://www.univention.de/


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, 2011-10-25 at 11:33 +0100, Philipp Hahn wrote:
[...snip explanations...]

Thanks Philipp, that all seems to make sense.

Tim had some suggestions on how/where this functionality could be better
implemented though.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hello Ian,

On Thursday 27 October 2011 12:21:05 Ian Campbell wrote:
> On Tue, 2011-10-25 at 11:33 +0100, Philipp Hahn wrote:
> [...snip explanations...]
>
> Thanks Philipp, that all seems to make sense.
>
> Tim had some suggestions on how/where this functionality could be better
> implemented though.
v2: For HybrisISOs use offset 0 in addition instead of replacement.

Signed-off-by: Philipp Hahn <hahn@univention.de>
---
grub-mkrescue internally uses xorriso, which generates a so-called "Hybrid 
ISO": The ISO images also contains a DOS partition table, which allows the 
identical ISO file to be stored on an USB stick for booting from it. This 
breaks PyGrub, since it (wrongly) detects only the DOS partition table and 
uses the first partition instead of the complete ISO file.

Add a check to detect HybridISO files and use offset 0 in addition to 
partition table parsing.
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -40,15 +40,20 @@ def enable_cursor(ison):
     except _curses.error:
         pass
 
-def is_disk_image(file):
+DISK_TYPE_RAW, DISK_TYPE_HYBRIDISO, DISK_TYPE_DOS = range(3)
+def identify_disk_image(file):
+    """Detect DOS partition table or HybridISO
format."""
     fd = os.open(file, os.O_RDONLY)
-    buf = os.read(fd, 512)
+    buf = os.read(fd, 0x8006)
     os.close(fd)
 
     if len(buf) >= 512 and \
            struct.unpack("H", buf[0x1fe: 0x200]) == (0xaa55,):
-        return True
-    return False
+        # HybridISO contains a DOS partition table for booting from USB 
devices, but really is an ISO image
+        if len(buf) >= 0x8006 and buf[0x8001:0x8006] ==
''CD001'':
+            return DISK_TYPE_HYBRIDISO
+        return DISK_TYPE_DOS
+    return DISK_TYPE_RAW
 
 SECTOR_SIZE=512
 DK_LABEL_LOC=1
@@ -87,12 +92,19 @@ FDISK_PART_SOLARIS_OLD=0x82
 FDISK_PART_GPT=0xee
 
 def get_partition_offsets(file):
-    if not is_disk_image(file):
+    image_type = identify_disk_image(file)
+    if image_type == DISK_TYPE_RAW:
         # No MBR: assume whole disk filesystem, which is like a 
         # single partition starting at 0
         return [0]
-
-    part_offs = []
+    elif image_type == DISK_TYPE_HYBRIDISO:
+        # A HybridISO contains an ISO filesystem at 0 in addition
+        # to the DOS partition table
+        part_offs = [0]
+    elif image_type == DISK_TYPE_DOS:
+        part_offs = []
+    else:
+        raise ValueError(''Unhandled image type returnd by 
identify_disk_image(): %d'' % (image_type,))
 
     fd = os.open(file, os.O_RDONLY)
     buf = os.read(fd, 512)

Sincerely
Philipp
-- 
Philipp Hahn           Open Source Software Engineer      hahn@univention.de
Univention GmbH        Linux for Your Business        fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
                                                   http://www.univention.de/


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 2011-10-28 at 08:46 +0100, Philipp Hahn wrote:
> Hello Ian,
> 
> On Thursday 27 October 2011 12:21:05 Ian Campbell wrote:
> > On Tue, 2011-10-25 at 11:33 +0100, Philipp Hahn wrote:
> > [...snip explanations...]
> >
> > Thanks Philipp, that all seems to make sense.
> >
> > Tim had some suggestions on how/where this functionality could be
better
> > implemented though.
> 
> v2: For HybrisISOs use offset 0 in addition instead of replacement.
> 
> Signed-off-by: Philipp Hahn <hahn@univention.de>
> ---
> grub-mkrescue internally uses xorriso, which generates a so-called
"Hybrid
> ISO": The ISO images also contains a DOS partition table, which allows
the
> identical ISO file to be stored on an USB stick for booting from it. This 
> breaks PyGrub, since it (wrongly) detects only the DOS partition table and 
> uses the first partition instead of the complete ISO file.
> 
> Add a check to detect HybridISO files and use offset 0 in addition to 
> partition table parsing.
> --- a/tools/pygrub/src/pygrub
> +++ b/tools/pygrub/src/pygrub
> @@ -40,15 +40,20 @@ def enable_cursor(ison):
>      except _curses.error:
>          pass
>  
> -def is_disk_image(file):
> +DISK_TYPE_RAW, DISK_TYPE_HYBRIDISO, DISK_TYPE_DOS = range(3)
> +def identify_disk_image(file):
> +    """Detect DOS partition table or HybridISO
format."""
>      fd = os.open(file, os.O_RDONLY)
> -    buf = os.read(fd, 512)
> +    buf = os.read(fd, 0x8006)
Can we avoid reading all that just for the 7 bytes we are interested in.
What about (just pseudo python, I didn''t actually lookup seek etc):

	buf = os.read(fd, 512)
	if len(buf) >= 512 and \
             not struct.unpack("H", buf[0x1fe: 0x200]) == (0xaa55,):
		return ...RAW

	os.seek(0x8000)
	buf = os.read(fd, 6)
	if len... and buf[0x8001...] == ''CD001'':
		return ...HYBRID
	else:
		return ...DOS

on the other hand I suppose it''s only 8k...

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks.
Ian
.
>      os.close(fd)
>  
>      if len(buf) >= 512 and \
>             struct.unpack("H", buf[0x1fe: 0x200]) == (0xaa55,):
> -        return True
> -    return False
> +        # HybridISO contains a DOS partition table for booting from USB 
> devices, but really is an ISO image
> +        if len(buf) >= 0x8006 and buf[0x8001:0x8006] ==
''CD001'':
> +            return DISK_TYPE_HYBRIDISO
> +        return DISK_TYPE_DOS
> +    return DISK_TYPE_RAW
>  
>  SECTOR_SIZE=512
>  DK_LABEL_LOC=1
> @@ -87,12 +92,19 @@ FDISK_PART_SOLARIS_OLD=0x82
>  FDISK_PART_GPT=0xee
>  
>  def get_partition_offsets(file):
> -    if not is_disk_image(file):
> +    image_type = identify_disk_image(file)
> +    if image_type == DISK_TYPE_RAW:
>          # No MBR: assume whole disk filesystem, which is like a 
>          # single partition starting at 0
>          return [0]
> -
> -    part_offs = []
> +    elif image_type == DISK_TYPE_HYBRIDISO:
> +        # A HybridISO contains an ISO filesystem at 0 in addition
> +        # to the DOS partition table
> +        part_offs = [0]
> +    elif image_type == DISK_TYPE_DOS:
> +        part_offs = []
> +    else:
> +        raise ValueError(''Unhandled image type returnd by 
> identify_disk_image(): %d'' % (image_type,))
>  
>      fd = os.open(file, os.O_RDONLY)
>      buf = os.read(fd, 512)
> 
> Sincerely
> Philipp


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Philipp Hahn writes ("Re: [Xen-devel] [PATCH v2] Add HybridISO support for
PyGrub2"):
> v2: For HybrisISOs use offset 0 in addition instead of replacement.
Thanks.  I tried to apply this but it has been linewrapped.  Can you
please resubmit it with a non-broken mailer ?  If you avoid lines >75
characters it probably won''t linewrap, or you could attach it I guess.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hello Ian,

Am Dienstag 01 November 2011 19:53:15 schrieb Ian
Jackson:
> Philipp Hahn writes ("Re: [Xen-devel] [PATCH v2] Add HybridISO support
for
PyGrub2"):
> > v2: For HybrisISOs use offset 0 in addition instead of replacement.
>
> Thanks.  I tried to apply this but it has been linewrapped.  Can you
> please resubmit it with a non-broken mailer ?  If you avoid lines >75
> characters it probably won''t linewrap, or you could attach it I
guess.
I''ve attached the patch. Thank you for asking back.

Sincerely
Philipp
-- 
Philipp Hahn           Open Source Software Engineer      hahn@univention.de
Univention GmbH        Linux for Your Business        fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
                                                   http://www.univention.de/



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Philipp Hahn writes ("Re: [Xen-devel] [PATCH v2] Add HybridISO support for
PyGrub2"):
> Am Dienstag 01 November 2011 19:53:15 schrieb Ian Jackson:
> > Thanks.  I tried to apply this but it has been linewrapped.  Can you
> > please resubmit it with a non-broken mailer ?  If you avoid lines
>75
> > characters it probably won''t linewrap, or you could attach it
I guess.
> 
> I''ve attached the patch. Thank you for asking back.
Great, thanks, applied.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel