[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;

[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);

[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;
 

[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)

[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 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>

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>

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>

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>

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

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

[Christoph Hellwig]

On Fri, Sep 11, 2026 at 08:59:19AM -0700, Darrick J. Wong wrote:
> 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:

That would be nice.  Especially with a comment explaining the odd
encoding a bit more.

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

[Carlos Maiolino]

On Thu, 10 Sep 2026 22:53:34 -0700, Darrick J. Wong wrote:
> 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.
> 
> [...]

Applied to for-next, thanks!

[1/6] xfs: check padding field in xfs_ioc_commit_range
      commit: 3083ba8dde765a9ab2337f3db68d00724a6b1202
[2/6] xfs: don't call xfs_exchange_range_finish for a dry run
      commit: 8fc18580ec17f90beac4c933fbe4c74dcd3b7f36
[3/6] xfs: use the correct reservations for rtrmap/refcount recovery
      commit: 471e0b6e2ddac9e16b8dc2153d6e5575fefb8a2f
[4/6] xfs: fix integer overflows in xbitmap set functions
      commit: 46c1b6674a7b3a8385e7fa27f4efc6f45eddf967
[5/6] xfs: only flag zero padding for dir3 data blocks, not dir3 block blocks
      commit: c54110d814c3e8ed6bbbb5e02874994ed9f668ac
[6/6] xfs: check di_forkoff correctly in scrub
      commit: e9193f2f1ce32d02b9230094ffdbfab715ab6137

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>