Re: [PATCH] fs/xfs: Avoid unreadble filesystem if V4 superblock

[Erwan Velu]

I should have cc Carlos Maiolino who authored this patch.

This is now done.

Le mer. 25 août 2021 à 15:32, Erwan Velu <erwanaliasr1@gmail.com> a écrit :

Commit 8b1e5d1936fffc490510e85c95f93248453586c1 introduced the support
of bigtime by adding the some features in inodes V3.

This change extended grub_xfs_inode struct by 76 bytes but also changed the
computation of XFS_V3_INODE_SIZE & XFS_V2_INODE_SIZE.

Prior this commit, XFS_V2_INODE_SIZE was 100 bytes, after the commit it's 84 bytes.
XFS_V2_INODE_SIZE becomes 16 bytes too small.

As a result, the data structure aren't properly aligned and generates
"attempt to read or write outside of partition" errors when trying to
read the filesystem.

                             GNU GRUB  version 2.11
        ....
        grub> set debug=efi,gpt,xfs
        grub> insmod part_gpt
        grub> ls (hd0,gpt1)/
        partmap/gpt.c:93: Read a valid GPT header
        partmap/gpt.c:115: GPT entry 0: start=4096, length=1953125
        fs/xfs.c:931: Reading sb
        fs/xfs.c:270: Validating superblock
        fs/xfs.c:295: XFS v4 superblock detected
        fs/xfs.c:962: Reading root ino 128
        fs/xfs.c:515: Reading inode (128) - 64, 0
        fs/xfs.c:515: Reading inode (739521961424144223) - 344365866970255880, 3840
        error: attempt to read or write outside of partition.

This commit change the XFS_V2_INODE_SIZE computation by substracting 76
instead of 92 from the actual size of grub_xfs_inode.
This 76 value is coming from the added :
        20 uint8   unused5
         1 uint64  flags2
        48 uint8   unused6

This patch explicit the split between the v2 and v3 parts of structure.
The unused4 is still ending to the v2 structures and the v3 starts at unused5.
This will avoid future corruption of v2 or v3.

The XFS_V2_INODE_SIZE is returning to its expected size and the
filesystem is back to a readable state.

                      GNU GRUB  version 2.11
        ....
        grub> set debug=efi,gpt,xfs
        grub> insmod part_gpt
        grub> ls (hd0,gpt1)/
        partmap/gpt.c:93: Read a valid GPT header
        partmap/gpt.c:115: GPT entry 0: start=4096, length=1953125
        fs/xfs.c:931: Reading sb
        fs/xfs.c:270: Validating superblock
        fs/xfs.c:295: XFS v4 superblock detected
        fs/xfs.c:962: Reading root ino 128
        fs/xfs.c:515: Reading inode (128) - 64, 0
        fs/xfs.c:515: Reading inode (128) - 64, 0
        fs/xfs.c:931: Reading sb
        fs/xfs.c:270: Validating superblock
        fs/xfs.c:295: XFS v4 superblock detected
        fs/xfs.c:962: Reading root ino 128
        fs/xfs.c:515: Reading inode (128) - 64, 0
        fs/xfs.c:515: Reading inode (128) - 64, 0
        fs/xfs.c:515: Reading inode (128) - 64, 0
        fs/xfs.c:515: Reading inode (131) - 64, 768
        efi/ fs/xfs.c:515: Reading inode (3145856) - 1464904, 0
        grub2/ fs/xfs.c:515: Reading inode (132) - 64, 1024
        grub/ fs/xfs.c:515: Reading inode (139) - 64, 2816
        grub>

Signed-off-by: Erwan Velu <e.velu@criteo.com>
---
 grub-core/fs/xfs.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
index 0f524c3a8a61..becea5bcdf61 100644
--- a/grub-core/fs/xfs.c
+++ b/grub-core/fs/xfs.c
@@ -192,6 +192,7 @@ struct grub_xfs_time_legacy
   grub_uint32_t nanosec;
 } GRUB_PACKED;

+// From https://mirrors.edge.kernel.org/pub/linux/utils/fs/xfs/docs/xfs_filesystem_structure.pdf about 'struct xfs_dinode_core'
 struct grub_xfs_inode
 {
   grub_uint8_t magic[2];
@@ -208,14 +209,15 @@ struct grub_xfs_inode
   grub_uint32_t nextents;
   grub_uint16_t unused3;
   grub_uint8_t fork_offset;
-  grub_uint8_t unused4[37];
+  grub_uint8_t unused4[17]; // Last member of inode v2
+  grub_uint8_t unused5[20]; // First member of inode v3
   grub_uint64_t flags2;
-  grub_uint8_t unused5[48];
+  grub_uint8_t unused6[48]; // Last member of inode v3
 } GRUB_PACKED;

 #define XFS_V3_INODE_SIZE      sizeof(struct grub_xfs_inode)
-/* Size of struct grub_xfs_inode until fork_offset (included). */
-#define XFS_V2_INODE_SIZE      (XFS_V3_INODE_SIZE - 92)
+/* Size of struct grub_xfs_inode v2, up to unused4 included */
+#define XFS_V2_INODE_SIZE      (XFS_V3_INODE_SIZE - 76)

 struct grub_xfs_dirblock_tail
 {
--
2.25.1