From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Ismael Luceno <iluceno@suse.de>
Date: Tue, 23 Jul 2024 11:19:35 +0000
Subject: [PATCH] Sanity-check the superblock during mount

Also, remove dead code and fix potential overflow in mask32lo.

Abort mounting a potentially malformed filesystem, and 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].


Ref: CVE-2023-4949
CC: Jan Beulich <jbeulich@suse.com>
[Based on patches by Alejandro Vallejo <alejandro.vallejo@cloud.com> to
 the Xen Project.]
Origin: Xen Project
Signed-off-by: Ismael Luceno <iluceno@suse.de>
---
 stage2/fsys_xfs.c |   69 +++++++++++++++++++++++++++++++-----------------------
 stage2/xfs.h      |   12 +++++++++
 2 files changed, 52 insertions(+), 29 deletions(-)

Index: grub-0.97/stage2/fsys_xfs.c
===================================================================
--- grub-0.97.orig/stage2/fsys_xfs.c
+++ grub-0.97/stage2/fsys_xfs.c
@@ -41,7 +41,6 @@ struct xfs_info {
 	int blklog;
 	int inopblog;
 	int agblklog;
-	int agnolog;
 	unsigned int nextents;
 	xfs_daddr_t next;
 	xfs_daddr_t daddr;
@@ -65,13 +64,11 @@ static struct xfs_info xfs;
 #define inode		((xfs_dinode_t *)((char *)FSYS_BUF + 8192))
 #define icore		(inode->di_core)
 
-#define	mask32lo(n)	(((xfs_uint32_t)1 << (n)) - 1)
+#define	mask32lo(n)	((xfs_uint32_t)((1ull << (n)) - 1))
 
 #define	XFS_INO_MASK(k)		((xfs_uint32_t)((1ULL << (k)) - 1))
 #define	XFS_INO_OFFSET_BITS	xfs.inopblog
-#define	XFS_INO_AGBNO_BITS	xfs.agblklog
 #define	XFS_INO_AGINO_BITS	(xfs.agblklog + xfs.inopblog)
-#define	XFS_INO_AGNO_BITS	xfs.agnolog
 
 static inline xfs_agblock_t
 agino2agbno (xfs_agino_t agino)
@@ -153,20 +150,6 @@ xt_len (xfs_bmbt_rec_32_t *r)
 	return le32(r->l3) & mask32lo(21);
 }
 
-static inline int
-xfs_highbit32(xfs_uint32_t v)
-{
-	int i;
-
-	if (--v) {
-		for (i = 0; i < 31; i++, v >>= 1) {
-			if (v == 0)
-				return i;
-		}
-	}
-	return 0;
-}
-
 static int
 isinxt (xfs_fileoff_t key, xfs_fileoff_t offset, xfs_filblks_t len)
 {
@@ -451,29 +434,57 @@ first_dentry (xfs_ino_t *ino)
 	return next_dentry (ino);
 }
 
+static int
+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);
+}
+
 int
 xfs_mount (void)
 {
 	xfs_sb_t super;
 
 	if (!devread (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;
-	xfs.agnolog = xfs_highbit32 (le32(super.sb_agcount));
+
+	/* Derived from sanitized parameters */
+	if (XFS_SB_BLOCKLOG_MIN < SECTOR_BITS)
+		return 0;
+	xfs.bdlog = super.sb_blocklog - SECTOR_BITS;
+	xfs.bsize = 1 << super.sb_blocklog;
+	xfs.blklog = super.sb_blocklog;
+	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)) /
Index: grub-0.97/stage2/xfs.h
===================================================================
--- grub-0.97.orig/stage2/xfs.h
+++ grub-0.97/stage2/xfs.h
@@ -135,6 +135,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 */
 
