"Out of Partition" error on XFS still occuring

"Out of Partition" error on XFS still occuring

[Dan Callahan]

Hi, I seem to be able to recreate the xfs "out of partition" error
using Debian's most recent grub-pc release, 1.96+20080724-2.

This bug is being tracked at
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=436943 , and was
first reported one year ago. A patch from Niels Böhm last month was
thought to resolve it, but I'm still seeing the issue.

A dd copy of the offending filesystem is available at
http://neoflux.net/tmp/xfs-boot.tar.gz (8.9 MB compressed, 243 MB
uncompressed, link should remain active through August.)

Mounted as a loopback device in grub-emu, ls (device) detects that it
is xfs, but ls (device)/ and ls (device)/grub fail with the out of
partition error.

Thanks,
-Dan

Re: "Out of Partition" error on XFS still occuring

[Pavel Roskin]

On Sat, 2008-07-26 at 20:56 -0500, Dan Callahan wrote:
> Hi, I seem to be able to recreate the xfs "out of partition" error
> using Debian's most recent grub-pc release, 1.96+20080724-2.
> 
> This bug is being tracked at
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=436943 , and was
> first reported one year ago. A patch from Niels Böhm last month was
> thought to resolve it, but I'm still seeing the issue.
> 
> A dd copy of the offending filesystem is available at
> http://neoflux.net/tmp/xfs-boot.tar.gz (8.9 MB compressed, 243 MB
> uncompressed, link should remain active through August.)
> 
> Mounted as a loopback device in grub-emu, ls (device) detects that it
> is xfs, but ls (device)/ and ls (device)/grub fail with the out of
> partition error.

Valgrind on x86_64 indicates something interesting:

$ valgrind grub-fstest /home/proski/tmp/xfs/xfs-boot.img ls /                 
==23594== Memcheck, a memory error detector.
==23594== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==23594== Using LibVEX rev 1804, a library for dynamic binary translation.
==23594== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==23594== Using valgrind-3.3.0, a dynamic binary instrumentation framework.
==23594== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==23594== For more details, rerun with: -v
==23594== 
em.map-2.6.25-2-amd64/    ==23594== Conditional jump or move depends on uninitialised value(s)
==23594==    at 0x415FB9: call_hook.2389 (xfs.c:378)
==23594==    by 0x4167DB: grub_xfs_iterate_dir (xfs.c:469)
==23594==    by 0x41697C: grub_xfs_dir (xfs.c:659)
==23594==    by 0x4083EC: grub_cmd_ls (ls.c:176)
==23594==    by 0x4011C7: execute_command (grub-fstest.c:120)
==23594==    by 0x401452: fstest (grub-fstest.c:305)
==23594==    by 0x4016DF: main (grub-fstest.c:513)

"em.map-2.6.25-2-amd64/" is apparently the ls output.  Yet Linux can
mount the volume, and it shows reasonable names.

-- 
Regards,
Pavel Roskin

Re: "Out of Partition" error on XFS still occuring

[Bean]

[-- Attachment #1: Type: text/plain, Size: 2662 bytes --]

On Sun, Jul 27, 2008 at 3:47 PM, Pavel Roskin <proski@gnu.org> wrote:
> On Sat, 2008-07-26 at 20:56 -0500, Dan Callahan wrote:
>> Hi, I seem to be able to recreate the xfs "out of partition" error
>> using Debian's most recent grub-pc release, 1.96+20080724-2.
>>
>> This bug is being tracked at
>> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=436943 , and was
>> first reported one year ago. A patch from Niels Böhm last month was
>> thought to resolve it, but I'm still seeing the issue.
>>
>> A dd copy of the offending filesystem is available at
>> http://neoflux.net/tmp/xfs-boot.tar.gz (8.9 MB compressed, 243 MB
>> uncompressed, link should remain active through August.)
>>
>> Mounted as a loopback device in grub-emu, ls (device) detects that it
>> is xfs, but ls (device)/ and ls (device)/grub fail with the out of
>> partition error.
>
> Valgrind on x86_64 indicates something interesting:
>
> $ valgrind grub-fstest /home/proski/tmp/xfs/xfs-boot.img ls /
> ==23594== Memcheck, a memory error detector.
> ==23594== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
> ==23594== Using LibVEX rev 1804, a library for dynamic binary translation.
> ==23594== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
> ==23594== Using valgrind-3.3.0, a dynamic binary instrumentation framework.
> ==23594== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
> ==23594== For more details, rerun with: -v
> ==23594==
> em.map-2.6.25-2-amd64/    ==23594== Conditional jump or move depends on uninitialised value(s)
> ==23594==    at 0x415FB9: call_hook.2389 (xfs.c:378)
> ==23594==    by 0x4167DB: grub_xfs_iterate_dir (xfs.c:469)
> ==23594==    by 0x41697C: grub_xfs_dir (xfs.c:659)
> ==23594==    by 0x4083EC: grub_cmd_ls (ls.c:176)
> ==23594==    by 0x4011C7: execute_command (grub-fstest.c:120)
> ==23594==    by 0x401452: fstest (grub-fstest.c:305)
> ==23594==    by 0x4016DF: main (grub-fstest.c:513)
>
> "em.map-2.6.25-2-amd64/" is apparently the ls output.  Yet Linux can
> mount the volume, and it shows reasonable names.

Hi,

This problem is caused by the previous patch, it expands the size of
grub_xfs_dir_header:

struct grub_xfs_dir_header
{
  grub_uint8_t count;
  grub_uint8_t i8count;
  union
  {
    grub_uint32_t i4;
    grub_uint64_t i8;
  } parent __attribute__ ((packed));
} __attribute__ ((packed));

