[PATCHSET] xfs: LLM-inspired bug fixes, part 14

[PATCHSET] xfs: LLM-inspired bug fixes, part 14

[Darrick J. Wong]

Hi all,

Here's a fourteenth batch of xfs fixes resulting from a LLaMma.  Mwa mwa
mwa...

If you're going to start using this code, I strongly recommend pulling
from my git trees, which are linked below.

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

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=llm-fixes-14
---
Commits in this patchset:
 * xfs: check padding field in xfs_ioc_commit_range
 * xfs: don't call xfs_exchange_range_finish for a dry run
 * xfs: use the correct reservations for rtrmap/refcount recovery
 * xfs: fix integer overflows in xbitmap set functions
 * xfs: only flag zero padding for dir3 data blocks, not dir3 block blocks
 * xfs: check di_forkoff correctly in scrub
---
 fs/xfs/scrub/bitmap.c      |   26 ++++++++++++++++++--------
 fs/xfs/scrub/dir.c         |    2 +-
 fs/xfs/scrub/inode.c       |    2 +-
 fs/xfs/xfs_exchrange.c     |   13 +++++++++----
 fs/xfs/xfs_refcount_item.c |    8 ++++++--
 fs/xfs/xfs_rmap_item.c     |    8 ++++++--
 6 files changed, 41 insertions(+), 18 deletions(-)

[PATCH 1/6] xfs: check padding field in xfs_ioc_commit_range

[Darrick J. Wong]

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

LOLLM points out that we don't check the ioctl padding field here, so
let's do that.  I don't think there are many users yet since exchrange
requires a new feature flag, so it's a good time to try to plug this
hole.

