* [PATCHSET] xfs: syzbot fixes for online fsck
@ 2026-01-21 6:40 Darrick J. Wong
2026-01-21 6:40 ` [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls Darrick J. Wong
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Darrick J. Wong @ 2026-01-21 6:40 UTC (permalink / raw)
To: cem, djwong; +Cc: stable, r772577952, r772577952, hch, linux-xfs
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(-)
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls 2026-01-21 6:40 [PATCHSET] xfs: syzbot fixes for online fsck Darrick J. Wong @ 2026-01-21 6:40 ` Darrick J. Wong 2026-01-21 7:03 ` Christoph Hellwig 2026-01-21 6:40 ` [PATCH 2/4] xfs: only call xf{array,blob}_destroy if we have a valid pointer Darrick J. Wong ` (2 subsequent siblings) 3 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2026-01-21 6:40 UTC (permalink / raw) To: cem, djwong; +Cc: r772577952, stable, r772577952, hch, linux-xfs 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); ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls 2026-01-21 6:40 ` [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls Darrick J. Wong @ 2026-01-21 7:03 ` Christoph Hellwig 2026-01-21 18:22 ` Darrick J. Wong 0 siblings, 1 reply; 15+ messages in thread From: Christoph Hellwig @ 2026-01-21 7:03 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, r772577952, stable, hch, linux-xfs 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--- ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls 2026-01-21 7:03 ` Christoph Hellwig @ 2026-01-21 18:22 ` Darrick J. Wong 2026-01-22 5:57 ` Christoph Hellwig 0 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2026-01-21 18:22 UTC (permalink / raw) To: Christoph Hellwig; +Cc: cem, r772577952, stable, linux-xfs 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--- > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls 2026-01-21 18:22 ` Darrick J. Wong @ 2026-01-22 5:57 ` Christoph Hellwig 2026-01-22 18:57 ` Darrick J. Wong 0 siblings, 1 reply; 15+ messages in thread From: Christoph Hellwig @ 2026-01-22 5:57 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Christoph Hellwig, cem, r772577952, stable, linux-xfs 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls 2026-01-22 5:57 ` Christoph Hellwig @ 2026-01-22 18:57 ` Darrick J. Wong 2026-01-23 5:33 ` Christoph Hellwig 0 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2026-01-22 18:57 UTC (permalink / raw) To: Christoph Hellwig; +Cc: cem, r772577952, stable, linux-xfs 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls 2026-01-22 18:57 ` Darrick J. Wong @ 2026-01-23 5:33 ` Christoph Hellwig 2026-01-23 7:00 ` Darrick J. Wong 0 siblings, 1 reply; 15+ messages in thread From: Christoph Hellwig @ 2026-01-23 5:33 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Christoph Hellwig, cem, r772577952, stable, linux-xfs 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls 2026-01-23 5:33 ` Christoph Hellwig @ 2026-01-23 7:00 ` Darrick J. Wong 0 siblings, 0 replies; 15+ messages in thread From: Darrick J. Wong @ 2026-01-23 7:00 UTC (permalink / raw) To: Christoph Hellwig; +Cc: cem, r772577952, stable, linux-xfs 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/4] xfs: only call xf{array,blob}_destroy if we have a valid pointer 2026-01-21 6:40 [PATCHSET] xfs: syzbot fixes for online fsck Darrick J. Wong 2026-01-21 6:40 ` [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls Darrick J. Wong @ 2026-01-21 6:40 ` Darrick J. Wong 2026-02-05 19:40 ` Chris Mason 2026-01-21 6:40 ` [PATCH 3/4] xfs: check return value of xchk_scrub_create_subord Darrick J. Wong 2026-01-21 6:41 ` [PATCH 4/4] xfs: fix UAF in xchk_btree_check_block_owner Darrick J. Wong 3 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2026-01-21 6:40 UTC (permalink / raw) To: cem, djwong; +Cc: r772577952, stable, r772577952, hch, linux-xfs 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); ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] xfs: only call xf{array,blob}_destroy if we have a valid pointer 2026-01-21 6:40 ` [PATCH 2/4] xfs: only call xf{array,blob}_destroy if we have a valid pointer Darrick J. Wong @ 2026-02-05 19:40 ` Chris Mason 2026-02-06 4:46 ` Darrick J. Wong 0 siblings, 1 reply; 15+ messages in thread From: Chris Mason @ 2026-02-05 19:40 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, r772577952, stable, hch, linux-xfs "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? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] xfs: only call xf{array,blob}_destroy if we have a valid pointer 2026-02-05 19:40 ` Chris Mason @ 2026-02-06 4:46 ` Darrick J. Wong 0 siblings, 0 replies; 15+ messages in thread From: Darrick J. Wong @ 2026-02-06 4:46 UTC (permalink / raw) To: Chris Mason; +Cc: cem, r772577952, stable, hch, linux-xfs 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/4] xfs: check return value of xchk_scrub_create_subord 2026-01-21 6:40 [PATCHSET] xfs: syzbot fixes for online fsck Darrick J. Wong 2026-01-21 6:40 ` [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls Darrick J. Wong 2026-01-21 6:40 ` [PATCH 2/4] xfs: only call xf{array,blob}_destroy if we have a valid pointer Darrick J. Wong @ 2026-01-21 6:40 ` Darrick J. Wong 2026-01-21 7:03 ` Christoph Hellwig 2026-01-21 6:41 ` [PATCH 4/4] xfs: fix UAF in xchk_btree_check_block_owner Darrick J. Wong 3 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2026-01-21 6:40 UTC (permalink / raw) To: cem, djwong; +Cc: r772577952, stable, r772577952, hch, linux-xfs 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; ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] xfs: check return value of xchk_scrub_create_subord 2026-01-21 6:40 ` [PATCH 3/4] xfs: check return value of xchk_scrub_create_subord Darrick J. Wong @ 2026-01-21 7:03 ` Christoph Hellwig 0 siblings, 0 replies; 15+ messages in thread From: Christoph Hellwig @ 2026-01-21 7:03 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, r772577952, stable, hch, linux-xfs Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/4] xfs: fix UAF in xchk_btree_check_block_owner 2026-01-21 6:40 [PATCHSET] xfs: syzbot fixes for online fsck Darrick J. Wong ` (2 preceding siblings ...) 2026-01-21 6:40 ` [PATCH 3/4] xfs: check return value of xchk_scrub_create_subord Darrick J. Wong @ 2026-01-21 6:41 ` Darrick J. Wong 2026-01-21 7:04 ` Christoph Hellwig 3 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2026-01-21 6:41 UTC (permalink / raw) To: cem, djwong; +Cc: r772577952, stable, r772577952, hch, linux-xfs 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: ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] xfs: fix UAF in xchk_btree_check_block_owner 2026-01-21 6:41 ` [PATCH 4/4] xfs: fix UAF in xchk_btree_check_block_owner Darrick J. Wong @ 2026-01-21 7:04 ` Christoph Hellwig 0 siblings, 0 replies; 15+ messages in thread From: Christoph Hellwig @ 2026-01-21 7:04 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, r772577952, stable, hch, linux-xfs 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> ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-02-06 4:46 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-21 6:40 [PATCHSET] xfs: syzbot fixes for online fsck Darrick J. Wong
2026-01-21 6:40 ` [PATCH 1/4] xfs: check the return value of xchk_xfile_*_descr calls Darrick J. Wong
2026-01-21 7:03 ` Christoph Hellwig
2026-01-21 18:22 ` Darrick J. Wong
2026-01-22 5:57 ` Christoph Hellwig
2026-01-22 18:57 ` Darrick J. Wong
2026-01-23 5:33 ` Christoph Hellwig
2026-01-23 7:00 ` Darrick J. Wong
2026-01-21 6:40 ` [PATCH 2/4] xfs: only call xf{array,blob}_destroy if we have a valid pointer Darrick J. Wong
2026-02-05 19:40 ` Chris Mason
2026-02-06 4:46 ` Darrick J. Wong
2026-01-21 6:40 ` [PATCH 3/4] xfs: check return value of xchk_scrub_create_subord Darrick J. Wong
2026-01-21 7:03 ` Christoph Hellwig
2026-01-21 6:41 ` [PATCH 4/4] xfs: fix UAF in xchk_btree_check_block_owner Darrick J. Wong
2026-01-21 7:04 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox