boot.img Fix

boot.img Fix

[Yves BLUSSEAU]

[-- Attachment #1.1: Type: text/plain, Size: 1069 bytes --]

Hi,

there is a "bug" in boot.img: if you install the boot.img into the 
volume boot sector of a FAT32 partition instead of MBR (i know it's a 
bad idea), you "destroyed" the partition (even grub will not recognize it).
The problem is that the FAT32 FileSystem need a 87 bytes long BPB 
instead of the 59 bytes long for FAT16 
(http://en.wikipedia.org/wiki/File_Allocation_Table#Boot_Sector).
So the boot.img need to preverse the first 90 bytes of the first sectors 
of a FAT32 partition.
I made this patch to fix it and it work perfectly.

What i have done is:
    * change the offset of the end of BPB
    * change the offsets of the data inside the region of Extended BPB
    * remove 2 constants that are never used
    * change the minor version of the GRUB_BOOT_VERSION 4.0 => 4.1
    * change the string of hd_probe_error_string from "Hard Drive" to "HD"
       because i needed 2 extras bytes and do not want to change the working
       code of the boot.

Now the bootloader (boot.img) can be install on any FAT volume partitions.

Best Regards,
Yves Blusseau

[-- Attachment #1.2: boot_img_fat32.diff --]
[-- Type: text/plain, Size: 3013 bytes --]

commit 1890e0781d4e210c6076568a81899ed598319249
Author: Yves Blusseau <cl7m42e02@sneakemail.com>
Date:   Tue Jul 7 10:04:33 2009 +0200

    Extend the BPB for compatibility with FAT32 volume boot partition

diff --git a/ChangeLog b/ChangeLog
index ed6b637..d52de45 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2009-07-07  Yves BLUSSEAU  <cl7m42e02@sneakemail.com>
+
+	* include/grub/boot.h (GRUB_BOOT_VERSION_MINOR):
+	change the GRUB_BOOT_VERSION from 4.0 to 4.1
+
+	* boot/i386/pc/boot.S: Change hd_probe_error_string from
+	"Hard Drive" to "HD" to gain 2 extra bytes
+
+	* include/grub/i386/pc/boot.h: Change the offsets to extend
+	the BPB for compatibility with FAT32 volume boot partitions
+
 2009-07-06  Pavel Roskin  <proski@gnu.org>
 
 	* commands/search.c (search_fs): Fix potential NULL pointer
diff --git a/boot/i386/pc/boot.S b/boot/i386/pc/boot.S
index 8d8c27c..d43aec6 100644
--- a/boot/i386/pc/boot.S
+++ b/boot/i386/pc/boot.S
@@ -434,11 +434,11 @@ general_error:
         int	$0x18
 stop:	jmp	stop
 
-notification_string:	.asciz "GRUB "
-geometry_error_string:	.asciz "Geom"
-hd_probe_error_string:	.asciz "Hard Disk"
-read_error_string:	.asciz "Read"
-general_error_string:	.asciz " Error"
+notification_string:   .asciz "GRUB "
+geometry_error_string: .asciz "Geom"
+hd_probe_error_string: .asciz "HD"
+read_error_string:     .asciz "Read"
+general_error_string:  .asciz " Error"
 
 /*
  * message: write the string pointed to by %si
diff --git a/include/grub/boot.h b/include/grub/boot.h
index 2357748..43cd3a5 100644
--- a/include/grub/boot.h
+++ b/include/grub/boot.h
@@ -20,7 +20,7 @@
 #define GRUB_BOOT_HEADER	1
 
 #define GRUB_BOOT_VERSION_MAJOR	4
-#define GRUB_BOOT_VERSION_MINOR	0
+#define GRUB_BOOT_VERSION_MINOR	1
 #define GRUB_BOOT_VERSION	((GRUB_BOOT_VERSION_MINOR << 8) \
 					| GRUB_BOOT_VERSION_MAJOR)
 
diff --git a/include/grub/i386/pc/boot.h b/include/grub/i386/pc/boot.h
index f35cb3a..57f53ca 100644
--- a/include/grub/i386/pc/boot.h
+++ b/include/grub/i386/pc/boot.h
@@ -26,25 +26,19 @@
 #define GRUB_BOOT_MACHINE_BPB_START	0x3
 
 /* The offset of the end of BPB (BIOS Parameter Block).  */
-#define GRUB_BOOT_MACHINE_BPB_END	0x3e
+#define GRUB_BOOT_MACHINE_BPB_END	0x5a
 
 /* The offset of the major version.  */
-#define GRUB_BOOT_MACHINE_VER_MAJ	0x3e
+#define GRUB_BOOT_MACHINE_VER_MAJ	0x5a
 
 /* The offset of BOOT_DRIVE.  */
-#define GRUB_BOOT_MACHINE_BOOT_DRIVE	0x4c
-
-/* The offset of KERNEL_ADDRESS.  */
-#define GRUB_BOOT_MACHINE_KERNEL_ADDRESS	0x40
+#define GRUB_BOOT_MACHINE_BOOT_DRIVE	0x68
 
 /* The offset of KERNEL_SECTOR.  */
-#define GRUB_BOOT_MACHINE_KERNEL_SECTOR	0x44
-
-/* The offset of KERNEL_SEGMENT.  */
-#define GRUB_BOOT_MACHINE_KERNEL_SEGMENT	0x42
+#define GRUB_BOOT_MACHINE_KERNEL_SECTOR	0x60
 
 /* The offset of BOOT_DRIVE_CHECK.  */
-#define GRUB_BOOT_MACHINE_DRIVE_CHECK	0x4e
+#define GRUB_BOOT_MACHINE_DRIVE_CHECK	0x6a
 
 /* The offset of a magic number used by Windows NT.  */
 #define GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC	0x1b8

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/x-pkcs7-signature, Size: 3313 bytes --]

Re: boot.img Fix

[Pavel Roskin]

On Tue, 2009-07-07 at 10:08 +0200, Yves BLUSSEAU wrote:
> Hi,
> 
> there is a "bug" in boot.img: if you install the boot.img into the 
> volume boot sector of a FAT32 partition instead of MBR (i know it's a 
> bad idea), you "destroyed" the partition (even grub will not recognize it).

I think a much bigger issue is that installing on FAT32 it was allowed
at all!  I assume that even with your patch, installing the GRUB
bootsector into some filesystems would still break them.  We need a
check that the bootsector is compatible with the filesystem.

> The problem is that the FAT32 FileSystem need a 87 bytes long BPB 
> instead of the 59 bytes long for FAT16 
> (http://en.wikipedia.org/wiki/File_Allocation_Table#Boot_Sector).
> So the boot.img need to preverse the first 90 bytes of the first sectors 
> of a FAT32 partition.
> I made this patch to fix it and it work perfectly.
> 
> What i have done is:
>     * change the offset of the end of BPB
>     * change the offsets of the data inside the region of Extended BPB
>     * remove 2 constants that are never used

Removing unused constants is better done in a separate patch.  And they
should be specified by name.

>     * change the minor version of the GRUB_BOOT_VERSION 4.0 => 4.1

I don't see where it's checked.  What's the effect of the change?

>     * change the string of hd_probe_error_string from "Hard Drive" to "HD"
>        because i needed 2 extras bytes and do not want to change the working
>        code of the boot.

It means the code is very tight.  We would not able to add any
workarounds or features in the future.

When is the ability to install on FAT32 important?  If it's just a way
to fix a missing check in grub-setup, then I'll rather have some extra
space.

The same boot.S has a hole for the partition table.  I think we should
need either one hole or another.  That would allow us to have two
versions of boot.img, one for FAT bootsector and another for MBR.

> -notification_string:	.asciz "GRUB "

It's better not to mix formatting and essential changes.  Besides, the
use of tabs is justified for alignment.

-- 
Regards,
Pavel Roskin

Re: boot.img Fix

[Pavel Roskin]

Quoting Pavel Roskin <proski@gnu.org>:

>>     * change the minor version of the GRUB_BOOT_VERSION 4.0 => 4.1
>
> I don't see where it's checked.  What's the effect of the change?

P.S. I think eventually grub-setup should learn to use ELF images.   
Then the boot version could be a discardable constant, and the  
locations of all data could be found using symbols.  There should be  
no need to keep the boot version in the installed image.

-- 
Regards,
Pavel Roskin

Re: boot.img Fix

[Yves BLUSSEAU]

[-- Attachment #1: Type: text/html, Size: 3296 bytes --]

Re: boot.img Fix

[Vladimir 'phcoder' Serbinenko]

Sorry for posting some not very-related ideas in one mail and
top-posting but I want to go to bed ASAP now. FAT install is
important. A friend asked me on RMLL to install GRUB on his
partionless SD card. This card was formatted with Symbian and
contained no "FAT32" string. I actually see no reason to keep the
check for this string. Originally it was added to ignore some recovery
partitions from the list. But there is no reason to do so even if they
normally contain no useful data. It's user's freedom to read even
useless data in GRUB.
Current grub-setup may have a bug to accidently detect FAT* as
pc_partition_map (they share signature) and try to embed with
potentially bad results
For FAT booting I would prefer to have 3 separate sectors able to
parse FAT32/FAT16/FAT12. I would prefer it to be able to go into
subdirectories but in worst case grub-install can copy core.img to the
root. Or we can use so-called "reserved sectors" on FAT to either
store second part of parser or embed core. For the latter the obstacle
is that it's typically only around 32 reserved sectors.

On Wed, Jul 8, 2009 at 12:17 AM, Pavel Roskin<proski@gnu.org> wrote:
> On Tue, 2009-07-07 at 10:08 +0200, Yves BLUSSEAU wrote:
>> Hi,
>>
>> there is a "bug" in boot.img: if you install the boot.img into the
>> volume boot sector of a FAT32 partition instead of MBR (i know it's a
>> bad idea), you "destroyed" the partition (even grub will not recognize it).
>
> I think a much bigger issue is that installing on FAT32 it was allowed
> at all!  I assume that even with your patch, installing the GRUB
> bootsector into some filesystems would still break them.  We need a
> check that the bootsector is compatible with the filesystem.
>
>> The problem is that the FAT32 FileSystem need a 87 bytes long BPB
>> instead of the 59 bytes long for FAT16
>> (http://en.wikipedia.org/wiki/File_Allocation_Table#Boot_Sector).
>> So the boot.img need to preverse the first 90 bytes of the first sectors
>> of a FAT32 partition.
>> I made this patch to fix it and it work perfectly.
>>
>> What i have done is:
>>     * change the offset of the end of BPB
>>     * change the offsets of the data inside the region of Extended BPB
>>     * remove 2 constants that are never used
>
> Removing unused constants is better done in a separate patch.  And they
> should be specified by name.
>
>>     * change the minor version of the GRUB_BOOT_VERSION 4.0 => 4.1
>
> I don't see where it's checked.  What's the effect of the change?
>
>>     * change the string of hd_probe_error_string from "Hard Drive" to "HD"
>>        because i needed 2 extras bytes and do not want to change the working
>>        code of the boot.
>
> It means the code is very tight.  We would not able to add any
> workarounds or features in the future.
>
> When is the ability to install on FAT32 important?  If it's just a way
> to fix a missing check in grub-setup, then I'll rather have some extra
> space.
>
> The same boot.S has a hole for the partition table.  I think we should
> need either one hole or another.  That would allow us to have two
> versions of boot.img, one for FAT bootsector and another for MBR.
>
>> -notification_string: .asciz "GRUB "
>
> It's better not to mix formatting and essential changes.  Besides, the
> use of tabs is justified for alignment.
>
> --
> Regards,
> Pavel Roskin
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

Re: boot.img Fix

[Robert Millan]

On Thu, Jul 09, 2009 at 01:09:20AM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Sorry for posting some not very-related ideas in one mail and
> top-posting but I want to go to bed ASAP now. FAT install is
> important. A friend asked me on RMLL to install GRUB on his
> partionless SD card. This card was formatted with Symbian and
> contained no "FAT32" string. I actually see no reason to keep the
> check for this string. Originally it was added to ignore some recovery
> partitions from the list. But there is no reason to do so even if they
> normally contain no useful data. It's user's freedom to read even
> useless data in GRUB.
> Current grub-setup may have a bug to accidently detect FAT* as
> pc_partition_map (they share signature) and try to embed with
> potentially bad results
> For FAT booting I would prefer to have 3 separate sectors able to
> parse FAT32/FAT16/FAT12. I would prefer it to be able to go into
> subdirectories but in worst case grub-install can copy core.img to the
> root. Or we can use so-called "reserved sectors" on FAT to either
> store second part of parser or embed core. For the latter the obstacle
> is that it's typically only around 32 reserved sectors.

I agree GRUB should be very careful not to destroy potentially valuable
data.  However, that's not to say FAT install is important.  Typically
our install doesn't collide or interfere with any filesystem, it's only
a few corner cases that do, and IMO we should try to discourage those.

But of course, destroying someone's filesystem is not the right way to
discourage something :-)

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: boot.img Fix

[Pavel Roskin]

Quoting Robert Millan <rmh@aybabtu.com>:

> I agree GRUB should be very careful not to destroy potentially valuable
> data.  However, that's not to say FAT install is important.  Typically
> our install doesn't collide or interfere with any filesystem, it's only
> a few corner cases that do, and IMO we should try to discourage those.
>
> But of course, destroying someone's filesystem is not the right way to
> discourage something :-)

I think installing to the first (or any) sector of a filesystem not  
known specifically to allow that should at least require --force.

I'm fine with supporting fat32, I just want to use the opportunity to  
fix some related issues.

-- 
Regards,
Pavel Roskin

Re: boot.img Fix

[Robert Millan]

On Sun, Jul 12, 2009 at 09:30:01AM -0400, Pavel Roskin wrote:
> Quoting Robert Millan <rmh@aybabtu.com>:
>
>> I agree GRUB should be very careful not to destroy potentially valuable
>> data.  However, that's not to say FAT install is important.  Typically
>> our install doesn't collide or interfere with any filesystem, it's only
>> a few corner cases that do, and IMO we should try to discourage those.
>>
>> But of course, destroying someone's filesystem is not the right way to
>> discourage something :-)
>
> I think installing to the first (or any) sector of a filesystem not  
> known specifically to allow that should at least require --force.

Agreed.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."