Cc: <stable@vger.kernel.org> # v6.12
Fixes: 398597c3ef7fb1 ("xfs: introduce new file range commit ioctls")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_exchrange.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_exchrange.c b/fs/xfs/xfs_exchrange.c
index c69ecd6a19de4f..07090487c581e8 100644
--- a/fs/xfs/xfs_exchrange.c
+++ b/fs/xfs/xfs_exchrange.c
@@ -902,7 +902,7 @@ xfs_ioc_commit_range(
 
 	if (copy_from_user(&args, argp, sizeof(args)))
 		return -EFAULT;
-	if (args.flags & ~XFS_EXCHANGE_RANGE_ALL_FLAGS)
+	if (args.pad || (args.flags & ~XFS_EXCHANGE_RANGE_ALL_FLAGS))
 		return -EINVAL;
 	if (kern_f->magic != XCR_FRESH_MAGIC)
 		return -EBUSY;

Re: [PATCH 1/6] xfs: check padding field in xfs_ioc_commit_range

[Christoph Hellwig]

On Thu, Sep 10, 2026 at 10:53:39PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that we don't check the ioctl padding field here, so
> let's do that.  I don't think there are many users yet since exchrange
> requires a new feature flag, so it's a good time to try to plug this
> hole.

Let's hope no one is doing stupid things here yet..

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

[PATCH 2/6] xfs: don't call xfs_exchange_range_finish for a dry run

[Darrick J. Wong]

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

LOLLM noticed that we strip file privileges and whatnot even for a dry
run.  We also shouldn't flush dirty data to disk or trim COW staging
events for a dry run.  Neither of those behaviors are allowed by the
manpage, so fix that by exiting early on DRY_RUN in various functions.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 42672471f938cd ("xfs: bind together the front and back ends of the file range exchange code")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_exchrange.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/xfs_exchrange.c b/fs/xfs/xfs_exchrange.c
index 07090487c581e8..fafb4e3f065c75 100644
--- a/fs/xfs/xfs_exchrange.c
+++ b/fs/xfs/xfs_exchrange.c
@@ -633,6 +633,9 @@ xfs_exchrange_prep(
 	if (error)
 		return error;
 
+	if (fxr->flags & XFS_EXCHANGE_RANGE_DRY_RUN)
+		return 0;
+
 	trace_xfs_exchrange_flush(fxr, ip1, ip2);
 
 	/* Flush the relevant ranges of both files. */
@@ -709,9 +712,11 @@ xfs_exchrange_contents(
 	 * other file write would do.  This may involve turning on support for
 	 * logged xattrs if either file has security capabilities.
 	 */
-	error = xfs_exchange_range_finish(fxr);
-	if (error)
-		goto out_unlock;
+	if (!(fxr->flags & XFS_EXCHANGE_RANGE_DRY_RUN)) {
+		error = xfs_exchange_range_finish(fxr);
+		if (error)
+			goto out_unlock;
+	}
 
 out_unlock:
 	xfs_iunlock2_io_mmap(ip1, ip2);

Re: [PATCH 2/6] xfs: don't call xfs_exchange_range_finish for a dry run

[Christoph Hellwig]

Looks good:

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

[PATCH 3/6] xfs: use the correct reservations for rtrmap/refcount recovery

[Darrick J. Wong]

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

LOLLM noticed that we might reserve the wrong number of blocks for
recovering rtrmap and rtrefcount updates after a crash.  Fix that.

Cc: <stable@vger.kernel.org> # v6.14
Fixes: 5e0679d1c62f25 ("xfs: support recovering rmap intent items targetting realtime extents")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_refcount_item.c |    8 ++++++--
 fs/xfs/xfs_rmap_item.c     |    8 ++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)


diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index 8bccf89a77668b..682c6e1b45e3ee 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -508,6 +508,7 @@ xfs_refcount_recover_work(
 	struct xfs_cui_log_item		*cuip = CUI_ITEM(lip);
 	struct xfs_trans		*tp;
 	struct xfs_mount		*mp = lip->li_log->l_mp;
+	unsigned int			dblocks;
 	bool				isrt = xfs_cui_item_isrt(lip);
 	int				i;
 	int				error = 0;
@@ -543,8 +544,11 @@ xfs_refcount_recover_work(
 	 * full btree split on either end of the refcount range.
 	 */
 	resv = xlog_recover_resv(&M_RES(mp)->tr_itruncate);
-	error = xfs_trans_alloc(mp, &resv, mp->m_refc_maxlevels * 2, 0,
-			XFS_TRANS_RESERVE, &tp);
+	if (isrt)
+		dblocks = mp->m_rtrefc_maxlevels * 2;
+	else
+		dblocks = mp->m_refc_maxlevels * 2;
+	error = xfs_trans_alloc(mp, &resv, dblocks, 0, XFS_TRANS_RESERVE, &tp);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index 2a3a73a8566d11..000cff1ce324f0 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -573,6 +573,7 @@ xfs_rmap_recover_work(
 	struct xfs_rui_log_item		*ruip = RUI_ITEM(lip);
 	struct xfs_trans		*tp;
 	struct xfs_mount		*mp = lip->li_log->l_mp;
+	unsigned int			dblocks;
 	bool				isrt = xfs_rui_item_isrt(lip);
 	int				i;
 	int				error = 0;
@@ -596,8 +597,11 @@ xfs_rmap_recover_work(
 	}
 
 	resv = xlog_recover_resv(&M_RES(mp)->tr_itruncate);
-	error = xfs_trans_alloc(mp, &resv, mp->m_rmap_maxlevels, 0,
-			XFS_TRANS_RESERVE, &tp);
+	if (isrt)
+		dblocks = mp->m_rtrmap_maxlevels;
+	else
+		dblocks = mp->m_rmap_maxlevels;
+	error = xfs_trans_alloc(mp, &resv, dblocks, 0, XFS_TRANS_RESERVE, &tp);
 	if (error)
 		return error;
 

Re: [PATCH 3/6] xfs: use the correct reservations for rtrmap/refcount recovery

[Christoph Hellwig]

On Thu, Sep 10, 2026 at 10:54:11PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that we might reserve the wrong number of blocks for
> recovering rtrmap and rtrefcount updates after a crash.  Fix that.

Looks good:

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

[PATCH 4/6] xfs: fix integer overflows in xbitmap set functions

[Darrick J. Wong]

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

LOLLM complains that the xbitmap set functions can suffer an integer
underflow or overflow and thereby return the wrong left and right
pointers.  Fix that logic bomb, even though (AFAICT) we never actually
try to set the *entire* bitmap.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/bitmap.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)


diff --git a/fs/xfs/scrub/bitmap.c b/fs/xfs/scrub/bitmap.c
index c7fa908d92b25e..08f216d26ec632 100644
--- a/fs/xfs/scrub/bitmap.c
+++ b/fs/xfs/scrub/bitmap.c
@@ -122,8 +122,8 @@ xbitmap64_set(
 	uint64_t		start,
 	uint64_t		len)
 {
-	struct xbitmap64_node	*left;
-	struct xbitmap64_node	*right;
+	struct xbitmap64_node	*left = NULL;
+	struct xbitmap64_node	*right = NULL;
 	uint64_t		last = start + len - 1;
 	int			error;
 
@@ -131,6 +131,7 @@ xbitmap64_set(
 	left = xbitmap64_tree_iter_first(&bitmap->xb_root, start, last);
 	if (left && left->bn_start <= start && left->bn_last >= last)
 		return 0;
+	left = NULL;
 
 	/* Clear out everything in the range we want to set. */
 	error = xbitmap64_clear(bitmap, start, len);
@@ -138,11 +139,15 @@ xbitmap64_set(
 		return error;
 
 	/* Do we have a left-adjacent extent? */
-	left = xbitmap64_tree_iter_first(&bitmap->xb_root, start - 1, start - 1);
+	if (start > 0)
+		left = xbitmap64_tree_iter_first(&bitmap->xb_root, start - 1,
+				start - 1);
 	ASSERT(!left || left->bn_last + 1 == start);
 
 	/* Do we have a right-adjacent extent? */
-	right = xbitmap64_tree_iter_first(&bitmap->xb_root, last + 1, last + 1);
+	if (last < U64_MAX)
+		right = xbitmap64_tree_iter_first(&bitmap->xb_root, last + 1,
+				last + 1);
 	ASSERT(!right || right->bn_start == last + 1);
 
 	if (left && right) {
@@ -397,8 +402,8 @@ xbitmap32_set(
 	uint32_t		start,
 	uint32_t		len)
 {
-	struct xbitmap32_node	*left;
-	struct xbitmap32_node	*right;
+	struct xbitmap32_node	*left = NULL;
+	struct xbitmap32_node	*right = NULL;
 	uint32_t		last = start + len - 1;
 	int			error;
 
@@ -406,6 +411,7 @@ xbitmap32_set(
 	left = xbitmap32_tree_iter_first(&bitmap->xb_root, start, last);
 	if (left && left->bn_start <= start && left->bn_last >= last)
 		return 0;
+	left = NULL;
 
 	/* Clear out everything in the range we want to set. */
 	error = xbitmap32_clear(bitmap, start, len);
@@ -413,11 +419,15 @@ xbitmap32_set(
 		return error;
 
 	/* Do we have a left-adjacent extent? */
-	left = xbitmap32_tree_iter_first(&bitmap->xb_root, start - 1, start - 1);
+	if (start > 0)
+		left = xbitmap32_tree_iter_first(&bitmap->xb_root, start - 1,
+				start - 1);
 	ASSERT(!left || left->bn_last + 1 == start);
 
 	/* Do we have a right-adjacent extent? */
-	right = xbitmap32_tree_iter_first(&bitmap->xb_root, last + 1, last + 1);
+	if (last < U32_MAX)
+		right = xbitmap32_tree_iter_first(&bitmap->xb_root, last + 1,
+				last + 1);
 	ASSERT(!right || right->bn_start == last + 1);
 
 	if (left && right) {

Re: [PATCH 4/6] xfs: fix integer overflows in xbitmap set functions

[Christoph Hellwig]

Looks good:

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

[PATCH 5/6] xfs: only flag zero padding for dir3 data blocks, not dir3 block blocks

[Darrick J. Wong]

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

LOLLM complains that xchk_directory_data_bestfree can be passed a
directory block that is either in "block" or "data" format, but the
check here unconditionally treats the dir3_block and dir3_data blocks as
if they have the same header format (they don't).  Consequently, we can
incorrectly set the preen state on dir3_block blocks, which of course
we can't preen away because dir3_block blocks do not have a padding
field.  Fix this.

Cc: <stable@vger.kernel.org> # v7.1-rc4
Fixes: 939919ccddfcc3 ("xfs: check directory data block header padding in scrub")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/dir.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index 2a037aae904d03..19d974c7e2b7a9 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -492,7 +492,7 @@ xchk_directory_data_bestfree(
 		goto out;
 	xchk_buffer_recheck(sc, bp);
 
-	if (xfs_has_crc(sc->mp)) {
+	if (!is_block && xfs_has_crc(sc->mp)) {
 		struct xfs_dir3_data_hdr    *hdr3 = bp->b_addr;
 
 		if (hdr3->pad)

Re: [PATCH 5/6] xfs: only flag zero padding for dir3 data blocks, not dir3 block blocks

[Christoph Hellwig]

On Thu, Sep 10, 2026 at 10:54:42PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM complains that xchk_directory_data_bestfree can be passed a
> directory block that is either in "block" or "data" format, but the
> check here unconditionally treats the dir3_block and dir3_data blocks as
> if they have the same header format (they don't).  Consequently, we can
> incorrectly set the preen state on dir3_block blocks, which of course
> we can't preen away because dir3_block blocks do not have a padding
> field.  Fix this.

Looks good:

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

[PATCH 6/6] xfs: check di_forkoff correctly in scrub

[Darrick J. Wong]

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

The di_forkoff check in xchk_dinode is incorrect, according to LOLLM.
XFS_DFORK_BOFF returns a byte count relative to the start of the literal
area, not the start of the inode.  Therefore, this check won't flag
di_forkoff values that are larger than the literal area but not the
inode size itself.  Fix this check; sadly the old APTR code was correct.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: 6b5d917780219d ("xfs: dont cast to char * for XFS_DFORK_*PTR macros")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/inode.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
index 65b13e31191681..46e9bf4a43179e 100644
--- a/fs/xfs/scrub/inode.c
+++ b/fs/xfs/scrub/inode.c
@@ -607,7 +607,7 @@ xchk_dinode(
 	}
 
 	/* di_forkoff */
-	if (XFS_DFORK_BOFF(dip) >= mp->m_sb.sb_inodesize)
+	if (dip->di_forkoff >= (XFS_LITINO(mp) >> 3))
 		xchk_ino_set_corrupt(sc, ino);
 	if (naextents != 0 && dip->di_forkoff == 0)
 		xchk_ino_set_corrupt(sc, ino);

Re: [PATCH 6/6] xfs: check di_forkoff correctly in scrub

[Christoph Hellwig]

On Thu, Sep 10, 2026 at 10:54:58PM -0700, Darrick J. Wong wrote:
> The di_forkoff check in xchk_dinode is incorrect, according to LOLLM.
> XFS_DFORK_BOFF returns a byte count relative to the start of the literal
> area, not the start of the inode.  Therefore, this check won't flag
> di_forkoff values that are larger than the literal area but not the
> inode size itself.  Fix this check; sadly the old APTR code was correct.

The fi looks good, but I really which we could encapsulate this
magic >> 3 a bit better (here and elsewhere).

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

Re: [PATCH 6/6] xfs: check di_forkoff correctly in scrub

[Darrick J. Wong]

On Fri, Sep 11, 2026 at 05:07:45PM +0200, Christoph Hellwig wrote:
> On Thu, Sep 10, 2026 at 10:54:58PM -0700, Darrick J. Wong wrote:
> > The di_forkoff check in xchk_dinode is incorrect, according to LOLLM.
> > XFS_DFORK_BOFF returns a byte count relative to the start of the literal
> > area, not the start of the inode.  Therefore, this check won't flag
> > di_forkoff values that are larger than the literal area but not the
> > inode size itself.  Fix this check; sadly the old APTR code was correct.
> 
> The fi looks good, but I really which we could encapsulate this
> magic >> 3 a bit better (here and elsewhere).

I /did/ think about adding a cleanup to do:

#define XFS_B_TO_FORKOFF(b)	((b) >> 3)
#define XFS_FORKOFF_TO_B(f)	((f) << 3)

and then code like this:

fs/xfs/libxfs/xfs_attr_leaf.c:782:      minforkoff = max_t(int64_t, dsize, xfs_bmdr_space_calc(MINDBTPTRS));
fs/xfs/libxfs/xfs_attr_leaf.c:783:      minforkoff = roundup(minforkoff, 8) >> 3;
fs/xfs/libxfs/xfs_attr_leaf.c:786:      maxforkoff = XFS_LITINO(mp) - xfs_bmdr_space_calc(MINABTPTRS);
fs/xfs/libxfs/xfs_attr_leaf.c:787:      maxforkoff = maxforkoff >> 3;   /* rounded down */

becomes:

	minforkoff = XFS_B_TO_FORKOFF(max_t(int64_t, dsize,
				xfs_bmdr_space_calc(MINDBTPTRS)));
	maxforkoff = XFS_B_TO_FORKOFF(XFS_LITINO(mp) -
				xfs_bmdr_space_calc(MINABTPTRS));

There aren't as many users of FORKOFF_TO_B though:

static inline unsigned int xfs_inode_fork_boff(struct xfs_inode *ip)
{
	return XFS_FORKOFF_TO_B(ip->i_forkoff);
}

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

Thanks!

--D