Both fat and ext[234] boot sectors present at once cause confusion

[Andrew Clausen]

Binary package hint: grub2

grub-probe incorrectly identifies my ext3 partition as containing a fat file system. This means it can't boot without manual tweaking.

I suspect the problem is that my partition contains stale fat signatures... this is probably a common problem, as mke2fs often doesn't wipe old signatures.

I attached first 8k of my file system for testing purposes.

[Andrew Clausen (clausen)]

The bug also affects grub's bootloader (not just the bootloader installer).

If both fat and ext2 modules are loaded, then grub is unable to find anything on my root filesystem because it incorrectly identifies it as fat.

I'm working on a patch.

[Andrew Clausen (clausen)]

Here's the patch I promised. In order for a file system to be considered detected, dir() must not only succeed, it also must find at least one file or directory. This is pretty effective at ruling out misdetecting a filesystem based on a stale signature that wasn't wiped by mkfs. This shouldn't have any collateral damage, because empty file systems are useless to Grub.

The patch is against the latest beta from alpha.gnu.org, but this code looks stable, and should apply cleanly to any version. Can someone please forward this upstream? I can be contacted at <email address hidden> if this patch isn't clear.

--- grub-1.97~beta4/kern/fs.c 2009-07-16 18:14:09.000000000 -0400
+++ grub-1.97~beta4-fixed/kern/fs.c 2009-10-28 22:36:49.000000000 -0400
@@ -65,12 +65,14 @@
 grub_fs_probe (grub_device_t device)
 {
   grub_fs_t p;
+ int found_file;
   auto int dummy_func (const char *filename,
          const struct grub_dirhook_info *info);

   int dummy_func (const char *filename __attribute__ ((unused)),
     const struct grub_dirhook_info *info __attribute__ ((unused)))
     {
+ found_file = 1;
       return 1;
     }

@@ -82,8 +84,10 @@
       for (p = grub_fs_list; p; p = p->next)
  {
    grub_dprintf ("fs", "Detecting %s...\n", p->name);
+
+ found_file = 0;
    (p->dir) (device, "/", dummy_func);
- if (grub_errno == GRUB_ERR_NONE)
+ if (grub_errno == GRUB_ERR_NONE && found_file)
      return p;

    grub_error_push ();

[Felix Zielcke (fzielcke)]

Am Donnerstag, den 29.10.2009, 03:00 +0000 schrieb Andrew Clausen:
> Can someone
> please forward this upstream? I can be contacted at
> <email address hidden> if this patch isn't clear.

Is there any reason why you don't want to take this directly up with
upstream?
We really prefer to have patch writers submit them directly upstream,
because of FSF/GNU policy.

--
Felix Zielcke
Proud Debian Maintainer and GNU GRUB developer

[Andrew Clausen (clausen)]

It's a hassle subscribing to the members-only <email address hidden> list. But I guess I should do it, since everything else is a hassle too.

[Andrew Clausen (clausen)]

Jordan Uggla informed me that the tabs got mangled in my patch, and gave me an updated patch that should apply correctly. Sorry and thanks!

[Andrew Clausen (clausen)]

The previous patch introduced a typo. This one will work. Promise!

[Andrew Clausen (clausen)]

Grrr, I uploaded the wrong file! This is it!

[Andrew Clausen (clausen)]

I started a discussion about this patch on the mailing list, which you can view here:

http://lists.gnu.org/archive/html/grub-devel/2009-10/msg00450.html

Here's a short summary:
 * there was some code to address the most important aspects of the misdetection problem, but it was commented out (and is scheduled to be reincluded at some point).
 * we agree that the existing commented out code would solve most practical misdetection problems.
 * we agree that it is almost impossible to solve all practical misdetection problems.
 * we disagreed about my patch: the maintainers thought that my patch could introduce new problems, whereas I claim there is no possibility of introducing new problems. The maintainers declined to spell out their concerns.

What should be done?
 * at the moment, the misdetection problem is severe... if you reformat a FAT or NTFS partition as your root ext3/4 partition, you should expect your system to not boot with grub2! (My computer became unbootable, which is how I found the problem.)
 * it is possible that many people will be affected. Certainly, thousands of people were affected by such problems when I maintained (lib)parted (before we improved our detection heuristics.)
 * my patch solves the problem
 * the commented out code probably solves of the problem
 * I think Ubuntu should either apply one of the patches ASAP, or revert to a more stable version of Grub (with the commented out code included.)

[Felix Zielcke (fzielcke)]

Am Donnerstag, den 05.11.2009, 20:57 +0000 schrieb Andrew Clausen:
> * I think Ubuntu should either apply one of the patches ASAP, or
> revert to a more stable version of Grub (with the commented out code
> included.)

It doestn't make sense to just uncomment that code again (or reverting
in your terms).
It was commented out because else grub-probe segfaults if /boot is on a
seperate partition then /
This is even more serious then the case of a wrongly included filesystem
in core.img.
More people have a seperate /boot then multiple filesystem signatures on
one device.
Anyway that part of the code is enabled now in my grub-mkrelpath, which
hopefully soons get merged into our experimental branch.

--
Felix Zielcke
Proud Debian Maintainer and GNU GRUB developer

[Andrew Clausen (clausen)]

I agree that having /boot on a separate partition is an important problem... that leaves applying my patch (at least temporarily).

[Felix Zielcke (fzielcke)]

The lucid package has the grub-probe check again activated.
But that will make grub-install only abort if the wrong filesystem code is used.

[Phillip Susi (psusi)]

Why on earth does mkfs leave the fat boot sector intact? Seems like that is the problem that needs fixed.

[Phillip Susi (psusi)]

Adding e2fsprogs since I believe that mke2fs should be clearing the fat boot sector when you format.

[Theodore Ts'o (tytso)]

The reason why mke2fs doesn't zap sector 0 is because if someone is installing Linux on the whole disk (i.e., on /dev/sda instead of /dev/sda1), that sector is also the partition label and boot sector. And there isn't a good/portable way to determine whether a device is the whole disk or partition. And it would be a pretty horrible abstraction violation if mke2fs was trying to figure out whether there was something valuable in sector 0, whether it's a grub boot sector, or grub2 boot sector, or a EFI boot sector, or a FAT label....

[Phillip Susi (psusi)]

If you are going to format the whole disk, then you don't want any partition table that may already be there do you?. As for the boot code, well, you have to reinstall LILO after you format and put a new kernel on the disk anyhow. If it is a grub MBR, well, you trashed grub anyhow when you write to the following 32 sectors. If it is an EFI system, you corrupt the GPT, so who cares about the protective MBR?

[Theodore Ts'o (tytso)]

Ok, so I've taken a closer look at this.

mke2fs *does* zap the first 1k of the file system (by writing all zeros). The only system where it doesn't do this is sparc, because apparently that seriously screws up the bootloader. I don't remember the details, only that the sparc folks were screaming at me about it.

Looking at the blkid probes at least, it always checks various offsets in the first 1k of the file system, and so this is enough to prevent blkid from getting confused. I don't know what grub is doing, but I consider that a potential grub problem, and not mine; if blkid does the right thing, and other programs are getting confused, then maybe they should just use the blkid library. :-)

Anyway, I don't have more time to look at this, but as near as I can tell mke2fs is doing the right thing. If someone disagrees, they need to specifically give me a request about what mke2fs should do differently.

[Phillip Susi (psusi)]

It looks like mke2fs does indeed zap the fat boot sector, so grub correctly identifies it as ext2, at least as far back as lucid. Since this was originally reported in 2009, maybe older versions didn't do this? Seems we can mark this one as fixed.

[Theodore Ts'o (tytso)]

this bug was not reproducible in 2011 and could not be reproduced as far back as Lucid. Might as well close this since there has been no activity since then.