[PATCH] gfs2: validate metadata block numbers before reading them

Li Pengfei posted 1 patch 1 week, 3 days ago
fs/gfs2/bmap.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
[PATCH] gfs2: validate metadata block numbers before reading them
Posted by Li Pengfei 1 week, 3 days ago
From: Pengfei Li <lipengfei28@xiaomi.com>

When mounting a crafted/corrupted gfs2 image, an inode's indirect block
may contain a block pointer that lies outside the filesystem.  Both the
metadata tree walk (__fillup_metapath -> gfs2_meta_buffer) and the
metadata read-ahead (gfs2_metapath_ra) take such a pointer straight from
disk and pass it to gfs2_getbuf() without any bounds check.

gfs2_getbuf() turns the block number into a page index via
"index = blkno >> shift".  A pointer of 0xffffffffffffffff therefore
inserts a metadata folio at index ULONG_MAX in the glock's metadata
mapping.  When the glock is put, __gfs2_glock_put() calls
truncate_inode_pages_final(), but that only scans the range
[0, ULONG_MAX-1] (truncate_inode_pages_range() passes end - 1 as the
inclusive last index), so the folio at ULONG_MAX is never removed.  The
withdraw path in __gfs2_glock_put() then skips the
GLOCK_BUG_ON(!mapping_empty()) assertion, and the glock -- with its
embedded address_space still holding the stray folio -- is freed via
RCU.  Later, when a per-cpu LRU batch is drained, folio_evictable()
dereferences the now-freed mapping, producing a slab-use-after-free that
surfaces far from gfs2 (e.g. in lru_add during an unrelated task's
shmem/fault path).

Reject block pointers that fall outside the block device before using
them.  In the tree-walk path flag the inode as inconsistent (which
withdraws the filesystem); in the read-ahead path, which is advisory,
just skip the bogus pointer.

Reported-by: syzbot+8921d5debffa05b33b27@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8921d5debffa05b33b27
Signed-off-by: Pengfei Li <lipengfei28@xiaomi.com>
---
 fs/gfs2/bmap.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 51ac1fd44..032acb964 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -291,8 +291,25 @@ static void clone_metapath(struct metapath *clone, struct metapath *mp)
 		get_bh(clone->mp_bh[hgt]);
 }
 
+/*
+ * On a corrupted filesystem, an indirect block may contain a pointer that
+ * lies outside the filesystem.  Feeding such a block number to gfs2_getbuf()
+ * creates a page far beyond the end of the metadata mapping (at an index up
+ * to ULONG_MAX), which truncate_inode_pages_final() never reaches; the page
+ * is then left dangling when the glock is freed.  Reject pointers that are
+ * outside the block device.
+ */
+static bool gfs2_meta_blk_out_of_range(const struct gfs2_sbd *sdp, u64 blk)
+{
+	u64 blocks = bdev_nr_bytes(sdp->sd_vfs->s_bdev) >>
+		     sdp->sd_sb.sb_bsize_shift;
+
+	return blk >= blocks;
+}
+
 static void gfs2_metapath_ra(struct gfs2_glock *gl, __be64 *start, __be64 *end)
 {
+	struct gfs2_sbd *sdp = glock_sbd(gl);
 	const __be64 *t;
 
 	for (t = start; t < end; t++) {
@@ -300,6 +317,8 @@ static void gfs2_metapath_ra(struct gfs2_glock *gl, __be64 *start, __be64 *end)
 
 		if (!*t)
 			continue;
+		if (gfs2_meta_blk_out_of_range(sdp, be64_to_cpu(*t)))
+			continue;
 
 		rabh = gfs2_getbuf(gl, be64_to_cpu(*t), CREATE);
 		if (trylock_buffer(rabh)) {
@@ -325,6 +344,8 @@ metapath_dibh(struct metapath *mp)
 static int __fillup_metapath(struct gfs2_inode *ip, struct metapath *mp,
 			     unsigned int x, unsigned int h)
 {
+	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
+
 	for (; x < h; x++) {
 		__be64 *ptr = metapointer(x, mp);
 		u64 dblock = be64_to_cpu(*ptr);
@@ -332,6 +353,10 @@ static int __fillup_metapath(struct gfs2_inode *ip, struct metapath *mp,
 
 		if (!dblock)
 			break;
+		if (gfs2_meta_blk_out_of_range(sdp, dblock)) {
+			gfs2_consist_inode(ip);
+			return -EIO;
+		}
 		ret = gfs2_meta_buffer(ip, GFS2_METATYPE_IN, dblock, &mp->mp_bh[x + 1]);
 		if (ret)
 			return ret;
-- 
2.34.1