libfsimage/xfs: Sanity-check the superblock during mounts

Sanity-check the XFS superblock for wellformedness at the mount handler.
This forces pygrub to abort parsing a potentially malformed filesystem and
ensures the invariants assumed throughout the rest of the code hold.

Also, derive parameters from previously sanitized parameters where possible
(rather than reading them off the superblock)

The code doesn't try to avoid overflowing the end of the disk, because
that's an unlikely and benign error. Parameters used in calculations of
xfs_daddr_t (like the root inode index) aren't in critical need of being
sanitized.

The sanitization of agblklog is basically checking that no obvious
overflows happen on agblklog, and then ensuring agblocks is contained in
the range (2^(sb_agblklog-1), 2^sb_agblklog].

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/tools/libfsimage/xfs/fsys_xfs.c b/tools/libfsimage/xfs/fsys_xfs.c
index dbdb21d156e03852e75aac0cf9770bb06663e0be..b5c53d3d222b4577a3905b366b3658a989f2a551 100644 (file)
--- a/tools/libfsimage/xfs/fsys_xfs.c
+++ b/tools/libfsimage/xfs/fsys_xfs.c
@@ -18,6 +18,7 @@
  */
 
 #include <stddef.h>
+#include <stdbool.h>
 #include <xenfsimage_grub.h>
 #include "xfs.h"
 
@@ -431,29 +432,56 @@ first_dentry (fsi_file_t *ffi, xfs_ino_t *ino)
        return next_dentry (ffi, ino);
 }
 
+static bool
+xfs_sb_is_invalid (const xfs_sb_t *super)
+{
+       return (le32(super->sb_magicnum) != XFS_SB_MAGIC)
+           || ((le16(super->sb_versionnum) & XFS_SB_VERSION_NUMBITS) !=
+               XFS_SB_VERSION_4)
+           || (super->sb_inodelog < XFS_SB_INODELOG_MIN)
+           || (super->sb_inodelog > XFS_SB_INODELOG_MAX)
+           || (super->sb_blocklog < XFS_SB_BLOCKLOG_MIN)
+           || (super->sb_blocklog > XFS_SB_BLOCKLOG_MAX)
+           || (super->sb_blocklog < super->sb_inodelog)
+           || (super->sb_agblklog > XFS_SB_AGBLKLOG_MAX)
+           || ((1ull << super->sb_agblklog) < le32(super->sb_agblocks))
+           || (((1ull << super->sb_agblklog) >> 1) >=
+               le32(super->sb_agblocks))
+           || ((super->sb_blocklog + super->sb_dirblklog) >=
+               XFS_SB_DIRBLK_NUMBITS);
+}
+
 static int
 xfs_mount (fsi_file_t *ffi, const char *options)
 {
        xfs_sb_t super;
 
        if (!devread (ffi, 0, 0, sizeof(super), (char *)&super)
-           || (le32(super.sb_magicnum) != XFS_SB_MAGIC)
-           || ((le16(super.sb_versionnum) 
-               & XFS_SB_VERSION_NUMBITS) != XFS_SB_VERSION_4) ) {
+           || xfs_sb_is_invalid(&super)) {
                return 0;
        }
 
-       xfs.bsize = le32 (super.sb_blocksize);
-       xfs.blklog = super.sb_blocklog;
-       xfs.bdlog = xfs.blklog - SECTOR_BITS;
+       /*
+        * Not sanitized. It's exclusively used to generate disk addresses,
+        * so it's not important from a security standpoint.
+        */
        xfs.rootino = le64 (super.sb_rootino);
-       xfs.isize = le16 (super.sb_inodesize);
-       xfs.agblocks = le32 (super.sb_agblocks);
-       xfs.dirbsize = xfs.bsize << super.sb_dirblklog;
 
-       xfs.inopblog = super.sb_inopblog;
+       /*
+        * Sanitized to be consistent with each other, only used to
+        * generate disk addresses, so it's safe
+        */
+       xfs.agblocks = le32 (super.sb_agblocks);
        xfs.agblklog = super.sb_agblklog;
 
+       /* Derived from sanitized parameters */
+       xfs.bsize = 1 << super.sb_blocklog;
+       xfs.blklog = super.sb_blocklog;
+       xfs.bdlog = super.sb_blocklog - SECTOR_BITS;
+       xfs.isize = 1 << super.sb_inodelog;
+       xfs.dirbsize = 1 << (super.sb_blocklog + super.sb_dirblklog);
+       xfs.inopblog = super.sb_blocklog - super.sb_inodelog;
+
        xfs.btnode_ptr0_off =
                ((xfs.bsize - sizeof(xfs_btree_block_t)) /
                (sizeof (xfs_bmbt_key_t) + sizeof (xfs_bmbt_ptr_t)))

diff --git a/tools/libfsimage/xfs/xfs.h b/tools/libfsimage/xfs/xfs.h
index 40699281e44d6a9587ef40a61563d71424292a4a..b87e37d3d7e9a20ab7d7f0cbbc00a84a866a073f 100644 (file)
--- a/tools/libfsimage/xfs/xfs.h
+++ b/tools/libfsimage/xfs/xfs.h
@@ -134,6 +134,18 @@ typedef struct xfs_sb
         xfs_uint8_t       sb_dummy[7];    /* padding */
 } xfs_sb_t;
 
+/* Bound taken from xfs.c in GRUB2. It doesn't exist in the spec */
+#define        XFS_SB_DIRBLK_NUMBITS   27
+/* Implied by the XFS specification. The minimum block size is 512 octets */
+#define        XFS_SB_BLOCKLOG_MIN     9
+/* Implied by the XFS specification. The maximum block size is 65536 octets */
+#define        XFS_SB_BLOCKLOG_MAX     16
+/* Implied by the XFS specification. The minimum inode size is 256 octets */
+#define        XFS_SB_INODELOG_MIN     8
+/* Implied by the XFS specification. The maximum inode size is 2048 octets */
+#define        XFS_SB_INODELOG_MAX     11
+/* High bound for sb_agblklog */
+#define        XFS_SB_AGBLKLOG_MAX     32
 
 /* those are from xfs_btree.h */