diff mbox series

[1/7] xfs_repair: don't crash on partially sparse inode clusters

Message ID 159950112377.567790.5885407242137390700.stgit@magnolia
State Accepted
Headers show
Series xfs_repair: more fuzzer fixes | expand

Commit Message

Darrick J. Wong Sept. 7, 2020, 5:52 p.m. UTC
From: Darrick J. Wong <darrick.wong@oracle.com>

While running xfs/364 to fuzz the middle bit of recs[2].holemask, I
observed a crash in xfs_repair stemming from the fact that each sparse
bit accounts for 4 inodes, but inode cluster buffers can map to more
than four inodes.

When the first inode in an inode cluster is marked sparse,
process_inode_chunk won't try to load the inode cluster buffer.
Unfortunately, if the holemask indicates that there are inodes present
anywhere in the rest of the cluster buffer, repair will try to check the
corresponding cluster buffer, even if we didn't load it.  This leads to
a null pointer dereference, which crashes repair.

Avoid the null pointer dereference by marking the inode sparse and
moving on to the next inode.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/dino_chunks.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Christoph Hellwig Sept. 8, 2020, 2:43 p.m. UTC | #1
On Mon, Sep 07, 2020 at 10:52:03AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> While running xfs/364 to fuzz the middle bit of recs[2].holemask, I
> observed a crash in xfs_repair stemming from the fact that each sparse
> bit accounts for 4 inodes, but inode cluster buffers can map to more
> than four inodes.
> 
> When the first inode in an inode cluster is marked sparse,
> process_inode_chunk won't try to load the inode cluster buffer.
> Unfortunately, if the holemask indicates that there are inodes present
> anywhere in the rest of the cluster buffer, repair will try to check the
> corresponding cluster buffer, even if we didn't load it.  This leads to
> a null pointer dereference, which crashes repair.
> 
> Avoid the null pointer dereference by marking the inode sparse and
> moving on to the next inode.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
index 96e2c1708b94..50a2003614df 100644
--- a/repair/dino_chunks.c
+++ b/repair/dino_chunks.c
@@ -792,6 +792,25 @@  process_inode_chunk(
 		if (is_inode_sparse(ino_rec, irec_offset))
 			goto process_next;
 
+		/*
+		 * Repair skips reading the cluster buffer if the first inode
+		 * in the cluster is marked as sparse.  If subsequent inodes in
+		 * the cluster buffer are /not/ marked sparse, there won't be
+		 * a buffer, so we need to avoid the null pointer dereference.
+		 */
+		if (bplist[bp_index] == NULL) {
+			do_warn(
+	_("imap claims inode %" PRIu64 " is present, but inode cluster is sparse, "),
+						ino);
+			if (verbose || !no_modify)
+				do_warn(_("correcting imap\n"));
+			else
+				do_warn(_("would correct imap\n"));
+			set_inode_sparse(ino_rec, irec_offset);
+			set_inode_free(ino_rec, irec_offset);
+			goto process_next;
+		}
+
 		/* make inode pointer */
 		dino = xfs_make_iptr(mp, bplist[bp_index], cluster_offset);