The size is always 10 bytes, but in fact, when small inode is used, it
should be 6 bytes. Then, in struct grub_xfs_inode, grub_xfs_dir_header
cause subsequence fields to move.

This patch revert the change, it should be ok now.

-- 
Bean

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xfs.diff --]
[-- Type: text/x-diff; name=xfs.diff, Size: 1327 bytes --]

diff --git a/fs/xfs.c b/fs/xfs.c
index 7da3e40..7b9d778 100644
--- a/fs/xfs.c
+++ b/fs/xfs.c
@@ -56,12 +56,8 @@ struct grub_xfs_sblock
 struct grub_xfs_dir_header
 {
   grub_uint8_t count;
-  grub_uint8_t i8count;
-  union
-  {
-    grub_uint32_t i4;
-    grub_uint64_t i8;
-  } parent __attribute__ ((packed));
+  grub_uint8_t smallno;
+  grub_uint32_t parent;
 } __attribute__ ((packed));
 
 struct grub_xfs_dir_entry
@@ -423,7 +419,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
     case XFS_INODE_FORMAT_INO:
       {
 	struct grub_xfs_dir_entry *de = &diro->inode.data.dir.direntry[0];
-	int smallino = !diro->inode.data.dir.dirhead.i8count;
+	int smallino = !diro->inode.data.dir.dirhead.smallno;
 	int i;
 	grub_uint64_t parent;
 
@@ -431,12 +427,12 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
 	   parent inode number is small too.  */
 	if (smallino)
 	  {
-	    parent = grub_be_to_cpu32 (diro->inode.data.dir.dirhead.parent.i4);
+	    parent = grub_be_to_cpu32 (diro->inode.data.dir.dirhead.parent);
 	    parent = grub_cpu_to_be64 (parent);
 	  }
 	else
 	  {
-	    parent = diro->inode.data.dir.dirhead.parent.i8;
+	    parent = *(grub_uint64_t *) &diro->inode.data.dir.dirhead.parent;
 	    /* The header is a bit bigger than usual.  */
 	    de = (struct grub_xfs_dir_entry *) ((char *) de + 4);
 	  }

Re: "Out of Partition" error on XFS still occuring

[Bean]

[-- Attachment #1: Type: text/plain, Size: 702 bytes --]

On Sun, Jul 27, 2008 at 6:06 PM, Bean <bean123ch@gmail.com> wrote:
> Hi,
>
> This problem is caused by the previous patch, it expands the size of
> grub_xfs_dir_header:
>
> struct grub_xfs_dir_header
> {
>  grub_uint8_t count;
>  grub_uint8_t i8count;
>  union
>  {
>    grub_uint32_t i4;
>    grub_uint64_t i8;
>  } parent __attribute__ ((packed));
> } __attribute__ ((packed));
>
> The size is always 10 bytes, but in fact, when small inode is used, it
> should be 6 bytes. Then, in struct grub_xfs_inode, grub_xfs_dir_header
> cause subsequence fields to move.
>
> This patch revert the change, it should be ok now.
>
> --
> Bean
>

Hi,

This new patch avoids the warning message of xfs.c

-- 
Bean

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xfs.diff --]
[-- Type: text/x-diff; name=xfs.diff, Size: 1536 bytes --]

diff --git a/fs/xfs.c b/fs/xfs.c
index 7da3e40..72d6a5f 100644
--- a/fs/xfs.c
+++ b/fs/xfs.c
@@ -56,7 +56,7 @@ struct grub_xfs_sblock
 struct grub_xfs_dir_header
 {
   grub_uint8_t count;
-  grub_uint8_t i8count;
+  grub_uint8_t smallno;
   union
   {
     grub_uint32_t i4;
@@ -423,7 +423,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
     case XFS_INODE_FORMAT_INO:
       {
 	struct grub_xfs_dir_entry *de = &diro->inode.data.dir.direntry[0];
-	int smallino = !diro->inode.data.dir.dirhead.i8count;
+	int smallino = !diro->inode.data.dir.dirhead.smallno;
 	int i;
 	grub_uint64_t parent;
 
@@ -433,12 +433,12 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
 	  {
 	    parent = grub_be_to_cpu32 (diro->inode.data.dir.dirhead.parent.i4);
 	    parent = grub_cpu_to_be64 (parent);
+            /* The header is a bit smaller than usual.  */
+	    de = (struct grub_xfs_dir_entry *) ((char *) de - 4);
 	  }
 	else
 	  {
-	    parent = diro->inode.data.dir.dirhead.parent.i8;
-	    /* The header is a bit bigger than usual.  */
-	    de = (struct grub_xfs_dir_entry *) ((char *) de + 4);
+	    parent = *(grub_uint64_t *) &diro->inode.data.dir.dirhead.parent.i8;
 	  }
 
 	/* Synthesize the direntries for `.' and `..'.  */
@@ -448,7 +448,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
 	if (call_hook (parent, ".."))
 	  return 1;
 
-	for (i = 0; i < diro->inode.data.dir.dirhead.count; i++)
+        for (i = 0; i < diro->inode.data.dir.dirhead.count; i++)
 	  {
 	    grub_uint64_t ino;
 	    void *inopos = (((char *) de)

Re: "Out of Partition" error on XFS still occuring

[Pavel Roskin]

On Sun, 2008-07-27 at 18:21 +0800, Bean wrote:

> This new patch avoids the warning message of xfs.c

Thanks.  Sorry for breaking it.

-- 
Regards,
Pavel Roskin