1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
|
From 2de503f8fd0d07401e92abed1097ceb5fd1801f6 Mon Sep 17 00:00:00 2001
From: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Date: Thu, 14 Sep 2023 13:22:52 +0100
Subject: [PATCH 17/30] 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>
(cherry picked from commit 620500dd1baf33347dfde5e7fde7cf7fe347da5c)
---
tools/libfsimage/xfs/fsys_xfs.c | 48 ++++++++++++++++++++++++++-------
tools/libfsimage/xfs/xfs.h | 12 +++++++++
2 files changed, 50 insertions(+), 10 deletions(-)
diff --git a/tools/libfsimage/xfs/fsys_xfs.c b/tools/libfsimage/xfs/fsys_xfs.c
index 4720bb4505..e4eb7e1ee2 100644
--- a/tools/libfsimage/xfs/fsys_xfs.c
+++ b/tools/libfsimage/xfs/fsys_xfs.c
@@ -17,6 +17,7 @@
* along with this program; If not, see <http://www.gnu.org/licenses/>.
*/
+#include <stdbool.h>
#include <xenfsimage_grub.h>
#include "xfs.h"
@@ -433,29 +434,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 40699281e4..b87e37d3d7 100644
--- 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 */
--
2.43.0
|
GitWeb