Bug#502333: Bug #502333 ext3 external journal_dev crashes grub2

2009-02-05 Thread Harald Braumann
On Wed, 04 Feb 2009 08:38:15 +0100
Felix Zielcke  wrote:

> Hello,
> 
> can you please try out if the attached patch solves your problem?
> You can find instructions for compiling your own grub packages at the
> Debian wiki: 
> http://wiki.debian.org/de/GRUB2/Anleitung/DebianPaketVonOriginalQuelltext
> Just place the patch in debian/patches.
> 

Yeah, that did the trick. Everything works fine with that patch
applied. I'm glad, my bug report can no longer be responsible
for delaying Lenny ;)

BTW, I also tried 1.96+20080724-14 but that didn't work. I noticed,
that while the `set root` immediately before the menu entries is gone
with -14, there is still a `set root (vgsys-lvroot)` at the very
top stanza. Is that intended? Anyway, I also removed this set root
command, and grub still crashed.

Cheers,
harry


signature.asc
Description: PGP signature


Bug#502333: Bug #502333 ext3 external journal_dev crashes grub2

2009-02-03 Thread Felix Zielcke
Hello,

can you please try out if the attached patch solves your problem?
You can find instructions for compiling your own grub packages at the
Debian wiki: 
http://wiki.debian.org/de/GRUB2/Anleitung/DebianPaketVonOriginalQuelltext
Just place the patch in debian/patches.

-- 
Felix Zielcke
Index: fs/ext2.c
===
--- fs/ext2.c	(revisiĆ³n: 1799)
+++ fs/ext2.c	(copia de trabajo)
@@ -71,8 +71,53 @@
  ? EXT2_GOOD_OLD_INODE_SIZE \
  : grub_le_to_cpu16 (data->sblock.inode_size))
 
-#define EXT3_FEATURE_COMPAT_HAS_JOURNAL	0x0004
+/* Superblock filesystem feature flags (RW compatible)
+ * A filesystem with any of these enabled can be read and written by a driver
+ * that does not understand them without causing metadata/data corruption */
+#define EXT2_FEATURE_COMPAT_DIR_PREALLOC	0x0001
+#define EXT2_FEATURE_COMPAT_IMAGIC_INODES	0x0002
+#define EXT3_FEATURE_COMPAT_HAS_JOURNAL		0x0004
+#define EXT2_FEATURE_COMPAT_EXT_ATTR		0x0008
+#define EXT2_FEATURE_COMPAT_RESIZE_INODE	0x0010
+#define EXT2_FEATURE_COMPAT_DIR_INDEX		0x0020
+/* Superblock filesystem feature flags (RO compatible)
+ * A filesystem with any of these enabled can be safely read by a driver that
+ * does not understand them, but should not be written to, usually because
+ * additional metadata is required */
+#define EXT2_FEATURE_RO_COMPAT_SPARSE_SUPER	0x0001
+#define EXT2_FEATURE_RO_COMPAT_LARGE_FILE	0x0002
+#define EXT2_FEATURE_RO_COMPAT_BTREE_DIR	0x0004
+#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM		0x0010
+#define EXT4_FEATURE_RO_COMPAT_DIR_NLINK	0x0020
+#define EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE	0x0040
+/* Superblock filesystem feature flags (back-incompatible)
+ * A filesystem with any of these enabled should not be attempted to be read
+ * by a driver that does not understand them, since they usually indicate
+ * metadata format changes that might confuse the reader. */
+#define EXT2_FEATURE_INCOMPAT_COMPRESSION	0x0001
+#define EXT2_FEATURE_INCOMPAT_FILETYPE		0x0002
+#define EXT3_FEATURE_INCOMPAT_RECOVER		0x0004 /* Needs recovery */
+#define EXT3_FEATURE_INCOMPAT_JOURNAL_DEV	0x0008 /* Volume is journal device */
+#define EXT2_FEATURE_INCOMPAT_META_BG		0x0010
+#define EXT4_FEATURE_INCOMPAT_EXTENTS		0x0040 /* Extents used */
+#define EXT4_FEATURE_INCOMPAT_64BIT		0x0080
+#define EXT4_FEATURE_INCOMPAT_FLEX_BG		0x0200
 
+/* The set of back-incompatible features this driver DOES support. Add (OR)
+ * flags here as the related features are implemented into the driver */
+#define EXT2_DRIVER_SUPPORTED_INCOMPAT ( EXT2_FEATURE_INCOMPAT_FILETYPE \
+   | EXT4_FEATURE_INCOMPAT_EXTENTS )
+/* List of rationales for the ignored "incompatible" features:
+ * needs_recovery: Not really back-incompatible - was added as such to forbid
+ * ext2 drivers from mounting an ext3 volume with a dirty
+ * journal because they will ignore the journal, but the next
+ * ext3 driver to mount the volume will find the journal and
+ * replay it, potentially corrupting the metadata written by
+ * the ext2 drivers
+ */
+#define EXT2_DRIVER_IGNORED_INCOMPAT ( EXT3_FEATURE_INCOMPAT_RECOVER )
+
+
 #define EXT3_JOURNAL_MAGIC_NUMBER	0xc03b3998U
 
 #define EXT3_JOURNAL_DESCRIPTOR_BLOCK	1
@@ -486,10 +531,12 @@
   return 0;
 }
 
+#define EXT2_DRIVER_MOUNT_FAIL(message) { local_error = (message); goto fail; }
 static struct grub_ext2_data *
 grub_ext2_mount (grub_disk_t disk)
 {
   struct grub_ext2_data *data;
+  const char *local_error = 0;
 
   data = grub_malloc (sizeof (struct grub_ext2_data));
   if (!data)
@@ -498,13 +545,19 @@
   /* Read the superblock.  */
   grub_disk_read (disk, 1 * 2, 0, sizeof (struct grub_ext2_sblock),
   (char *) &data->sblock);
-  if (grub_errno)
-goto fail;
+  if (grub_errno != GRUB_ERR_NONE)
+EXT2_DRIVER_MOUNT_FAIL(0);
 
   /* Make sure this is an ext2 filesystem.  */
   if (grub_le_to_cpu16 (data->sblock.magic) != EXT2_MAGIC)
-goto fail;
+EXT2_DRIVER_MOUNT_FAIL("not an ext2 filesystem");
   
+  /* Check the FS doesn't have feature bits enabled that we don't support. */
+  if (grub_le_to_cpu32 (data->sblock.feature_incompat)
+& ~(EXT2_DRIVER_SUPPORTED_INCOMPAT | EXT2_DRIVER_IGNORED_INCOMPAT))
+EXT2_DRIVER_MOUNT_FAIL("filesystem has unsupported incompatible features");
+
+  
   data->disk = disk;
 
   data->diropen.data = data;
@@ -514,13 +567,15 @@
   data->inode = &data->diropen.inode;
 
   grub_ext2_read_inode (data, 2, data->inode);
-  if (grub_errno)
-goto fail;
+  if (grub_errno != GRUB_ERR_NONE)
+EXT2_DRIVER_MOUNT_FAIL(0);
   
   return data;
 
  fail:
-  grub_error (GRUB_ERR_BAD_FS, "not an ext2 filesystem");
+  /* Only call grub_error if the fail was _not_ caused by underlying errors.  */
+  if (local_error)
+grub_error (GRUB_ERR_BAD_FS, local_error);
   grub_free (data);
   return 0;
 }