[PATCHSET] xfs: syzbot fixes for online fsck

[PATCHSET] xfs: syzbot fixes for online fsck

[Darrick J. Wong]

Hi all,

Fix various syzbot complaints about scrub.

With a bit of luck, this should all go splendidly.
Comments and questions are, as always, welcome.

--D
---
Commits in this patchset:
 * xfs: check the return value of xchk_xfile_*_descr calls
 * xfs: only call xf{array,blob}_destroy if we have a valid pointer
 * xfs: check return value of xchk_scrub_create_subord
 * xfs: fix UAF in xchk_btree_check_block_owner
---
 fs/xfs/scrub/agheader_repair.c   |   14 ++++++++++++--
 fs/xfs/scrub/alloc_repair.c      |    5 +++++
 fs/xfs/scrub/attr_repair.c       |   26 ++++++++++++++++++++++++--
 fs/xfs/scrub/bmap_repair.c       |    5 +++++
 fs/xfs/scrub/btree.c             |    7 +++++--
 fs/xfs/scrub/common.c            |    3 +++
 fs/xfs/scrub/dir.c               |   10 ++++++++++
 fs/xfs/scrub/dir_repair.c        |   16 ++++++++++++++--
 fs/xfs/scrub/dirtree.c           |   18 ++++++++++++++++--
 fs/xfs/scrub/ialloc_repair.c     |    5 +++++
 fs/xfs/scrub/nlinks.c            |    8 +++++++-
 fs/xfs/scrub/parent.c            |    8 ++++++++
 fs/xfs/scrub/parent_repair.c     |   20 ++++++++++++++++++++
 fs/xfs/scrub/quotacheck.c        |   15 +++++++++++++++
 fs/xfs/scrub/refcount_repair.c   |    8 ++++++++
 fs/xfs/scrub/repair.c            |    3 +++
 fs/xfs/scrub/rmap_repair.c       |    3 +++
 fs/xfs/scrub/rtbitmap_repair.c   |    3 +++
 fs/xfs/scrub/rtrefcount_repair.c |    8 ++++++++
 fs/xfs/scrub/rtrmap_repair.c     |    3 +++
 fs/xfs/scrub/rtsummary.c         |    3 +++
 fs/xfs/scrub/scrub.c             |    2 +-
 22 files changed, 181 insertions(+), 12 deletions(-)

[PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

The xchk_xfile_*_descr macros call kasprintf, which can fail to allocate
memory if the formatted string is larger than 16 bytes (or whatever the
nofail guarantees are nowadays).  Some of them could easily exceed that,
so let's just add return value checking across the board.  Note that
this patch touches a number of commits, most of which were merged
between 6.6 and 6.14.

Cc: r772577952@gmail.com
Cc: <stable@vger.kernel.org> # v6.12
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/agheader_repair.c   |    6 ++++++
 fs/xfs/scrub/alloc_repair.c      |    5 +++++
 fs/xfs/scrub/attr_repair.c       |   20 ++++++++++++++++++++
 fs/xfs/scrub/bmap_repair.c       |    5 +++++
 fs/xfs/scrub/dir.c               |   10 ++++++++++
 fs/xfs/scrub/dir_repair.c        |    8 ++++++++
 fs/xfs/scrub/dirtree.c           |   10 ++++++++++
 fs/xfs/scrub/ialloc_repair.c     |    5 +++++
 fs/xfs/scrub/nlinks.c            |    5 +++++
 fs/xfs/scrub/parent.c            |    8 ++++++++
 fs/xfs/scrub/parent_repair.c     |   20 ++++++++++++++++++++
 fs/xfs/scrub/quotacheck.c        |   15 +++++++++++++++
 fs/xfs/scrub/refcount_repair.c   |    8 ++++++++
 fs/xfs/scrub/rmap_repair.c       |    3 +++
 fs/xfs/scrub/rtbitmap_repair.c   |    3 +++
 fs/xfs/scrub/rtrefcount_repair.c |    8 ++++++++
 fs/xfs/scrub/rtrmap_repair.c     |    3 +++
 fs/xfs/scrub/rtsummary.c         |    3 +++
 18 files changed, 145 insertions(+)


diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index cd6f0223879f49..8d7762cf5daffd 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -1743,6 +1743,9 @@ xrep_agi(
 	sc->buf_cleanup = xrep_agi_buf_cleanup;
 
 	descr = xchk_xfile_ag_descr(sc, "iunlinked next pointers");
+	if (!descr)
+		return -ENOMEM;
+
 	error = xfarray_create(descr, 0, sizeof(xfs_agino_t),
 			&ragi->iunlink_next);
 	kfree(descr);
@@ -1750,6 +1753,9 @@ xrep_agi(
 		return error;
 
 	descr = xchk_xfile_ag_descr(sc, "iunlinked prev pointers");
+	if (!descr)
+		return -ENOMEM;
+
 	error = xfarray_create(descr, 0, sizeof(xfs_agino_t),
 			&ragi->iunlink_prev);
 	kfree(descr);
diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
index bed6a09aa79112..2e1d62efba72a7 100644
--- a/fs/xfs/scrub/alloc_repair.c
+++ b/fs/xfs/scrub/alloc_repair.c
@@ -877,6 +877,11 @@ xrep_allocbt(
 
 	/* Set up enough storage to handle maximally fragmented free space. */
 	descr = xchk_xfile_ag_descr(sc, "free space records");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_ra;
+	}
+
 	error = xfarray_create(descr, mp->m_sb.sb_agblocks / 2,
 			sizeof(struct xfs_alloc_rec_incore),
 			&ra->free_records);
diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c
index c7eb94069cafcd..73684ce9b81bc5 100644
--- a/fs/xfs/scrub/attr_repair.c
+++ b/fs/xfs/scrub/attr_repair.c
@@ -1556,6 +1556,11 @@ xrep_xattr_setup_scan(
 
 	/* Set up some staging for salvaged attribute keys and values */
 	descr = xchk_xfile_ino_descr(sc, "xattr keys");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_rx;
+	}
+
 	error = xfarray_create(descr, 0, sizeof(struct xrep_xattr_key),
 			&rx->xattr_records);
 	kfree(descr);
@@ -1563,6 +1568,11 @@ xrep_xattr_setup_scan(
 		goto out_rx;
 
 	descr = xchk_xfile_ino_descr(sc, "xattr names");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_keys;
+	}
+
 	error = xfblob_create(descr, &rx->xattr_blobs);
 	kfree(descr);
 	if (error)
@@ -1573,6 +1583,11 @@ xrep_xattr_setup_scan(
 
 		descr = xchk_xfile_ino_descr(sc,
 				"xattr retained parent pointer entries");
+		if (!descr) {
+			error = -ENOMEM;
+			goto out_values;
+		}
+
 		error = xfarray_create(descr, 0,
 				sizeof(struct xrep_xattr_pptr),
 				&rx->pptr_recs);
@@ -1582,6 +1597,11 @@ xrep_xattr_setup_scan(
 
 		descr = xchk_xfile_ino_descr(sc,
 				"xattr retained parent pointer names");
+		if (!descr) {
+			error = -ENOMEM;
+			goto out_pprecs;
+		}
+
 		error = xfblob_create(descr, &rx->pptr_names);
 		kfree(descr);
 		if (error)
diff --git a/fs/xfs/scrub/bmap_repair.c b/fs/xfs/scrub/bmap_repair.c
index 1084213b8e9b88..74df05142dcf4c 100644
--- a/fs/xfs/scrub/bmap_repair.c
+++ b/fs/xfs/scrub/bmap_repair.c
@@ -947,6 +947,11 @@ xrep_bmap(
 	max_bmbt_recs = xfs_iext_max_nextents(large_extcount, whichfork);
 	descr = xchk_xfile_ino_descr(sc, "%s fork mapping records",
 			whichfork == XFS_DATA_FORK ? "data" : "attr");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_rb;
+	}
+
 	error = xfarray_create(descr, max_bmbt_recs,
 			sizeof(struct xfs_bmbt_rec), &rb->bmap_records);
 	kfree(descr);
diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index c877bde71e6280..58346d54042b07 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -1109,6 +1109,11 @@ xchk_directory(
 		 * due to locking contention.
 		 */
 		descr = xchk_xfile_ino_descr(sc, "slow directory entries");
+		if (!descr) {
+			error = -ENOMEM;
+			goto out_sd;
+		}
+
 		error = xfarray_create(descr, 0, sizeof(struct xchk_dirent),
 				&sd->dir_entries);
 		kfree(descr);
@@ -1116,6 +1121,11 @@ xchk_directory(
 			goto out_sd;
 
 		descr = xchk_xfile_ino_descr(sc, "slow directory entry names");
+		if (!descr) {
+			error = -ENOMEM;
+			goto out_entries;
+		}
+
 		error = xfblob_create(descr, &sd->dir_names);
 		kfree(descr);
 		if (error)
diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index 8d3b550990b58a..50e0af4bdaa63a 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -1789,6 +1789,9 @@ xrep_dir_setup_scan(
 
 	/* Set up some staging memory for salvaging dirents. */
 	descr = xchk_xfile_ino_descr(sc, "directory entries");
+	if (!descr)
+		return -ENOMEM;
+
 	error = xfarray_create(descr, 0, sizeof(struct xrep_dirent),
 			&rd->dir_entries);
 	kfree(descr);
@@ -1796,6 +1799,11 @@ xrep_dir_setup_scan(
 		return error;
 
 	descr = xchk_xfile_ino_descr(sc, "directory entry names");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_xfarray;
+	}
+
 	error = xfblob_create(descr, &rd->dir_names);
 	kfree(descr);
 	if (error)
diff --git a/fs/xfs/scrub/dirtree.c b/fs/xfs/scrub/dirtree.c
index 3a9cdf8738b6db..7f8ad41e3ec20e 100644
--- a/fs/xfs/scrub/dirtree.c
+++ b/fs/xfs/scrub/dirtree.c
@@ -117,6 +117,11 @@ xchk_setup_dirtree(
 	mutex_init(&dl->lock);
 
 	descr = xchk_xfile_ino_descr(sc, "dirtree path steps");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_dl;
+	}
+
 	error = xfarray_create(descr, 0, sizeof(struct xchk_dirpath_step),
 			&dl->path_steps);
 	kfree(descr);
@@ -124,6 +129,11 @@ xchk_setup_dirtree(
 		goto out_dl;
 
 	descr = xchk_xfile_ino_descr(sc, "dirtree path names");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_steps;
+	}
+
 	error = xfblob_create(descr, &dl->path_names);
 	kfree(descr);
 	if (error)
diff --git a/fs/xfs/scrub/ialloc_repair.c b/fs/xfs/scrub/ialloc_repair.c
index 14e48d3f1912bf..3055380cf29271 100644
--- a/fs/xfs/scrub/ialloc_repair.c
+++ b/fs/xfs/scrub/ialloc_repair.c
@@ -817,6 +817,11 @@ xrep_iallocbt(
 	xfs_agino_range(mp, pag_agno(sc->sa.pag), &first_agino, &last_agino);
 	last_agino /= XFS_INODES_PER_CHUNK;
 	descr = xchk_xfile_ag_descr(sc, "inode index records");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_ri;
+	}
+
 	error = xfarray_create(descr, last_agino,
 			sizeof(struct xfs_inobt_rec_incore),
 			&ri->inode_records);
diff --git a/fs/xfs/scrub/nlinks.c b/fs/xfs/scrub/nlinks.c
index 091c79e432e592..c71b065ccb4c45 100644
--- a/fs/xfs/scrub/nlinks.c
+++ b/fs/xfs/scrub/nlinks.c
@@ -1008,6 +1008,11 @@ xchk_nlinks_setup_scan(
 	xfs_agino_range(mp, last_agno, &first_agino, &last_agino);
 	max_inos = XFS_AGINO_TO_INO(mp, last_agno, last_agino) + 1;
 	descr = xchk_xfile_descr(sc, "file link counts");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_teardown;
+	}
+
 	error = xfarray_create(descr, min(XFS_MAXINUMBER + 1, max_inos),
 			sizeof(struct xchk_nlink), &xnc->nlinks);
 	kfree(descr);
diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c
index 11d5de10fd567b..11c70e5d3e03de 100644
--- a/fs/xfs/scrub/parent.c
+++ b/fs/xfs/scrub/parent.c
@@ -769,6 +769,9 @@ xchk_parent_pptr(
 	 * due to locking contention.
 	 */
 	descr = xchk_xfile_ino_descr(sc, "slow parent pointer entries");
+	if (!descr)
+		return -ENOMEM;
+
 	error = xfarray_create(descr, 0, sizeof(struct xchk_pptr),
 			&pp->pptr_entries);
 	kfree(descr);
@@ -776,6 +779,11 @@ xchk_parent_pptr(
 		goto out_pp;
 
 	descr = xchk_xfile_ino_descr(sc, "slow parent pointer names");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_entries;
+	}
+
 	error = xfblob_create(descr, &pp->pptr_names);
 	kfree(descr);
 	if (error)
diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c
index 2949feda627175..8683317f2342df 100644
--- a/fs/xfs/scrub/parent_repair.c
+++ b/fs/xfs/scrub/parent_repair.c
@@ -1526,6 +1526,11 @@ xrep_parent_setup_scan(
 
 	/* Set up some staging memory for logging parent pointer updates. */
 	descr = xchk_xfile_ino_descr(sc, "parent pointer entries");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_xattr_value;
+	}
+
 	error = xfarray_create(descr, 0, sizeof(struct xrep_pptr),
 			&rp->pptr_recs);
 	kfree(descr);
@@ -1533,6 +1538,11 @@ xrep_parent_setup_scan(
 		goto out_xattr_value;
 
 	descr = xchk_xfile_ino_descr(sc, "parent pointer names");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_recs;
+	}
+
 	error = xfblob_create(descr, &rp->pptr_names);
 	kfree(descr);
 	if (error)
@@ -1541,6 +1551,11 @@ xrep_parent_setup_scan(
 	/* Set up some storage for copying attrs before the mapping exchange */
 	descr = xchk_xfile_ino_descr(sc,
 				"parent pointer retained xattr entries");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_names;
+	}
+
 	error = xfarray_create(descr, 0, sizeof(struct xrep_parent_xattr),
 			&rp->xattr_records);
 	kfree(descr);
@@ -1549,6 +1564,11 @@ xrep_parent_setup_scan(
 
 	descr = xchk_xfile_ino_descr(sc,
 				"parent pointer retained xattr values");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_attr_keys;
+	}
+
 	error = xfblob_create(descr, &rp->xattr_blobs);
 	kfree(descr);
 	if (error)
diff --git a/fs/xfs/scrub/quotacheck.c b/fs/xfs/scrub/quotacheck.c
index d412a8359784ee..7d0ad19ddf577d 100644
--- a/fs/xfs/scrub/quotacheck.c
+++ b/fs/xfs/scrub/quotacheck.c
@@ -757,6 +757,11 @@ xqcheck_setup_scan(
 	error = -ENOMEM;
 	if (xfs_this_quota_on(sc->mp, XFS_DQTYPE_USER)) {
 		descr = xchk_xfile_descr(sc, "user dquot records");
+		if (!descr) {
+			error = -ENOMEM;
+			goto out_teardown;
+		}
+
 		error = xfarray_create(descr, max_dquots,
 				sizeof(struct xqcheck_dquot), &xqc->ucounts);
 		kfree(descr);
@@ -766,6 +771,11 @@ xqcheck_setup_scan(
 
 	if (xfs_this_quota_on(sc->mp, XFS_DQTYPE_GROUP)) {
 		descr = xchk_xfile_descr(sc, "group dquot records");
+		if (!descr) {
+			error = -ENOMEM;
+			goto out_teardown;
+		}
+
 		error = xfarray_create(descr, max_dquots,
 				sizeof(struct xqcheck_dquot), &xqc->gcounts);
 		kfree(descr);
@@ -775,6 +785,11 @@ xqcheck_setup_scan(
 
 	if (xfs_this_quota_on(sc->mp, XFS_DQTYPE_PROJ)) {
 		descr = xchk_xfile_descr(sc, "project dquot records");
+		if (!descr) {
+			error = -ENOMEM;
+			goto out_teardown;
+		}
+
 		error = xfarray_create(descr, max_dquots,
 				sizeof(struct xqcheck_dquot), &xqc->pcounts);
 		kfree(descr);
diff --git a/fs/xfs/scrub/refcount_repair.c b/fs/xfs/scrub/refcount_repair.c
index 9c8cb5332da042..d53c9a5bb7809c 100644
--- a/fs/xfs/scrub/refcount_repair.c
+++ b/fs/xfs/scrub/refcount_repair.c
@@ -127,6 +127,9 @@ xrep_setup_ag_refcountbt(
 	int			error;
 
 	descr = xchk_xfile_ag_descr(sc, "rmap record bag");
+	if (!descr)
+		return -ENOMEM;
+
 	error = xrep_setup_xfbtree(sc, descr);
 	kfree(descr);
 	return error;
@@ -718,6 +721,11 @@ xrep_refcountbt(
 
 	/* Set up enough storage to handle one refcount record per block. */
 	descr = xchk_xfile_ag_descr(sc, "reference count records");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_rr;
+	}
+
 	error = xfarray_create(descr, mp->m_sb.sb_agblocks,
 			sizeof(struct xfs_refcount_irec),
 			&rr->refcount_records);
diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
index 17d4a38d735cb8..c619ba469e36de 100644
--- a/fs/xfs/scrub/rmap_repair.c
+++ b/fs/xfs/scrub/rmap_repair.c
@@ -170,6 +170,9 @@ xrep_setup_ag_rmapbt(
 	xchk_fsgates_enable(sc, XCHK_FSGATES_RMAP);
 
 	descr = xchk_xfile_ag_descr(sc, "reverse mapping records");
+	if (!descr)
+		return -ENOMEM;
+
 	error = xrep_setup_xfbtree(sc, descr);
 	kfree(descr);
 	if (error)
diff --git a/fs/xfs/scrub/rtbitmap_repair.c b/fs/xfs/scrub/rtbitmap_repair.c
index 203a1a97c5026e..070347df717c46 100644
--- a/fs/xfs/scrub/rtbitmap_repair.c
+++ b/fs/xfs/scrub/rtbitmap_repair.c
@@ -53,6 +53,9 @@ xrep_setup_rtbitmap(
 
 	/* Create an xfile to hold our reconstructed bitmap. */
 	descr = xchk_xfile_rtgroup_descr(sc, "bitmap file");
+	if (!descr)
+		return -ENOMEM;
+
 	error = xfile_create(descr, blocks * mp->m_sb.sb_blocksize, &sc->xfile);
 	kfree(descr);
 	if (error)
diff --git a/fs/xfs/scrub/rtrefcount_repair.c b/fs/xfs/scrub/rtrefcount_repair.c
index 983362447826de..029e3e332f605e 100644
--- a/fs/xfs/scrub/rtrefcount_repair.c
+++ b/fs/xfs/scrub/rtrefcount_repair.c
@@ -132,6 +132,9 @@ xrep_setup_rtrefcountbt(
 	int			error;
 
 	descr = xchk_xfile_ag_descr(sc, "rmap record bag");
+	if (!descr)
+		return -ENOMEM;
+
 	error = xrep_setup_xfbtree(sc, descr);
 	kfree(descr);
 	return error;
@@ -723,6 +726,11 @@ xrep_rtrefcountbt(
 
 	/* Set up enough storage to handle one refcount record per rt extent. */
 	descr = xchk_xfile_ag_descr(sc, "reference count records");
+	if (!descr) {
+		error = -ENOMEM;
+		goto out_rr;
+	}
+
 	error = xfarray_create(descr, mp->m_sb.sb_rextents,
 			sizeof(struct xfs_refcount_irec),
 			&rr->refcount_records);
diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
index 7561941a337a1f..c74d640068d1c8 100644
--- a/fs/xfs/scrub/rtrmap_repair.c
+++ b/fs/xfs/scrub/rtrmap_repair.c
@@ -109,6 +109,9 @@ xrep_setup_rtrmapbt(
 	xchk_fsgates_enable(sc, XCHK_FSGATES_RMAP);
 
 	descr = xchk_xfile_rtgroup_descr(sc, "reverse mapping records");
+	if (!descr)
+		return -ENOMEM;
+
 	error = xrep_setup_xfbtree(sc, descr);
 	kfree(descr);
 	if (error)
diff --git a/fs/xfs/scrub/rtsummary.c b/fs/xfs/scrub/rtsummary.c
index 4ac679c1bd29cd..bf2b96e51d070c 100644
--- a/fs/xfs/scrub/rtsummary.c
+++ b/fs/xfs/scrub/rtsummary.c
@@ -71,6 +71,9 @@ xchk_setup_rtsummary(
 	 * us to avoid pinning kernel memory for this purpose.
 	 */
 	descr = xchk_xfile_descr(sc, "realtime summary file");
+	if (!descr)
+		return -ENOMEM;
+
 	error = xfile_create(descr, XFS_FSB_TO_B(mp, mp->m_rsumblocks),
 			&sc->xfile);
 	kfree(descr);

[PATCH 2/4] xfs: only call xf{array,blob}_destroy if we have a valid pointer

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Only call the xfarray and xfblob destructor if we have a valid pointer,
and be sure to null out that pointer afterwards.  Note that this patch
fixes a large number of commits, most of which were merged between 6.9
and 6.10.

Cc: r772577952@gmail.com
Cc: <stable@vger.kernel.org> # v6.12
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/agheader_repair.c |    8 ++++++--
 fs/xfs/scrub/attr_repair.c     |    6 ++++--
 fs/xfs/scrub/dir_repair.c      |    8 ++++++--
 fs/xfs/scrub/dirtree.c         |    8 ++++++--
 fs/xfs/scrub/nlinks.c          |    3 ++-
 5 files changed, 24 insertions(+), 9 deletions(-)


diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 8d7762cf5daffd..07fc028302dc40 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -837,8 +837,12 @@ xrep_agi_buf_cleanup(
 {
 	struct xrep_agi	*ragi = buf;
 
-	xfarray_destroy(ragi->iunlink_prev);
-	xfarray_destroy(ragi->iunlink_next);
+	if (ragi->iunlink_prev)
+		xfarray_destroy(ragi->iunlink_prev);
+	ragi->iunlink_prev = NULL;
+	if (ragi->iunlink_next)
+		xfarray_destroy(ragi->iunlink_next);
+	ragi->iunlink_next = NULL;
 	xagino_bitmap_destroy(&ragi->iunlink_bmp);
 }
 
diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c
index 73684ce9b81bc5..ceae6eaf894535 100644
--- a/fs/xfs/scrub/attr_repair.c
+++ b/fs/xfs/scrub/attr_repair.c
@@ -1516,8 +1516,10 @@ xrep_xattr_teardown(
 		xfblob_destroy(rx->pptr_names);
 	if (rx->pptr_recs)
 		xfarray_destroy(rx->pptr_recs);
-	xfblob_destroy(rx->xattr_blobs);
-	xfarray_destroy(rx->xattr_records);
+	if (rx->xattr_blobs)
+		xfblob_destroy(rx->xattr_blobs);
+	if (rx->xattr_records)
+		xfarray_destroy(rx->xattr_records);
 	mutex_destroy(&rx->lock);
 	kfree(rx);
 }
diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index 50e0af4bdaa63a..ca841e445e2d9d 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -172,8 +172,12 @@ xrep_dir_teardown(
 	struct xrep_dir		*rd = sc->buf;
 
 	xrep_findparent_scan_teardown(&rd->pscan);
-	xfblob_destroy(rd->dir_names);
-	xfarray_destroy(rd->dir_entries);
+	if (rd->dir_names)
+		xfblob_destroy(rd->dir_names);
+	rd->dir_names = NULL;
+	if (rd->dir_entries)
+		xfarray_destroy(rd->dir_entries);
+	rd->dir_names = NULL;
 }
 
 /* Set up for a directory repair. */
diff --git a/fs/xfs/scrub/dirtree.c b/fs/xfs/scrub/dirtree.c
index 7f8ad41e3ec20e..4064d3f3dca09b 100644
--- a/fs/xfs/scrub/dirtree.c
+++ b/fs/xfs/scrub/dirtree.c
@@ -81,8 +81,12 @@ xchk_dirtree_buf_cleanup(
 		kfree(path);
 	}
 
-	xfblob_destroy(dl->path_names);
-	xfarray_destroy(dl->path_steps);
+	if (dl->path_names)
+		xfblob_destroy(dl->path_names);
+	dl->path_names = NULL;
+	if (dl->path_steps)
+		xfarray_destroy(dl->path_steps);
+	dl->path_steps = NULL;
 	mutex_destroy(&dl->lock);
 }
 
diff --git a/fs/xfs/scrub/nlinks.c b/fs/xfs/scrub/nlinks.c
index c71b065ccb4c45..d1ac39d7867a21 100644
--- a/fs/xfs/scrub/nlinks.c
+++ b/fs/xfs/scrub/nlinks.c
@@ -971,7 +971,8 @@ xchk_nlinks_teardown_scan(
 
 	xfs_dir_hook_del(xnc->sc->mp, &xnc->dhook);
 
-	xfarray_destroy(xnc->nlinks);
+	if (xnc->nlinks)
+		xfarray_destroy(xnc->nlinks);
 	xnc->nlinks = NULL;
 
 	xchk_iscan_teardown(&xnc->collect_iscan);

[PATCH 3/4] xfs: check return value of xchk_scrub_create_subord

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Fix this function to return NULL instead of a mangled ENOMEM, then fix
the callers to actually check for a null pointer and return ENOMEM.
Most of the corrections here are for code merged between 6.2 and 6.10.

Cc: r772577952@gmail.com
Cc: <stable@vger.kernel.org> # v6.12
Fixes: 1a5f6e08d4e379 ("xfs: create subordinate scrub contexts for xchk_metadata_inode_subtype")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/common.c |    3 +++
 fs/xfs/scrub/repair.c |    3 +++
 fs/xfs/scrub/scrub.c  |    2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index 7bfa37c99480f0..257aefd33d04ff 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -1395,6 +1395,9 @@ xchk_metadata_inode_subtype(
 	int			error;
 
 	sub = xchk_scrub_create_subord(sc, scrub_type);
+	if (!sub)
+		return -ENOMEM;
+
 	error = sub->sc.ops->scrub(&sub->sc);
 	xchk_scrub_free_subord(sub);
 	return error;
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index efd5a7ccdf624a..4d45d39e67f11e 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -1136,6 +1136,9 @@ xrep_metadata_inode_subtype(
 	 * setup/teardown routines.
 	 */
 	sub = xchk_scrub_create_subord(sc, scrub_type);
+	if (!sub)
+		return -ENOMEM;
+
 	error = sub->sc.ops->scrub(&sub->sc);
 	if (error)
 		goto out;
diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
index 3c3b0d25006ff4..c312f0a672e65f 100644
--- a/fs/xfs/scrub/scrub.c
+++ b/fs/xfs/scrub/scrub.c
@@ -634,7 +634,7 @@ xchk_scrub_create_subord(
 
 	sub = kzalloc(sizeof(*sub), XCHK_GFP_FLAGS);
 	if (!sub)
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 
 	sub->old_smtype = sc->sm->sm_type;
 	sub->old_smflags = sc->sm->sm_flags;

[PATCH 4/4] xfs: fix UAF in xchk_btree_check_block_owner

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

We cannot dereference bs->cur when trying to determine if bs->cur
aliases bs->sc->sa.{bno,rmap}_cur after the latter has been freed.
Fix this by sampling before type before any freeing could happen.
The correct temporal ordering was broken when we removed xfs_btnum_t.

Cc: r772577952@gmail.com
Cc: <stable@vger.kernel.org> # v6.9
Fixes: ec793e690f801d ("xfs: remove xfs_btnum_t")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/btree.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
index cd6f0ff382a7c8..acade92c5fce1a 100644
--- a/fs/xfs/scrub/btree.c
+++ b/fs/xfs/scrub/btree.c
@@ -370,12 +370,15 @@ xchk_btree_check_block_owner(
 {
 	xfs_agnumber_t		agno;
 	xfs_agblock_t		agbno;
+	bool			is_bnobt, is_rmapbt;
 	bool			init_sa;
 	int			error = 0;
 
 	if (!bs->cur)
 		return 0;
 
+	is_bnobt = xfs_btree_is_bno(bs->cur->bc_ops);
+	is_rmapbt = xfs_btree_is_rmap(bs->cur->bc_ops);
 	agno = xfs_daddr_to_agno(bs->cur->bc_mp, daddr);
 	agbno = xfs_daddr_to_agbno(bs->cur->bc_mp, daddr);
 
@@ -398,11 +401,11 @@ xchk_btree_check_block_owner(
 	 * have to nullify it (to shut down further block owner checks) if
 	 * self-xref encounters problems.
 	 */
-	if (!bs->sc->sa.bno_cur && xfs_btree_is_bno(bs->cur->bc_ops))
+	if (!bs->sc->sa.bno_cur && is_bnobt)
 		bs->cur = NULL;
 
 	xchk_xref_is_only_owned_by(bs->sc, agbno, 1, bs->oinfo);
-	if (!bs->sc->sa.rmap_cur && xfs_btree_is_rmap(bs->cur->bc_ops))
+	if (!bs->sc->sa.rmap_cur && is_rmapbt)
 		bs->cur = NULL;
 
 out_free:

Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls

[Christoph Hellwig]

On Tue, Jan 20, 2026 at 10:40:23PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> The xchk_xfile_*_descr macros call kasprintf, which can fail to allocate
> memory if the formatted string is larger than 16 bytes (or whatever the
> nofail guarantees are nowadays).  Some of them could easily exceed that,
> so let's just add return value checking across the board.  Note that
> this patch touches a number of commits, most of which were merged
> between 6.6 and 6.14.

Hmm.  I think this goes back to a discussion we had before, and I fear I
forgot your answer:

xchk_xfile_*_descr is used to pass the name to xfarray_create or
xfblob_create.  I still think it would make this a lot more robust if
those took a format string and varags, and then we'd have wrappers for
the common types.  Even if that still ends up doing kasprintf underneath,
that would be isolated to the low-level functions that only need to
implement error handling and freeing once.

> 
> Cc: r772577952@gmail.com
> Cc: <stable@vger.kernel.org> # v6.12
> Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  fs/xfs/scrub/agheader_repair.c   |    6 ++++++
>  fs/xfs/scrub/alloc_repair.c      |    5 +++++
>  fs/xfs/scrub/attr_repair.c       |   20 ++++++++++++++++++++
>  fs/xfs/scrub/bmap_repair.c       |    5 +++++
>  fs/xfs/scrub/dir.c               |   10 ++++++++++
>  fs/xfs/scrub/dir_repair.c        |    8 ++++++++
>  fs/xfs/scrub/dirtree.c           |   10 ++++++++++
>  fs/xfs/scrub/ialloc_repair.c     |    5 +++++
>  fs/xfs/scrub/nlinks.c            |    5 +++++
>  fs/xfs/scrub/parent.c            |    8 ++++++++
>  fs/xfs/scrub/parent_repair.c     |   20 ++++++++++++++++++++
>  fs/xfs/scrub/quotacheck.c        |   15 +++++++++++++++
>  fs/xfs/scrub/refcount_repair.c   |    8 ++++++++
>  fs/xfs/scrub/rmap_repair.c       |    3 +++
>  fs/xfs/scrub/rtbitmap_repair.c   |    3 +++
>  fs/xfs/scrub/rtrefcount_repair.c |    8 ++++++++
>  fs/xfs/scrub/rtrmap_repair.c     |    3 +++
>  fs/xfs/scrub/rtsummary.c         |    3 +++
>  18 files changed, 145 insertions(+)
> 
> 
> diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> index cd6f0223879f49..8d7762cf5daffd 100644
> --- a/fs/xfs/scrub/agheader_repair.c
> +++ b/fs/xfs/scrub/agheader_repair.c
> @@ -1743,6 +1743,9 @@ xrep_agi(
>  	sc->buf_cleanup = xrep_agi_buf_cleanup;
>  
>  	descr = xchk_xfile_ag_descr(sc, "iunlinked next pointers");
> +	if (!descr)
> +		return -ENOMEM;
> +
>  	error = xfarray_create(descr, 0, sizeof(xfs_agino_t),
>  			&ragi->iunlink_next);
>  	kfree(descr);
> @@ -1750,6 +1753,9 @@ xrep_agi(
>  		return error;
>  
>  	descr = xchk_xfile_ag_descr(sc, "iunlinked prev pointers");
> +	if (!descr)
> +		return -ENOMEM;
> +
>  	error = xfarray_create(descr, 0, sizeof(xfs_agino_t),
>  			&ragi->iunlink_prev);
>  	kfree(descr);
> diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
> index bed6a09aa79112..2e1d62efba72a7 100644
> --- a/fs/xfs/scrub/alloc_repair.c
> +++ b/fs/xfs/scrub/alloc_repair.c
> @@ -877,6 +877,11 @@ xrep_allocbt(
>  
>  	/* Set up enough storage to handle maximally fragmented free space. */
>  	descr = xchk_xfile_ag_descr(sc, "free space records");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_ra;
> +	}
> +
>  	error = xfarray_create(descr, mp->m_sb.sb_agblocks / 2,
>  			sizeof(struct xfs_alloc_rec_incore),
>  			&ra->free_records);
> diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c
> index c7eb94069cafcd..73684ce9b81bc5 100644
> --- a/fs/xfs/scrub/attr_repair.c
> +++ b/fs/xfs/scrub/attr_repair.c
> @@ -1556,6 +1556,11 @@ xrep_xattr_setup_scan(
>  
>  	/* Set up some staging for salvaged attribute keys and values */
>  	descr = xchk_xfile_ino_descr(sc, "xattr keys");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_rx;
> +	}
> +
>  	error = xfarray_create(descr, 0, sizeof(struct xrep_xattr_key),
>  			&rx->xattr_records);
>  	kfree(descr);
> @@ -1563,6 +1568,11 @@ xrep_xattr_setup_scan(
>  		goto out_rx;
>  
>  	descr = xchk_xfile_ino_descr(sc, "xattr names");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_keys;
> +	}
> +
>  	error = xfblob_create(descr, &rx->xattr_blobs);
>  	kfree(descr);
>  	if (error)
> @@ -1573,6 +1583,11 @@ xrep_xattr_setup_scan(
>  
>  		descr = xchk_xfile_ino_descr(sc,
>  				"xattr retained parent pointer entries");
> +		if (!descr) {
> +			error = -ENOMEM;
> +			goto out_values;
> +		}
> +
>  		error = xfarray_create(descr, 0,
>  				sizeof(struct xrep_xattr_pptr),
>  				&rx->pptr_recs);
> @@ -1582,6 +1597,11 @@ xrep_xattr_setup_scan(
>  
>  		descr = xchk_xfile_ino_descr(sc,
>  				"xattr retained parent pointer names");
> +		if (!descr) {
> +			error = -ENOMEM;
> +			goto out_pprecs;
> +		}
> +
>  		error = xfblob_create(descr, &rx->pptr_names);
>  		kfree(descr);
>  		if (error)
> diff --git a/fs/xfs/scrub/bmap_repair.c b/fs/xfs/scrub/bmap_repair.c
> index 1084213b8e9b88..74df05142dcf4c 100644
> --- a/fs/xfs/scrub/bmap_repair.c
> +++ b/fs/xfs/scrub/bmap_repair.c
> @@ -947,6 +947,11 @@ xrep_bmap(
>  	max_bmbt_recs = xfs_iext_max_nextents(large_extcount, whichfork);
>  	descr = xchk_xfile_ino_descr(sc, "%s fork mapping records",
>  			whichfork == XFS_DATA_FORK ? "data" : "attr");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_rb;
> +	}
> +
>  	error = xfarray_create(descr, max_bmbt_recs,
>  			sizeof(struct xfs_bmbt_rec), &rb->bmap_records);
>  	kfree(descr);
> diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
> index c877bde71e6280..58346d54042b07 100644
> --- a/fs/xfs/scrub/dir.c
> +++ b/fs/xfs/scrub/dir.c
> @@ -1109,6 +1109,11 @@ xchk_directory(
>  		 * due to locking contention.
>  		 */
>  		descr = xchk_xfile_ino_descr(sc, "slow directory entries");
> +		if (!descr) {
> +			error = -ENOMEM;
> +			goto out_sd;
> +		}
> +
>  		error = xfarray_create(descr, 0, sizeof(struct xchk_dirent),
>  				&sd->dir_entries);
>  		kfree(descr);
> @@ -1116,6 +1121,11 @@ xchk_directory(
>  			goto out_sd;
>  
>  		descr = xchk_xfile_ino_descr(sc, "slow directory entry names");
> +		if (!descr) {
> +			error = -ENOMEM;
> +			goto out_entries;
> +		}
> +
>  		error = xfblob_create(descr, &sd->dir_names);
>  		kfree(descr);
>  		if (error)
> diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
> index 8d3b550990b58a..50e0af4bdaa63a 100644
> --- a/fs/xfs/scrub/dir_repair.c
> +++ b/fs/xfs/scrub/dir_repair.c
> @@ -1789,6 +1789,9 @@ xrep_dir_setup_scan(
>  
>  	/* Set up some staging memory for salvaging dirents. */
>  	descr = xchk_xfile_ino_descr(sc, "directory entries");
> +	if (!descr)
> +		return -ENOMEM;
> +
>  	error = xfarray_create(descr, 0, sizeof(struct xrep_dirent),
>  			&rd->dir_entries);
>  	kfree(descr);
> @@ -1796,6 +1799,11 @@ xrep_dir_setup_scan(
>  		return error;
>  
>  	descr = xchk_xfile_ino_descr(sc, "directory entry names");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_xfarray;
> +	}
> +
>  	error = xfblob_create(descr, &rd->dir_names);
>  	kfree(descr);
>  	if (error)
> diff --git a/fs/xfs/scrub/dirtree.c b/fs/xfs/scrub/dirtree.c
> index 3a9cdf8738b6db..7f8ad41e3ec20e 100644
> --- a/fs/xfs/scrub/dirtree.c
> +++ b/fs/xfs/scrub/dirtree.c
> @@ -117,6 +117,11 @@ xchk_setup_dirtree(
>  	mutex_init(&dl->lock);
>  
>  	descr = xchk_xfile_ino_descr(sc, "dirtree path steps");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_dl;
> +	}
> +
>  	error = xfarray_create(descr, 0, sizeof(struct xchk_dirpath_step),
>  			&dl->path_steps);
>  	kfree(descr);
> @@ -124,6 +129,11 @@ xchk_setup_dirtree(
>  		goto out_dl;
>  
>  	descr = xchk_xfile_ino_descr(sc, "dirtree path names");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_steps;
> +	}
> +
>  	error = xfblob_create(descr, &dl->path_names);
>  	kfree(descr);
>  	if (error)
> diff --git a/fs/xfs/scrub/ialloc_repair.c b/fs/xfs/scrub/ialloc_repair.c
> index 14e48d3f1912bf..3055380cf29271 100644
> --- a/fs/xfs/scrub/ialloc_repair.c
> +++ b/fs/xfs/scrub/ialloc_repair.c
> @@ -817,6 +817,11 @@ xrep_iallocbt(
>  	xfs_agino_range(mp, pag_agno(sc->sa.pag), &first_agino, &last_agino);
>  	last_agino /= XFS_INODES_PER_CHUNK;
>  	descr = xchk_xfile_ag_descr(sc, "inode index records");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_ri;
> +	}
> +
>  	error = xfarray_create(descr, last_agino,
>  			sizeof(struct xfs_inobt_rec_incore),
>  			&ri->inode_records);
> diff --git a/fs/xfs/scrub/nlinks.c b/fs/xfs/scrub/nlinks.c
> index 091c79e432e592..c71b065ccb4c45 100644
> --- a/fs/xfs/scrub/nlinks.c
> +++ b/fs/xfs/scrub/nlinks.c
> @@ -1008,6 +1008,11 @@ xchk_nlinks_setup_scan(
>  	xfs_agino_range(mp, last_agno, &first_agino, &last_agino);
>  	max_inos = XFS_AGINO_TO_INO(mp, last_agno, last_agino) + 1;
>  	descr = xchk_xfile_descr(sc, "file link counts");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_teardown;
> +	}
> +
>  	error = xfarray_create(descr, min(XFS_MAXINUMBER + 1, max_inos),
>  			sizeof(struct xchk_nlink), &xnc->nlinks);
>  	kfree(descr);
> diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c
> index 11d5de10fd567b..11c70e5d3e03de 100644
> --- a/fs/xfs/scrub/parent.c
> +++ b/fs/xfs/scrub/parent.c
> @@ -769,6 +769,9 @@ xchk_parent_pptr(
>  	 * due to locking contention.
>  	 */
>  	descr = xchk_xfile_ino_descr(sc, "slow parent pointer entries");
> +	if (!descr)
> +		return -ENOMEM;
> +
>  	error = xfarray_create(descr, 0, sizeof(struct xchk_pptr),
>  			&pp->pptr_entries);
>  	kfree(descr);
> @@ -776,6 +779,11 @@ xchk_parent_pptr(
>  		goto out_pp;
>  
>  	descr = xchk_xfile_ino_descr(sc, "slow parent pointer names");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_entries;
> +	}
> +
>  	error = xfblob_create(descr, &pp->pptr_names);
>  	kfree(descr);
>  	if (error)
> diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c
> index 2949feda627175..8683317f2342df 100644
> --- a/fs/xfs/scrub/parent_repair.c
> +++ b/fs/xfs/scrub/parent_repair.c
> @@ -1526,6 +1526,11 @@ xrep_parent_setup_scan(
>  
>  	/* Set up some staging memory for logging parent pointer updates. */
>  	descr = xchk_xfile_ino_descr(sc, "parent pointer entries");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_xattr_value;
> +	}
> +
>  	error = xfarray_create(descr, 0, sizeof(struct xrep_pptr),
>  			&rp->pptr_recs);
>  	kfree(descr);
> @@ -1533,6 +1538,11 @@ xrep_parent_setup_scan(
>  		goto out_xattr_value;
>  
>  	descr = xchk_xfile_ino_descr(sc, "parent pointer names");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_recs;
> +	}
> +
>  	error = xfblob_create(descr, &rp->pptr_names);
>  	kfree(descr);
>  	if (error)
> @@ -1541,6 +1551,11 @@ xrep_parent_setup_scan(
>  	/* Set up some storage for copying attrs before the mapping exchange */
>  	descr = xchk_xfile_ino_descr(sc,
>  				"parent pointer retained xattr entries");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_names;
> +	}
> +
>  	error = xfarray_create(descr, 0, sizeof(struct xrep_parent_xattr),
>  			&rp->xattr_records);
>  	kfree(descr);
> @@ -1549,6 +1564,11 @@ xrep_parent_setup_scan(
>  
>  	descr = xchk_xfile_ino_descr(sc,
>  				"parent pointer retained xattr values");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_attr_keys;
> +	}
> +
>  	error = xfblob_create(descr, &rp->xattr_blobs);
>  	kfree(descr);
>  	if (error)
> diff --git a/fs/xfs/scrub/quotacheck.c b/fs/xfs/scrub/quotacheck.c
> index d412a8359784ee..7d0ad19ddf577d 100644
> --- a/fs/xfs/scrub/quotacheck.c
> +++ b/fs/xfs/scrub/quotacheck.c
> @@ -757,6 +757,11 @@ xqcheck_setup_scan(
>  	error = -ENOMEM;
>  	if (xfs_this_quota_on(sc->mp, XFS_DQTYPE_USER)) {
>  		descr = xchk_xfile_descr(sc, "user dquot records");
> +		if (!descr) {
> +			error = -ENOMEM;
> +			goto out_teardown;
> +		}
> +
>  		error = xfarray_create(descr, max_dquots,
>  				sizeof(struct xqcheck_dquot), &xqc->ucounts);
>  		kfree(descr);
> @@ -766,6 +771,11 @@ xqcheck_setup_scan(
>  
>  	if (xfs_this_quota_on(sc->mp, XFS_DQTYPE_GROUP)) {
>  		descr = xchk_xfile_descr(sc, "group dquot records");
> +		if (!descr) {
> +			error = -ENOMEM;
> +			goto out_teardown;
> +		}
> +
>  		error = xfarray_create(descr, max_dquots,
>  				sizeof(struct xqcheck_dquot), &xqc->gcounts);
>  		kfree(descr);
> @@ -775,6 +785,11 @@ xqcheck_setup_scan(
>  
>  	if (xfs_this_quota_on(sc->mp, XFS_DQTYPE_PROJ)) {
>  		descr = xchk_xfile_descr(sc, "project dquot records");
> +		if (!descr) {
> +			error = -ENOMEM;
> +			goto out_teardown;
> +		}
> +
>  		error = xfarray_create(descr, max_dquots,
>  				sizeof(struct xqcheck_dquot), &xqc->pcounts);
>  		kfree(descr);
> diff --git a/fs/xfs/scrub/refcount_repair.c b/fs/xfs/scrub/refcount_repair.c
> index 9c8cb5332da042..d53c9a5bb7809c 100644
> --- a/fs/xfs/scrub/refcount_repair.c
> +++ b/fs/xfs/scrub/refcount_repair.c
> @@ -127,6 +127,9 @@ xrep_setup_ag_refcountbt(
>  	int			error;
>  
>  	descr = xchk_xfile_ag_descr(sc, "rmap record bag");
> +	if (!descr)
> +		return -ENOMEM;
> +
>  	error = xrep_setup_xfbtree(sc, descr);
>  	kfree(descr);
>  	return error;
> @@ -718,6 +721,11 @@ xrep_refcountbt(
>  
>  	/* Set up enough storage to handle one refcount record per block. */
>  	descr = xchk_xfile_ag_descr(sc, "reference count records");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_rr;
> +	}
> +
>  	error = xfarray_create(descr, mp->m_sb.sb_agblocks,
>  			sizeof(struct xfs_refcount_irec),
>  			&rr->refcount_records);
> diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
> index 17d4a38d735cb8..c619ba469e36de 100644
> --- a/fs/xfs/scrub/rmap_repair.c
> +++ b/fs/xfs/scrub/rmap_repair.c
> @@ -170,6 +170,9 @@ xrep_setup_ag_rmapbt(
>  	xchk_fsgates_enable(sc, XCHK_FSGATES_RMAP);
>  
>  	descr = xchk_xfile_ag_descr(sc, "reverse mapping records");
> +	if (!descr)
> +		return -ENOMEM;
> +
>  	error = xrep_setup_xfbtree(sc, descr);
>  	kfree(descr);
>  	if (error)
> diff --git a/fs/xfs/scrub/rtbitmap_repair.c b/fs/xfs/scrub/rtbitmap_repair.c
> index 203a1a97c5026e..070347df717c46 100644
> --- a/fs/xfs/scrub/rtbitmap_repair.c
> +++ b/fs/xfs/scrub/rtbitmap_repair.c
> @@ -53,6 +53,9 @@ xrep_setup_rtbitmap(
>  
>  	/* Create an xfile to hold our reconstructed bitmap. */
>  	descr = xchk_xfile_rtgroup_descr(sc, "bitmap file");
> +	if (!descr)
> +		return -ENOMEM;
> +
>  	error = xfile_create(descr, blocks * mp->m_sb.sb_blocksize, &sc->xfile);
>  	kfree(descr);
>  	if (error)
> diff --git a/fs/xfs/scrub/rtrefcount_repair.c b/fs/xfs/scrub/rtrefcount_repair.c
> index 983362447826de..029e3e332f605e 100644
> --- a/fs/xfs/scrub/rtrefcount_repair.c
> +++ b/fs/xfs/scrub/rtrefcount_repair.c
> @@ -132,6 +132,9 @@ xrep_setup_rtrefcountbt(
>  	int			error;
>  
>  	descr = xchk_xfile_ag_descr(sc, "rmap record bag");
> +	if (!descr)
> +		return -ENOMEM;
> +
>  	error = xrep_setup_xfbtree(sc, descr);
>  	kfree(descr);
>  	return error;
> @@ -723,6 +726,11 @@ xrep_rtrefcountbt(
>  
>  	/* Set up enough storage to handle one refcount record per rt extent. */
>  	descr = xchk_xfile_ag_descr(sc, "reference count records");
> +	if (!descr) {
> +		error = -ENOMEM;
> +		goto out_rr;
> +	}
> +
>  	error = xfarray_create(descr, mp->m_sb.sb_rextents,
>  			sizeof(struct xfs_refcount_irec),
>  			&rr->refcount_records);
> diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
> index 7561941a337a1f..c74d640068d1c8 100644
> --- a/fs/xfs/scrub/rtrmap_repair.c
> +++ b/fs/xfs/scrub/rtrmap_repair.c
> @@ -109,6 +109,9 @@ xrep_setup_rtrmapbt(
>  	xchk_fsgates_enable(sc, XCHK_FSGATES_RMAP);
>  
>  	descr = xchk_xfile_rtgroup_descr(sc, "reverse mapping records");
> +	if (!descr)
> +		return -ENOMEM;
> +
>  	error = xrep_setup_xfbtree(sc, descr);
>  	kfree(descr);
>  	if (error)
> diff --git a/fs/xfs/scrub/rtsummary.c b/fs/xfs/scrub/rtsummary.c
> index 4ac679c1bd29cd..bf2b96e51d070c 100644
> --- a/fs/xfs/scrub/rtsummary.c
> +++ b/fs/xfs/scrub/rtsummary.c
> @@ -71,6 +71,9 @@ xchk_setup_rtsummary(
>  	 * us to avoid pinning kernel memory for this purpose.
>  	 */
>  	descr = xchk_xfile_descr(sc, "realtime summary file");
> +	if (!descr)
> +		return -ENOMEM;
> +
>  	error = xfile_create(descr, XFS_FSB_TO_B(mp, mp->m_rsumblocks),
>  			&sc->xfile);
>  	kfree(descr);
---end quoted text---

Re: [PATCH 3/4] xfs: check return value of xchk_scrub_create_subord

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 4/4] xfs: fix UAF in xchk_btree_check_block_owner

[Christoph Hellwig]

On Tue, Jan 20, 2026 at 10:41:10PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> We cannot dereference bs->cur when trying to determine if bs->cur
> aliases bs->sc->sa.{bno,rmap}_cur after the latter has been freed.
> Fix this by sampling before type before any freeing could happen.
> The correct temporal ordering was broken when we removed xfs_btnum_t.

Looks good:

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

Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls

[Darrick J. Wong]

On Wed, Jan 21, 2026 at 08:03:23AM +0100, Christoph Hellwig wrote:
> On Tue, Jan 20, 2026 at 10:40:23PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > The xchk_xfile_*_descr macros call kasprintf, which can fail to allocate
> > memory if the formatted string is larger than 16 bytes (or whatever the
> > nofail guarantees are nowadays).  Some of them could easily exceed that,
> > so let's just add return value checking across the board.  Note that
> > this patch touches a number of commits, most of which were merged
> > between 6.6 and 6.14.
> 
> Hmm.  I think this goes back to a discussion we had before, and I fear I
> forgot your answer:
> 
> xchk_xfile_*_descr is used to pass the name to xfarray_create or
> xfblob_create.  I still think it would make this a lot more robust if
> those took a format string and varags, and then we'd have wrappers for
> the common types.  Even if that still ends up doing kasprintf underneath,
> that would be isolated to the low-level functions that only need to
> implement error handling and freeing once.

Alternately we just drop all the helpers and kasprintf crap in favor of
feeding the raw string ("iunlinked next pointers") all the way through
to shmem_kernel_file_setup.  That reduces ease of observability but now
there's one less way to fail.  Most people probably aren't going to
ls -la /proc/$xfs_scrub_pid/fd/ and anyone looking at ftrace can figure
out the group/inode/whatever from the other tracepoints.

--D

> > 
> > Cc: r772577952@gmail.com
> > Cc: <stable@vger.kernel.org> # v6.12
> > Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> >  fs/xfs/scrub/agheader_repair.c   |    6 ++++++
> >  fs/xfs/scrub/alloc_repair.c      |    5 +++++
> >  fs/xfs/scrub/attr_repair.c       |   20 ++++++++++++++++++++
> >  fs/xfs/scrub/bmap_repair.c       |    5 +++++
> >  fs/xfs/scrub/dir.c               |   10 ++++++++++
> >  fs/xfs/scrub/dir_repair.c        |    8 ++++++++
> >  fs/xfs/scrub/dirtree.c           |   10 ++++++++++
> >  fs/xfs/scrub/ialloc_repair.c     |    5 +++++
> >  fs/xfs/scrub/nlinks.c            |    5 +++++
> >  fs/xfs/scrub/parent.c            |    8 ++++++++
> >  fs/xfs/scrub/parent_repair.c     |   20 ++++++++++++++++++++
> >  fs/xfs/scrub/quotacheck.c        |   15 +++++++++++++++
> >  fs/xfs/scrub/refcount_repair.c   |    8 ++++++++
> >  fs/xfs/scrub/rmap_repair.c       |    3 +++
> >  fs/xfs/scrub/rtbitmap_repair.c   |    3 +++
> >  fs/xfs/scrub/rtrefcount_repair.c |    8 ++++++++
> >  fs/xfs/scrub/rtrmap_repair.c     |    3 +++
> >  fs/xfs/scrub/rtsummary.c         |    3 +++
> >  18 files changed, 145 insertions(+)
> > 
> > 
> > diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> > index cd6f0223879f49..8d7762cf5daffd 100644
> > --- a/fs/xfs/scrub/agheader_repair.c
> > +++ b/fs/xfs/scrub/agheader_repair.c
> > @@ -1743,6 +1743,9 @@ xrep_agi(
> >  	sc->buf_cleanup = xrep_agi_buf_cleanup;
> >  
> >  	descr = xchk_xfile_ag_descr(sc, "iunlinked next pointers");
> > +	if (!descr)
> > +		return -ENOMEM;
> > +
> >  	error = xfarray_create(descr, 0, sizeof(xfs_agino_t),
> >  			&ragi->iunlink_next);
> >  	kfree(descr);
> > @@ -1750,6 +1753,9 @@ xrep_agi(
> >  		return error;
> >  
> >  	descr = xchk_xfile_ag_descr(sc, "iunlinked prev pointers");
> > +	if (!descr)
> > +		return -ENOMEM;
> > +
> >  	error = xfarray_create(descr, 0, sizeof(xfs_agino_t),
> >  			&ragi->iunlink_prev);
> >  	kfree(descr);
> > diff --git a/fs/xfs/scrub/alloc_repair.c b/fs/xfs/scrub/alloc_repair.c
> > index bed6a09aa79112..2e1d62efba72a7 100644
> > --- a/fs/xfs/scrub/alloc_repair.c
> > +++ b/fs/xfs/scrub/alloc_repair.c
> > @@ -877,6 +877,11 @@ xrep_allocbt(
> >  
> >  	/* Set up enough storage to handle maximally fragmented free space. */
> >  	descr = xchk_xfile_ag_descr(sc, "free space records");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_ra;
> > +	}
> > +
> >  	error = xfarray_create(descr, mp->m_sb.sb_agblocks / 2,
> >  			sizeof(struct xfs_alloc_rec_incore),
> >  			&ra->free_records);
> > diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c
> > index c7eb94069cafcd..73684ce9b81bc5 100644
> > --- a/fs/xfs/scrub/attr_repair.c
> > +++ b/fs/xfs/scrub/attr_repair.c
> > @@ -1556,6 +1556,11 @@ xrep_xattr_setup_scan(
> >  
> >  	/* Set up some staging for salvaged attribute keys and values */
> >  	descr = xchk_xfile_ino_descr(sc, "xattr keys");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_rx;
> > +	}
> > +
> >  	error = xfarray_create(descr, 0, sizeof(struct xrep_xattr_key),
> >  			&rx->xattr_records);
> >  	kfree(descr);
> > @@ -1563,6 +1568,11 @@ xrep_xattr_setup_scan(
> >  		goto out_rx;
> >  
> >  	descr = xchk_xfile_ino_descr(sc, "xattr names");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_keys;
> > +	}
> > +
> >  	error = xfblob_create(descr, &rx->xattr_blobs);
> >  	kfree(descr);
> >  	if (error)
> > @@ -1573,6 +1583,11 @@ xrep_xattr_setup_scan(
> >  
> >  		descr = xchk_xfile_ino_descr(sc,
> >  				"xattr retained parent pointer entries");
> > +		if (!descr) {
> > +			error = -ENOMEM;
> > +			goto out_values;
> > +		}
> > +
> >  		error = xfarray_create(descr, 0,
> >  				sizeof(struct xrep_xattr_pptr),
> >  				&rx->pptr_recs);
> > @@ -1582,6 +1597,11 @@ xrep_xattr_setup_scan(
> >  
> >  		descr = xchk_xfile_ino_descr(sc,
> >  				"xattr retained parent pointer names");
> > +		if (!descr) {
> > +			error = -ENOMEM;
> > +			goto out_pprecs;
> > +		}
> > +
> >  		error = xfblob_create(descr, &rx->pptr_names);
> >  		kfree(descr);
> >  		if (error)
> > diff --git a/fs/xfs/scrub/bmap_repair.c b/fs/xfs/scrub/bmap_repair.c
> > index 1084213b8e9b88..74df05142dcf4c 100644
> > --- a/fs/xfs/scrub/bmap_repair.c
> > +++ b/fs/xfs/scrub/bmap_repair.c
> > @@ -947,6 +947,11 @@ xrep_bmap(
> >  	max_bmbt_recs = xfs_iext_max_nextents(large_extcount, whichfork);
> >  	descr = xchk_xfile_ino_descr(sc, "%s fork mapping records",
> >  			whichfork == XFS_DATA_FORK ? "data" : "attr");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_rb;
> > +	}
> > +
> >  	error = xfarray_create(descr, max_bmbt_recs,
> >  			sizeof(struct xfs_bmbt_rec), &rb->bmap_records);
> >  	kfree(descr);
> > diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
> > index c877bde71e6280..58346d54042b07 100644
> > --- a/fs/xfs/scrub/dir.c
> > +++ b/fs/xfs/scrub/dir.c
> > @@ -1109,6 +1109,11 @@ xchk_directory(
> >  		 * due to locking contention.
> >  		 */
> >  		descr = xchk_xfile_ino_descr(sc, "slow directory entries");
> > +		if (!descr) {
> > +			error = -ENOMEM;
> > +			goto out_sd;
> > +		}
> > +
> >  		error = xfarray_create(descr, 0, sizeof(struct xchk_dirent),
> >  				&sd->dir_entries);
> >  		kfree(descr);
> > @@ -1116,6 +1121,11 @@ xchk_directory(
> >  			goto out_sd;
> >  
> >  		descr = xchk_xfile_ino_descr(sc, "slow directory entry names");
> > +		if (!descr) {
> > +			error = -ENOMEM;
> > +			goto out_entries;
> > +		}
> > +
> >  		error = xfblob_create(descr, &sd->dir_names);
> >  		kfree(descr);
> >  		if (error)
> > diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
> > index 8d3b550990b58a..50e0af4bdaa63a 100644
> > --- a/fs/xfs/scrub/dir_repair.c
> > +++ b/fs/xfs/scrub/dir_repair.c
> > @@ -1789,6 +1789,9 @@ xrep_dir_setup_scan(
> >  
> >  	/* Set up some staging memory for salvaging dirents. */
> >  	descr = xchk_xfile_ino_descr(sc, "directory entries");
> > +	if (!descr)
> > +		return -ENOMEM;
> > +
> >  	error = xfarray_create(descr, 0, sizeof(struct xrep_dirent),
> >  			&rd->dir_entries);
> >  	kfree(descr);
> > @@ -1796,6 +1799,11 @@ xrep_dir_setup_scan(
> >  		return error;
> >  
> >  	descr = xchk_xfile_ino_descr(sc, "directory entry names");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_xfarray;
> > +	}
> > +
> >  	error = xfblob_create(descr, &rd->dir_names);
> >  	kfree(descr);
> >  	if (error)
> > diff --git a/fs/xfs/scrub/dirtree.c b/fs/xfs/scrub/dirtree.c
> > index 3a9cdf8738b6db..7f8ad41e3ec20e 100644
> > --- a/fs/xfs/scrub/dirtree.c
> > +++ b/fs/xfs/scrub/dirtree.c
> > @@ -117,6 +117,11 @@ xchk_setup_dirtree(
> >  	mutex_init(&dl->lock);
> >  
> >  	descr = xchk_xfile_ino_descr(sc, "dirtree path steps");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_dl;
> > +	}
> > +
> >  	error = xfarray_create(descr, 0, sizeof(struct xchk_dirpath_step),
> >  			&dl->path_steps);
> >  	kfree(descr);
> > @@ -124,6 +129,11 @@ xchk_setup_dirtree(
> >  		goto out_dl;
> >  
> >  	descr = xchk_xfile_ino_descr(sc, "dirtree path names");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_steps;
> > +	}
> > +
> >  	error = xfblob_create(descr, &dl->path_names);
> >  	kfree(descr);
> >  	if (error)
> > diff --git a/fs/xfs/scrub/ialloc_repair.c b/fs/xfs/scrub/ialloc_repair.c
> > index 14e48d3f1912bf..3055380cf29271 100644
> > --- a/fs/xfs/scrub/ialloc_repair.c
> > +++ b/fs/xfs/scrub/ialloc_repair.c
> > @@ -817,6 +817,11 @@ xrep_iallocbt(
> >  	xfs_agino_range(mp, pag_agno(sc->sa.pag), &first_agino, &last_agino);
> >  	last_agino /= XFS_INODES_PER_CHUNK;
> >  	descr = xchk_xfile_ag_descr(sc, "inode index records");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_ri;
> > +	}
> > +
> >  	error = xfarray_create(descr, last_agino,
> >  			sizeof(struct xfs_inobt_rec_incore),
> >  			&ri->inode_records);
> > diff --git a/fs/xfs/scrub/nlinks.c b/fs/xfs/scrub/nlinks.c
> > index 091c79e432e592..c71b065ccb4c45 100644
> > --- a/fs/xfs/scrub/nlinks.c
> > +++ b/fs/xfs/scrub/nlinks.c
> > @@ -1008,6 +1008,11 @@ xchk_nlinks_setup_scan(
> >  	xfs_agino_range(mp, last_agno, &first_agino, &last_agino);
> >  	max_inos = XFS_AGINO_TO_INO(mp, last_agno, last_agino) + 1;
> >  	descr = xchk_xfile_descr(sc, "file link counts");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_teardown;
> > +	}
> > +
> >  	error = xfarray_create(descr, min(XFS_MAXINUMBER + 1, max_inos),
> >  			sizeof(struct xchk_nlink), &xnc->nlinks);
> >  	kfree(descr);
> > diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c
> > index 11d5de10fd567b..11c70e5d3e03de 100644
> > --- a/fs/xfs/scrub/parent.c
> > +++ b/fs/xfs/scrub/parent.c
> > @@ -769,6 +769,9 @@ xchk_parent_pptr(
> >  	 * due to locking contention.
> >  	 */
> >  	descr = xchk_xfile_ino_descr(sc, "slow parent pointer entries");
> > +	if (!descr)
> > +		return -ENOMEM;
> > +
> >  	error = xfarray_create(descr, 0, sizeof(struct xchk_pptr),
> >  			&pp->pptr_entries);
> >  	kfree(descr);
> > @@ -776,6 +779,11 @@ xchk_parent_pptr(
> >  		goto out_pp;
> >  
> >  	descr = xchk_xfile_ino_descr(sc, "slow parent pointer names");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_entries;
> > +	}
> > +
> >  	error = xfblob_create(descr, &pp->pptr_names);
> >  	kfree(descr);
> >  	if (error)
> > diff --git a/fs/xfs/scrub/parent_repair.c b/fs/xfs/scrub/parent_repair.c
> > index 2949feda627175..8683317f2342df 100644
> > --- a/fs/xfs/scrub/parent_repair.c
> > +++ b/fs/xfs/scrub/parent_repair.c
> > @@ -1526,6 +1526,11 @@ xrep_parent_setup_scan(
> >  
> >  	/* Set up some staging memory for logging parent pointer updates. */
> >  	descr = xchk_xfile_ino_descr(sc, "parent pointer entries");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_xattr_value;
> > +	}
> > +
> >  	error = xfarray_create(descr, 0, sizeof(struct xrep_pptr),
> >  			&rp->pptr_recs);
> >  	kfree(descr);
> > @@ -1533,6 +1538,11 @@ xrep_parent_setup_scan(
> >  		goto out_xattr_value;
> >  
> >  	descr = xchk_xfile_ino_descr(sc, "parent pointer names");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_recs;
> > +	}
> > +
> >  	error = xfblob_create(descr, &rp->pptr_names);
> >  	kfree(descr);
> >  	if (error)
> > @@ -1541,6 +1551,11 @@ xrep_parent_setup_scan(
> >  	/* Set up some storage for copying attrs before the mapping exchange */
> >  	descr = xchk_xfile_ino_descr(sc,
> >  				"parent pointer retained xattr entries");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_names;
> > +	}
> > +
> >  	error = xfarray_create(descr, 0, sizeof(struct xrep_parent_xattr),
> >  			&rp->xattr_records);
> >  	kfree(descr);
> > @@ -1549,6 +1564,11 @@ xrep_parent_setup_scan(
> >  
> >  	descr = xchk_xfile_ino_descr(sc,
> >  				"parent pointer retained xattr values");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_attr_keys;
> > +	}
> > +
> >  	error = xfblob_create(descr, &rp->xattr_blobs);
> >  	kfree(descr);
> >  	if (error)
> > diff --git a/fs/xfs/scrub/quotacheck.c b/fs/xfs/scrub/quotacheck.c
> > index d412a8359784ee..7d0ad19ddf577d 100644
> > --- a/fs/xfs/scrub/quotacheck.c
> > +++ b/fs/xfs/scrub/quotacheck.c
> > @@ -757,6 +757,11 @@ xqcheck_setup_scan(
> >  	error = -ENOMEM;
> >  	if (xfs_this_quota_on(sc->mp, XFS_DQTYPE_USER)) {
> >  		descr = xchk_xfile_descr(sc, "user dquot records");
> > +		if (!descr) {
> > +			error = -ENOMEM;
> > +			goto out_teardown;
> > +		}
> > +
> >  		error = xfarray_create(descr, max_dquots,
> >  				sizeof(struct xqcheck_dquot), &xqc->ucounts);
> >  		kfree(descr);
> > @@ -766,6 +771,11 @@ xqcheck_setup_scan(
> >  
> >  	if (xfs_this_quota_on(sc->mp, XFS_DQTYPE_GROUP)) {
> >  		descr = xchk_xfile_descr(sc, "group dquot records");
> > +		if (!descr) {
> > +			error = -ENOMEM;
> > +			goto out_teardown;
> > +		}
> > +
> >  		error = xfarray_create(descr, max_dquots,
> >  				sizeof(struct xqcheck_dquot), &xqc->gcounts);
> >  		kfree(descr);
> > @@ -775,6 +785,11 @@ xqcheck_setup_scan(
> >  
> >  	if (xfs_this_quota_on(sc->mp, XFS_DQTYPE_PROJ)) {
> >  		descr = xchk_xfile_descr(sc, "project dquot records");
> > +		if (!descr) {
> > +			error = -ENOMEM;
> > +			goto out_teardown;
> > +		}
> > +
> >  		error = xfarray_create(descr, max_dquots,
> >  				sizeof(struct xqcheck_dquot), &xqc->pcounts);
> >  		kfree(descr);
> > diff --git a/fs/xfs/scrub/refcount_repair.c b/fs/xfs/scrub/refcount_repair.c
> > index 9c8cb5332da042..d53c9a5bb7809c 100644
> > --- a/fs/xfs/scrub/refcount_repair.c
> > +++ b/fs/xfs/scrub/refcount_repair.c
> > @@ -127,6 +127,9 @@ xrep_setup_ag_refcountbt(
> >  	int			error;
> >  
> >  	descr = xchk_xfile_ag_descr(sc, "rmap record bag");
> > +	if (!descr)
> > +		return -ENOMEM;
> > +
> >  	error = xrep_setup_xfbtree(sc, descr);
> >  	kfree(descr);
> >  	return error;
> > @@ -718,6 +721,11 @@ xrep_refcountbt(
> >  
> >  	/* Set up enough storage to handle one refcount record per block. */
> >  	descr = xchk_xfile_ag_descr(sc, "reference count records");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_rr;
> > +	}
> > +
> >  	error = xfarray_create(descr, mp->m_sb.sb_agblocks,
> >  			sizeof(struct xfs_refcount_irec),
> >  			&rr->refcount_records);
> > diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
> > index 17d4a38d735cb8..c619ba469e36de 100644
> > --- a/fs/xfs/scrub/rmap_repair.c
> > +++ b/fs/xfs/scrub/rmap_repair.c
> > @@ -170,6 +170,9 @@ xrep_setup_ag_rmapbt(
> >  	xchk_fsgates_enable(sc, XCHK_FSGATES_RMAP);
> >  
> >  	descr = xchk_xfile_ag_descr(sc, "reverse mapping records");
> > +	if (!descr)
> > +		return -ENOMEM;
> > +
> >  	error = xrep_setup_xfbtree(sc, descr);
> >  	kfree(descr);
> >  	if (error)
> > diff --git a/fs/xfs/scrub/rtbitmap_repair.c b/fs/xfs/scrub/rtbitmap_repair.c
> > index 203a1a97c5026e..070347df717c46 100644
> > --- a/fs/xfs/scrub/rtbitmap_repair.c
> > +++ b/fs/xfs/scrub/rtbitmap_repair.c
> > @@ -53,6 +53,9 @@ xrep_setup_rtbitmap(
> >  
> >  	/* Create an xfile to hold our reconstructed bitmap. */
> >  	descr = xchk_xfile_rtgroup_descr(sc, "bitmap file");
> > +	if (!descr)
> > +		return -ENOMEM;
> > +
> >  	error = xfile_create(descr, blocks * mp->m_sb.sb_blocksize, &sc->xfile);
> >  	kfree(descr);
> >  	if (error)
> > diff --git a/fs/xfs/scrub/rtrefcount_repair.c b/fs/xfs/scrub/rtrefcount_repair.c
> > index 983362447826de..029e3e332f605e 100644
> > --- a/fs/xfs/scrub/rtrefcount_repair.c
> > +++ b/fs/xfs/scrub/rtrefcount_repair.c
> > @@ -132,6 +132,9 @@ xrep_setup_rtrefcountbt(
> >  	int			error;
> >  
> >  	descr = xchk_xfile_ag_descr(sc, "rmap record bag");
> > +	if (!descr)
> > +		return -ENOMEM;
> > +
> >  	error = xrep_setup_xfbtree(sc, descr);
> >  	kfree(descr);
> >  	return error;
> > @@ -723,6 +726,11 @@ xrep_rtrefcountbt(
> >  
> >  	/* Set up enough storage to handle one refcount record per rt extent. */
> >  	descr = xchk_xfile_ag_descr(sc, "reference count records");
> > +	if (!descr) {
> > +		error = -ENOMEM;
> > +		goto out_rr;
> > +	}
> > +
> >  	error = xfarray_create(descr, mp->m_sb.sb_rextents,
> >  			sizeof(struct xfs_refcount_irec),
> >  			&rr->refcount_records);
> > diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
> > index 7561941a337a1f..c74d640068d1c8 100644
> > --- a/fs/xfs/scrub/rtrmap_repair.c
> > +++ b/fs/xfs/scrub/rtrmap_repair.c
> > @@ -109,6 +109,9 @@ xrep_setup_rtrmapbt(
> >  	xchk_fsgates_enable(sc, XCHK_FSGATES_RMAP);
> >  
> >  	descr = xchk_xfile_rtgroup_descr(sc, "reverse mapping records");
> > +	if (!descr)
> > +		return -ENOMEM;
> > +
> >  	error = xrep_setup_xfbtree(sc, descr);
> >  	kfree(descr);
> >  	if (error)
> > diff --git a/fs/xfs/scrub/rtsummary.c b/fs/xfs/scrub/rtsummary.c
> > index 4ac679c1bd29cd..bf2b96e51d070c 100644
> > --- a/fs/xfs/scrub/rtsummary.c
> > +++ b/fs/xfs/scrub/rtsummary.c
> > @@ -71,6 +71,9 @@ xchk_setup_rtsummary(
> >  	 * us to avoid pinning kernel memory for this purpose.
> >  	 */
> >  	descr = xchk_xfile_descr(sc, "realtime summary file");
> > +	if (!descr)
> > +		return -ENOMEM;
> > +
> >  	error = xfile_create(descr, XFS_FSB_TO_B(mp, mp->m_rsumblocks),
> >  			&sc->xfile);
> >  	kfree(descr);
> ---end quoted text---
> 

Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls

[Christoph Hellwig]

On Wed, Jan 21, 2026 at 10:22:08AM -0800, Darrick J. Wong wrote:
> > xchk_xfile_*_descr is used to pass the name to xfarray_create or
> > xfblob_create.  I still think it would make this a lot more robust if
> > those took a format string and varags, and then we'd have wrappers for
> > the common types.  Even if that still ends up doing kasprintf underneath,
> > that would be isolated to the low-level functions that only need to
> > implement error handling and freeing once.
> 
> Alternately we just drop all the helpers and kasprintf crap in favor of
> feeding the raw string ("iunlinked next pointers") all the way through
> to shmem_kernel_file_setup.

But wouldn't we get duplicate names for different inodes?

Anyway, I did a quick take at format string / varags version of the
helpers, and that works out nicely, but that _descr macros still confuse
me a bit.  Maybe I'll have something until the start of your Thursday.

Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls

[Darrick J. Wong]

On Thu, Jan 22, 2026 at 06:57:48AM +0100, Christoph Hellwig wrote:
> On Wed, Jan 21, 2026 at 10:22:08AM -0800, Darrick J. Wong wrote:
> > > xchk_xfile_*_descr is used to pass the name to xfarray_create or
> > > xfblob_create.  I still think it would make this a lot more robust if
> > > those took a format string and varags, and then we'd have wrappers for
> > > the common types.  Even if that still ends up doing kasprintf underneath,
> > > that would be isolated to the low-level functions that only need to
> > > implement error handling and freeing once.
> > 
> > Alternately we just drop all the helpers and kasprintf crap in favor of
> > feeding the raw string ("iunlinked next pointers") all the way through
> > to shmem_kernel_file_setup.
> 
> But wouldn't we get duplicate names for different inodes?

Yes, but that's only used for readlink of /proc/$pid/fd/* so (AFAICT) it
makes tracing more confusing but doesn't affect functionality.
xfs_healthmon just passes in "xfs_healthmon" and I can run healers on
multiple filesystems just fine.

anon inodes are ... uh ... magic.

> Anyway, I did a quick take at format string / varags version of the
> helpers, and that works out nicely, but that _descr macros still confuse
> me a bit.  Maybe I'll have something until the start of your Thursday.

--D

Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls

[Christoph Hellwig]

On Thu, Jan 22, 2026 at 10:57:01AM -0800, Darrick J. Wong wrote:
> > > Alternately we just drop all the helpers and kasprintf crap in favor of
> > > feeding the raw string ("iunlinked next pointers") all the way through
> > > to shmem_kernel_file_setup.
> > 
> > But wouldn't we get duplicate names for different inodes?
> 
> Yes, but that's only used for readlink of /proc/$pid/fd/* so (AFAICT) it
> makes tracing more confusing but doesn't affect functionality.
> xfs_healthmon just passes in "xfs_healthmon" and I can run healers on
> multiple filesystems just fine.
> 
> anon inodes are ... uh ... magic.

Ok, that certainly would simply things a lot, and I'd be ok with it.

My ideas didn't really work out.  The last idea I had was to be able
to specify a prefix in a new method in struct xchk_meta_ops, but
this starts to feel like severe overengineering.

Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls

[Darrick J. Wong]

On Fri, Jan 23, 2026 at 06:33:23AM +0100, Christoph Hellwig wrote:
> On Thu, Jan 22, 2026 at 10:57:01AM -0800, Darrick J. Wong wrote:
> > > > Alternately we just drop all the helpers and kasprintf crap in favor of
> > > > feeding the raw string ("iunlinked next pointers") all the way through
> > > > to shmem_kernel_file_setup.
> > > 
> > > But wouldn't we get duplicate names for different inodes?
> > 
> > Yes, but that's only used for readlink of /proc/$pid/fd/* so (AFAICT) it
> > makes tracing more confusing but doesn't affect functionality.
> > xfs_healthmon just passes in "xfs_healthmon" and I can run healers on
> > multiple filesystems just fine.
> > 
> > anon inodes are ... uh ... magic.
> 
> Ok, that certainly would simply things a lot, and I'd be ok with it.
> 
> My ideas didn't really work out.  The last idea I had was to be able
> to specify a prefix in a new method in struct xchk_meta_ops, but
> this starts to feel like severe overengineering.

<nod> I just replaced them with static strings and everything seemed to
work fine.  Will send a replacement patchset shortly.

--D

Re: [PATCH 2/4] xfs: only call xf{array,blob}_destroy if we have a valid pointer

[Chris Mason]

"Darrick J. Wong" <djwong@kernel.org> wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Only call the xfarray and xfblob destructor if we have a valid pointer,
> and be sure to null out that pointer afterwards.  Note that this patch
> fixes a large number of commits, most of which were merged between 6.9
> and 6.10.

Hi everyone,

I'm trying out my AI review prompts on a few more trees, and I ran it
on the fuse-iomap-cache branch:

https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=fuse-iomap-cache

This was the latest email lore could find, apologies if I grabbed the wrong one.

Hopefully the review below is useful, please let me know if there are places it
has gone wildly wrong.  I kept the header to make it obvious exactly which
commit I'd run through.

I was really going for the fuse patches, but this ended up in my git
rev-list output, and it looks valid to me:

commit 1e4e64674c788423e24610612e4981eb1dc9b28b
Author: Darrick J. Wong <djwong@kernel.org>

xfs: only call xf{array,blob}_destroy if we have a valid pointer

This patch adds null pointer checks before calling xfarray_destroy and
xfblob_destroy functions across multiple XFS scrub/repair teardown
functions, and nulls out the pointers afterwards to prevent dangling
pointer issues.

Cc: r772577952@gmail.com
Cc: <stable@vger.kernel.org> # v6.12
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")

> diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> index a2f6a7f71d839..6e3fef36d6614 100644
> --- a/fs/xfs/scrub/agheader_repair.c
> +++ b/fs/xfs/scrub/agheader_repair.c

[ ... ]

> diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
> index 7a21b688a4715..d5a55eabf6801 100644
> --- a/fs/xfs/scrub/dir_repair.c
> +++ b/fs/xfs/scrub/dir_repair.c
> @@ -172,8 +172,12 @@ xrep_dir_teardown(
>  	struct xrep_dir		*rd = sc->buf;
>
>  	xrep_findparent_scan_teardown(&rd->pscan);
> -	xfblob_destroy(rd->dir_names);
> -	xfarray_destroy(rd->dir_entries);
> +	if (rd->dir_names)
> +		xfblob_destroy(rd->dir_names);
> +	rd->dir_names = NULL;
> +	if (rd->dir_entries)
> +		xfarray_destroy(rd->dir_entries);
> +	rd->dir_names = NULL;
                 ^^^^^^^^^^

In xrep_dir_teardown, after destroying rd->dir_entries, the code sets
rd->dir_names = NULL instead of rd->dir_entries = NULL. This leaves
dir_entries as a dangling pointer after the xfarray is freed.

The commit message states the patch should "be sure to null out that
pointer afterwards", and the pattern in xrep_agi_buf_cleanup correctly
nulls both iunlink_prev and iunlink_next pointers. This change should
null both dir_names (line 177) and dir_entries (line 180), but line 180
incorrectly nulls dir_names again.

Shouldn't line 180 be rd->dir_entries = NULL?

[ ... ]

> Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")

The existing Fixes: tag points to commit ab97f4b1c03075, but this is not
the first commit that introduced the bug. The bug was first introduced by
commit f1184081ac97 ("xfs: teach scrub to check file nlinks") on
2024-02-22, which unconditionally called xfarray_destroy without checking
for NULL pointers. The commit ab97f4b1c03075 (2024-04-15) is one of
several later commits that perpetuated the same bug pattern.

Should the Fixes: tag reference f1184081ac97 instead?

Re: [PATCH 2/4] xfs: only call xf{array,blob}_destroy if we have a valid pointer

[Darrick J. Wong]

On Thu, Feb 05, 2026 at 11:40:27AM -0800, Chris Mason wrote:
> "Darrick J. Wong" <djwong@kernel.org> wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Only call the xfarray and xfblob destructor if we have a valid pointer,
> > and be sure to null out that pointer afterwards.  Note that this patch
> > fixes a large number of commits, most of which were merged between 6.9
> > and 6.10.
> 
> Hi everyone,
> 
> I'm trying out my AI review prompts on a few more trees, and I ran it
> on the fuse-iomap-cache branch:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=fuse-iomap-cache
> 
> This was the latest email lore could find, apologies if I grabbed the wrong one.
> 
> Hopefully the review below is useful, please let me know if there are places it
> has gone wildly wrong.  I kept the header to make it obvious exactly which
> commit I'd run through.
> 
> I was really going for the fuse patches, but this ended up in my git
> rev-list output, and it looks valid to me:
> 
> commit 1e4e64674c788423e24610612e4981eb1dc9b28b
> Author: Darrick J. Wong <djwong@kernel.org>
> 
> xfs: only call xf{array,blob}_destroy if we have a valid pointer
> 
> This patch adds null pointer checks before calling xfarray_destroy and
> xfblob_destroy functions across multiple XFS scrub/repair teardown
> functions, and nulls out the pointers afterwards to prevent dangling
> pointer issues.
> 
> Cc: r772577952@gmail.com
> Cc: <stable@vger.kernel.org> # v6.12
> Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
> 
> > diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> > index a2f6a7f71d839..6e3fef36d6614 100644
> > --- a/fs/xfs/scrub/agheader_repair.c
> > +++ b/fs/xfs/scrub/agheader_repair.c
> 
> [ ... ]
> 
> > diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
> > index 7a21b688a4715..d5a55eabf6801 100644
> > --- a/fs/xfs/scrub/dir_repair.c
> > +++ b/fs/xfs/scrub/dir_repair.c
> > @@ -172,8 +172,12 @@ xrep_dir_teardown(
> >  	struct xrep_dir		*rd = sc->buf;
> >
> >  	xrep_findparent_scan_teardown(&rd->pscan);
> > -	xfblob_destroy(rd->dir_names);
> > -	xfarray_destroy(rd->dir_entries);
> > +	if (rd->dir_names)
> > +		xfblob_destroy(rd->dir_names);
> > +	rd->dir_names = NULL;
> > +	if (rd->dir_entries)
> > +		xfarray_destroy(rd->dir_entries);
> > +	rd->dir_names = NULL;
>                  ^^^^^^^^^^
> 
> In xrep_dir_teardown, after destroying rd->dir_entries, the code sets
> rd->dir_names = NULL instead of rd->dir_entries = NULL. This leaves
> dir_entries as a dangling pointer after the xfarray is freed.
> 
> The commit message states the patch should "be sure to null out that
> pointer afterwards", and the pattern in xrep_agi_buf_cleanup correctly
> nulls both iunlink_prev and iunlink_next pointers. This change should
> null both dir_names (line 177) and dir_entries (line 180), but line 180
> incorrectly nulls dir_names again.
> 
> Shouldn't line 180 be rd->dir_entries = NULL?

Yep.  Will fix this.

> [ ... ]
> 
> > Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
> 
> The existing Fixes: tag points to commit ab97f4b1c03075, but this is not
> the first commit that introduced the bug. The bug was first introduced by
> commit f1184081ac97 ("xfs: teach scrub to check file nlinks") on
> 2024-02-22, which unconditionally called xfarray_destroy without checking
> for NULL pointers. The commit ab97f4b1c03075 (2024-04-15) is one of
> several later commits that perpetuated the same bug pattern.
> 
> Should the Fixes: tag reference f1184081ac97 instead?

Strictly speaking there should have been separate patches for each of
the files fixed in this patch, but I went with the more recent commit
which was introduced in 6.12 rather than the oldest commit from 6.9
because only 6.12 is receiving fixes anyway.

--D