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

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

[Darrick J. Wong]

Hi all,

Here's a fifteenth 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-15
---
Commits in this patchset:
 * xfs: always call xfs_qm_adjust_dqlimits during quotacheck repair
 * xfs: call xfs_dquot_set_prealloc_limits if we installed default rtb limits
 * xfs: fix rtgroup repair estimations
 * xfs: don't cross reference rmapbt with bitmaps if they're incomplete
 * xfs: don't let memory failures leak blocks and kill repairs
 * xfs: don't merge different file IO error types
 * xfs: fix blockgc group quota scanning when usrquota isn't enforced
 * xfs: fix cursor and pointer handling when recovering iunlink buckets
 * xfs: drop dquot flush lock when we can't find a buffer to flush
 * xfs: don't let hidden_space go negative in xfs_metafile_resv_init
 * xfs: fix wild memcpy access when formatting ondisk rtrefcount btree roots
---
 fs/xfs/scrub/trace.h                 |   12 +++++--
 fs/xfs/libxfs/xfs_metafile.c         |    8 ++---
 fs/xfs/libxfs/xfs_rtrefcount_btree.c |    4 +-
 fs/xfs/scrub/newbt.c                 |    8 +++--
 fs/xfs/scrub/quotacheck_repair.c     |   55 +++++++++++++++++++++++++++++++++-
 fs/xfs/scrub/repair.c                |   19 +++++++++++-
 fs/xfs/scrub/rmap.c                  |   20 +++++++++---
 fs/xfs/xfs_dquot.c                   |    8 ++++-
 fs/xfs/xfs_healthmon.c               |    4 ++
 fs/xfs/xfs_icache.c                  |    2 +
 fs/xfs/xfs_log_recover.c             |   13 +++++---
 fs/xfs/xfs_qm.c                      |   10 +++++-
 12 files changed, 130 insertions(+), 33 deletions(-)

[PATCH 01/11] xfs: always call xfs_qm_adjust_dqlimits during quotacheck repair

[Darrick J. Wong]

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

LOLLM notices that everywhere else in the kernel, a call to
xfs_qm_adjust_dqlimits precedes every call to xfs_qm_adjust_dqtimers.
In particular, mount-time quotacheck does this.

Looking at xfs_qm_adjust_dqlimits, that function is in charge of
conveying default limits to a dquot if that dquot's limits have been
zeroed.  That's quite possible in a repair, so let's follow what
everyone else does and set it.  This requires us to add a new predicate
to decide if xfs_qm_adjust_dqlimits is likely to reset any of the
dquot's limits, and to set the dirty flag to true if so.

A different LOLLM noticed that we have no way to force xchk_commit_dquot
to call xfs_qm_adjust_dqtimers, so pack that in at the same time.

Cc: <stable@vger.kernel.org> # v6.9
Fixes: 96ed2ae4a9b06b ("xfs: repair dquots based on live quotacheck results")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/quotacheck_repair.c |   55 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/quotacheck_repair.c b/fs/xfs/scrub/quotacheck_repair.c
index dbb522e1513b0b..c35982ba2e1721 100644
--- a/fs/xfs/scrub/quotacheck_repair.c
+++ b/fs/xfs/scrub/quotacheck_repair.c
@@ -39,6 +39,54 @@
  * dquot is locked.
  */
 
+static bool
+xqcheck_dqres_force_dirty(
+	const struct xfs_dquot_res	*res,
+	const struct xfs_quota_limits	*qlim)
+{
+	/* zero limits mean that we should set the default limits */
+	if (res->softlimit == 0 && qlim->soft != 0)
+		return true;
+	if (res->hardlimit == 0 && qlim->hard != 0)
+		return true;
+
+	/* do we need to adjust the timer setting? */
+	if ((res->softlimit && res->count > res->softlimit) ||
+	    (res->hardlimit && res->count > res->hardlimit)) {
+		if (res->timer == 0 && qlim->time != 0)
+			return true;
+	} else {
+		if (res->timer)
+			return true;
+	}
+
+	return false;
+}
+
+/* Decide if we need to adjust the dquot limits or timers */
+static bool
+xqcheck_dquot_force_dirty(
+	const struct xfs_dquot	*dq)
+{
+	struct xfs_quotainfo	*qi = dq->q_mount->m_quotainfo;
+	struct xfs_def_quota	*defq;
+
+	/* root dquot does not enforce limits */
+	if (dq->q_id == 0)
+		return false;
+
+	defq = xfs_get_defquota(qi, xfs_dquot_type(dq));
+
+	if (xqcheck_dqres_force_dirty(&dq->q_blk, &defq->blk))
+		return true;
+	if (xqcheck_dqres_force_dirty(&dq->q_ino, &defq->ino))
+		return true;
+	if (xqcheck_dqres_force_dirty(&dq->q_rtb, &defq->rtb))
+		return true;
+
+	return false;
+}
+
 /* Commit new counters to a dquot. */
 static int
 xqcheck_commit_dquot(
@@ -91,6 +139,9 @@ xqcheck_commit_dquot(
 		dirty = true;
 	}
 
+	if (!dirty && xqcheck_dquot_force_dirty(dq))
+		dirty = true;
+
 	xcdq.flags |= (XQCHECK_DQUOT_REPAIR_SCANNED | XQCHECK_DQUOT_WRITTEN);
 	error = xfarray_store(counts, dq->q_id, &xcdq);
 	if (error == -EFBIG) {
@@ -110,8 +161,10 @@ xqcheck_commit_dquot(
 
 	/* Commit the dirty dquot to disk. */
 	dq->q_flags |= XFS_DQFLAG_DIRTY;
-	if (dq->q_id)
+	if (dq->q_id) {
+		xfs_qm_adjust_dqlimits(dq);
 		xfs_qm_adjust_dqtimers(dq);
+	}
 	xfs_trans_log_dquot(xqc->sc->tp, dq);
 	return xrep_trans_commit(xqc->sc);
 

[PATCH 02/11] xfs: call xfs_dquot_set_prealloc_limits if we installed default rtb limits

[Darrick J. Wong]

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

Now that we have quotas for the realtime volume, we also have
precomputed watermark limits for the realtime block counts.  These
precomputations should be done any time we change the rtb limits, which
means that xfs_qm_adjust_dqlimits needs to ensure that if we installed
a default rtb limit.

Cc: <stable@vger.kernel.org> # v6.13
Fixes: 5dd70852b03901 ("xfs: create quota preallocation watermarks for realtime quota")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_dquot.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index b4f6c594808ce9..e696ee36c2e8d2 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -139,10 +139,14 @@ xfs_qm_adjust_dqlimits(
 		dq->q_ino.softlimit = defq->ino.soft;
 	if (!dq->q_ino.hardlimit)
 		dq->q_ino.hardlimit = defq->ino.hard;
-	if (!dq->q_rtb.softlimit)
+	if (!dq->q_rtb.softlimit) {
 		dq->q_rtb.softlimit = defq->rtb.soft;
-	if (!dq->q_rtb.hardlimit)
+		prealloc = 1;
+	}
+	if (!dq->q_rtb.hardlimit) {
 		dq->q_rtb.hardlimit = defq->rtb.hard;
+		prealloc = 1;
+	}
 
 	if (prealloc)
 		xfs_dquot_set_prealloc_limits(dq);

[PATCH 03/11] xfs: fix rtgroup repair estimations

[Darrick J. Wong]

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

When I added online fsck for realtime reflink, I forgot to update
xrep_calc_rtgroup_resblks to factor in the size of the refcount btree
when it guesses how much space we need to start a repair.  This hasn't
been a huge problem in practice because there are few filesystems with
(a) realtime, (b) rtgroups, (c) reflink, and (d) no rmap.  But let's fix
this before someone stumbles upon it, especially since LOLLM flagged
this for me.

Cc: <stable@vger.kernel.org> # v6.14
Fixes: 83ccffc489975d ("xfs: online repair of the realtime refcount btree")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/trace.h  |   12 ++++++++----
 fs/xfs/scrub/repair.c |   19 +++++++++++++++++--
 2 files changed, 25 insertions(+), 6 deletions(-)


diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
index 0f5adc293962fa..cb85f75ce10180 100644
--- a/fs/xfs/scrub/trace.h
+++ b/fs/xfs/scrub/trace.h
@@ -2376,25 +2376,29 @@ TRACE_EVENT(xrep_calc_ag_resblks_btsize,
 #ifdef CONFIG_XFS_RT
 TRACE_EVENT(xrep_calc_rtgroup_resblks_btsize,
 	TP_PROTO(struct xfs_mount *mp, xfs_rgnumber_t rgno,
-		 xfs_rgblock_t usedlen, xfs_rgblock_t rmapbt_sz),
-	TP_ARGS(mp, rgno, usedlen, rmapbt_sz),
+		 xfs_rgblock_t usedlen, xfs_rgblock_t rmapbt_sz,
+		 xfs_rgblock_t refcbt_sz),
+	TP_ARGS(mp, rgno, usedlen, rmapbt_sz, refcbt_sz),
 	TP_STRUCT__entry(
 		__field(dev_t, dev)
 		__field(xfs_rgnumber_t, rgno)
 		__field(xfs_rgblock_t, usedlen)
 		__field(xfs_rgblock_t, rmapbt_sz)
+		__field(xfs_rgblock_t, refcbt_sz)
 	),
 	TP_fast_assign(
 		__entry->dev = mp->m_super->s_dev;
 		__entry->rgno = rgno;
 		__entry->usedlen = usedlen;
 		__entry->rmapbt_sz = rmapbt_sz;
+		__entry->refcbt_sz = refcbt_sz;
 	),
-	TP_printk("dev %d:%d rgno 0x%x usedlen %u rmapbt %u",
+	TP_printk("dev %d:%d rgno 0x%x usedlen %u rmapbt %u refcountbt %u",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->rgno,
 		  __entry->usedlen,
-		  __entry->rmapbt_sz)
+		  __entry->rmapbt_sz,
+		  __entry->refcbt_sz)
 );
 #endif /* CONFIG_XFS_RT */
 
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 11697a8b2a1d13..c2a437416227a8 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -399,6 +399,7 @@ xrep_calc_rtgroup_resblks(
 	struct xfs_mount		*mp = sc->mp;
 	struct xfs_scrub_metadata	*sm = sc->sm;
 	uint64_t			usedlen;
+	xfs_extlen_t			refcbt_sz = 0;
 	xfs_extlen_t			rmapbt_sz = 0;
 
 	if (!(sm->sm_flags & XFS_SCRUB_IFLAG_REPAIR))
@@ -411,13 +412,27 @@ xrep_calc_rtgroup_resblks(
 	usedlen = xfs_rtbxlen_to_blen(mp, xfs_rtgroup_extents(mp, sm->sm_agno));
 	ASSERT(usedlen <= XFS_MAX_RGBLOCKS);
 
+	if (xfs_has_reflink(mp))
+		refcbt_sz = xfs_rtrefcountbt_calc_size(mp, usedlen);
+
 	if (xfs_has_rmapbt(mp))
 		rmapbt_sz = xfs_rtrmapbt_calc_size(mp, usedlen);
 
+	/*
+	 * Guess how many blocks we need to rebuild the rmapbt.  For
+	 * non-reflink filesystems we can't have more records than used blocks.
+	 * However, with reflink it's possible to have more than one rmap
+	 * record per rtgroup block.  We don't know how many rmaps there could
+	 * be in the rtgroup, so we start off with what we hope is an generous
+	 * over-estimation.
+	 */
+	if (refcbt_sz > 0 && rmapbt_sz > 0)
+		rmapbt_sz *= 2;
+
 	trace_xrep_calc_rtgroup_resblks_btsize(mp, sm->sm_agno, usedlen,
-			rmapbt_sz);
+			rmapbt_sz, refcbt_sz);
 
-	return rmapbt_sz;
+	return max(rmapbt_sz, refcbt_sz);
 }
 #endif /* CONFIG_XFS_RT */
 

[PATCH 04/11] xfs: don't cross reference rmapbt with bitmaps if they're incomplete

[Darrick J. Wong]

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

LOLLM points out that runtime errors (e.g. ENOMEM) when we're trying to
compute space usag bitmaps are silently dropped by the rmapbt scrubber.
We ought to flag that as an incomplete scrub instead of reporting
cross-referencing errors based on faulty data.

Cc: <stable@vger.kernel.org> # v6.4
Fixes: fed050f3452da0 ("xfs: cross-reference rmap records with ag btrees")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/rmap.c |   20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)


diff --git a/fs/xfs/scrub/rmap.c b/fs/xfs/scrub/rmap.c
index 0cd3eecd2ca5b4..68e2847c962bb2 100644
--- a/fs/xfs/scrub/rmap.c
+++ b/fs/xfs/scrub/rmap.c
@@ -493,11 +493,18 @@ xchk_rmapbt_walk_ag_metadata(
 	 * If there's an error, set XFAIL and disable the bitmap
 	 * cross-referencing checks, but proceed with the scrub anyway.
 	 */
-	if (error)
-		xchk_btree_xref_process_error(sc, sc->sa.rmap_cur,
-				sc->sa.rmap_cur->bc_nlevels - 1, &error);
-	else
-		cr->bitmaps_complete = true;
+	if (error) {
+		if (!xchk_btree_xref_process_error(sc, sc->sa.rmap_cur,
+				sc->sa.rmap_cur->bc_nlevels - 1, &error)) {
+			/* only set incomplete if we didn't set xfail */
+			if (error)
+				xchk_set_incomplete(sc);
+		}
+
+		return 0;
+	}
+
+	cr->bitmaps_complete = true;
 	return 0;
 }
 
@@ -567,7 +574,8 @@ xchk_rmapbt(
 	if (error)
 		goto out;
 
-	xchk_rmapbt_check_bitmaps(sc, cr);
+	if (cr->bitmaps_complete)
+		xchk_rmapbt_check_bitmaps(sc, cr);
 
 out:
 	xagb_bitmap_destroy(&cr->refcbt_owned);

[PATCH 05/11] xfs: don't let memory failures leak blocks and kill repairs

[Darrick J. Wong]

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

LOLLM complains that a memory allocation failure in
xrep_newbt_add_blocks results in online repair leaking blocks that were
previously allocated to write a new btree, but the problem is worse than
that -- a limitation of the codebase is that the callers cannot undo the
transaction /and/ return the error -- either you undo all changes and
commit the transaction, or you error out and the filesystem goes down.

However, the new btree space reservation object isn't that big (~48
bytes).  Let's just do a NOFAIL allocation and the problem goes away.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: be408417630427 ("xfs: implement block reservation accounting for btrees we're staging")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/newbt.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/scrub/newbt.c b/fs/xfs/scrub/newbt.c
index c82f4631fd9c27..584076b2a6eed9 100644
--- a/fs/xfs/scrub/newbt.c
+++ b/fs/xfs/scrub/newbt.c
@@ -193,9 +193,11 @@ xrep_newbt_add_blocks(
 	struct xrep_newbt_resv		*resv;
 	int				error;
 
-	resv = kmalloc_obj(struct xrep_newbt_resv, XCHK_GFP_FLAGS);
-	if (!resv)
-		return -ENOMEM;
+	/*
+	 * We have no way to clean up the allocated space *and* return an
+	 * ENOMEM if we fail to allocate this control structure.
+	 */
+	resv = kmalloc_obj(struct xrep_newbt_resv, GFP_KERNEL | __GFP_NOFAIL);
 
 	INIT_LIST_HEAD(&resv->list);
 	resv->agbno = XFS_FSB_TO_AGBNO(mp, args->fsbno);

[PATCH 06/11] xfs: don't merge different file IO error types

[Darrick J. Wong]

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

LOLLM noticed that we can accidentally merge file range health
monitoring events even if they have different errors.  We shouldn't do
that.

Cc: <stable@vger.kernel.org> # v7.0
Fixes: dfa8bad3a8796c ("xfs: convey file I/O errors to the health monitor")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_healthmon.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_healthmon.c b/fs/xfs/xfs_healthmon.c
index c3749675ef1921..2bdd747f1ead24 100644
--- a/fs/xfs/xfs_healthmon.c
+++ b/fs/xfs/xfs_healthmon.c
@@ -247,7 +247,9 @@ xfs_healthmon_merge_events(
 	case XFS_HEALTHMON_DIOWRITE:
 	case XFS_HEALTHMON_DATALOST:
 		/* logically adjacent file ranges can merge */
-		if (existing->fino != new->fino || existing->fgen != new->fgen)
+		if (existing->fino != new->fino ||
+		    existing->fgen != new->fgen ||
+		    existing->error != new->error)
 			return false;
 
 		if (existing->fpos + existing->flen == new->fpos) {

[PATCH 07/11] xfs: fix blockgc group quota scanning when usrquota isn't enforced

[Darrick J. Wong]

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

LOLLM noticed the copy-paste error here -- if user quotas aren't
enforced but we're near the group quota limit, we fail to set FLAG_GID
and hence we might not actually free any preallocations, causing
unnecessary EDQUOT.  Fix that.

Cc: <stable@vger.kernel.org> # v5.12
Fixes: c237dd7c709432 ("xfs: flush eof/cowblocks if we can't reserve quota for inode creation")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_icache.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 82dac88e3c4c71..de8be344e987d0 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1653,7 +1653,7 @@ xfs_blockgc_free_dquots(
 		do_work = true;
 	}
 
-	if (XFS_IS_UQUOTA_ENFORCED(mp) && gdqp && xfs_dquot_lowsp(gdqp)) {
+	if (XFS_IS_GQUOTA_ENFORCED(mp) && gdqp && xfs_dquot_lowsp(gdqp)) {
 		icw.icw_gid = make_kgid(mp->m_super->s_user_ns, gdqp->q_id);
 		icw.icw_flags |= XFS_ICWALK_FLAG_GID;
 		do_work = true;

[PATCH 08/11] xfs: fix cursor and pointer handling when recovering iunlink buckets

[Darrick J. Wong]

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

LOLLM pointed out a bug in xlog_recover_iunlink_bucket:

1. We don't null out prev_ip after releasing it, which can lead to UAF
   problems if the inodegc flush call in the loop fails.

at which point I noticed even more bugs:

2. If the inodegc flush inside the loop fails, we also leak @ip.

3. We set prev_agino to agino having already advanced agino, which
   results in inodes with i_prev_unlinked set to itself.

4. If we exit the bottom of the loop with prev_ip set, then prev_ip
   aliases ip and we also set its i_prev_unlinked to itself.

Bugs 3 and 4 introduce loops into the unlinked list, though these loops
don't surface because we immediately flush each unlinked inode after
loading it.

Fix all of these issues.

Cc: <stable@vger.kernel.org> # v6.0
Fixes: 04755d2e5821b3 ("xfs: refactor xlog_recover_process_iunlinks()")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_log_recover.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)


diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index e7e49529658be3..cf0d610265fe76 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2736,12 +2736,13 @@ xlog_recover_iunlink_bucket(
 {
 	struct xfs_mount	*mp = pag_mount(pag);
 	struct xfs_inode	*prev_ip = NULL;
-	struct xfs_inode	*ip;
 	xfs_agino_t		prev_agino, agino;
 	int			error = 0;
 
 	agino = be32_to_cpu(agi->agi_unlinked[bucket]);
 	while (agino != NULLAGINO) {
+		struct xfs_inode	*ip;
+
 		error = xfs_iget(mp, NULL, xfs_agino_to_ino(pag, agino), 0, 0,
 				&ip);
 		if (error)
@@ -2750,11 +2751,11 @@ xlog_recover_iunlink_bucket(
 		ASSERT(VFS_I(ip)->i_nlink == 0);
 		ASSERT(VFS_I(ip)->i_mode != 0);
 		xfs_iflags_clear(ip, XFS_IRECOVERY);
-		agino = ip->i_next_unlinked;
 
 		if (prev_ip) {
 			ip->i_prev_unlinked = prev_agino;
 			xfs_irele(prev_ip);
+			prev_ip = NULL;
 
 			/*
 			 * Ensure the inode is removed from the unlinked list
@@ -2766,18 +2767,20 @@ xlog_recover_iunlink_bucket(
 			 * complete.
 			 */
 			error = xfs_inodegc_flush(mp);
-			if (error)
-				break;
+			if (error) {
+				xfs_irele(ip);
+				return error;
+			}
 		}
 
 		prev_agino = agino;
+		agino = ip->i_next_unlinked;
 		prev_ip = ip;
 	}
 
 	if (prev_ip) {
 		int	error2;
 
-		ip->i_prev_unlinked = prev_agino;
 		xfs_irele(prev_ip);
 
 		error2 = xfs_inodegc_flush(mp);

[PATCH 09/11] xfs: drop dquot flush lock when we can't find a buffer to flush

[Darrick J. Wong]

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

LOLLM noticed that xfs_qm_flush_one fails to drop the dquot flush lock
if it can't grab the buffer associated with the dquot.  Since there's no
buffer, nobody else is going to drop the dqflock, so we need to do it
ourselves.

Cc: <stable@vger.kernel.org> # v6.13
Fixes: ca378189fdfa89 ("xfs: convert quotacheck to attach dquot buffers")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_qm.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 99a82107b8e6f2..54d00d543b513a 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1432,16 +1432,22 @@ xfs_qm_flush_one(
 
 	error = xfs_dquot_use_attached_buf(dqp, &bp);
 	if (error)
-		goto out_unlock;
+		goto out_dqflock;
 	if (!bp) {
 		error = -EFSCORRUPTED;
-		goto out_unlock;
+		goto out_dqflock;
 	}
 
 	error = xfs_qm_dqflush(dqp, bp);
 	if (!error)
 		xfs_buf_delwri_queue(bp, buffer_list);
 	xfs_buf_relse(bp);
+	mutex_unlock(&dqp->q_qlock);
+	xfs_qm_dqrele(dqp);
+	return error;
+
+out_dqflock:
+	xfs_dqfunlock(dqp);
 out_unlock:
 	mutex_unlock(&dqp->q_qlock);
 	xfs_qm_dqrele(dqp);

[PATCH 10/11] xfs: don't let hidden_space go negative in xfs_metafile_resv_init

[Darrick J. Wong]

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

LOLLM points out that if the amount of fdblocks that we can reserve for
a metadata btree file goes below the space already used by that file,
then the hidden_space subtraction can underflow, causing
xfs_dec_fdblocks to subtract a huge amount of space.  We never want the
target to be less than the used sapce, so fix the logic that adjusts
dblocks_avail downwards.

Also fix an error in the adjacent comment.

Cc: <stable@vger.kernel.org> # v6.15
Fixes: 1df8d75030b787 ("xfs: make metabtree reservations global")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/libxfs/xfs_metafile.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_metafile.c b/fs/xfs/libxfs/xfs_metafile.c
index 71f004e9dc6451..1f54d39003c27c 100644
--- a/fs/xfs/libxfs/xfs_metafile.c
+++ b/fs/xfs/libxfs/xfs_metafile.c
@@ -297,14 +297,14 @@ xfs_metafile_resv_init(
 		goto out_unlock;
 
 	/*
-	 * Space taken by the per-AG metadata btrees are accounted on-disk as
-	 * used space.  We therefore only hide the space that is reserved but
-	 * not used by the trees.
+	 * Space taken by metadata btrees are accounted on-disk as used space.
+	 * We therefore only hide the space that is reserved but not used by
+	 * the trees.
 	 */
 	if (used > target)
 		target = used;
 	else if (target > dblocks_avail)
-		target = dblocks_avail;
+		target = max(dblocks_avail, used);
 	hidden_space = target - used;
 
 	error = xfs_dec_fdblocks(mp, hidden_space, true);

[PATCH 11/11] xfs: fix wild memcpy access when formatting ondisk rtrefcount btree roots

[Darrick J. Wong]

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

LOLLM noticed that the inode btree root formatting methods copy too many
bytes -- there's only one set of keys in node blocks, not two.  This
causes memory corruption of whatever's beyond the buffers.

Cc: <stable@vger.kernel.org> # v6.14
Fixes: f0415af60f482a ("xfs: wire up a new metafile type for the realtime refcount")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/libxfs/xfs_rtrefcount_btree.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_rtrefcount_btree.c b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
index e2950dbe20681b..dcc89b8e149b05 100644
--- a/fs/xfs/libxfs/xfs_rtrefcount_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
@@ -617,7 +617,7 @@ xfs_rtrefcountbt_from_disk(
 		fpp = xfs_rtrefcount_droot_ptr_addr(dblock, 1, maxrecs);
 		tpp = xfs_rtrefcount_broot_ptr_addr(mp, rblock, 1, rblocklen);
 		numrecs = be16_to_cpu(dblock->bb_numrecs);
-		memcpy(tkp, fkp, 2 * sizeof(*fkp) * numrecs);
+		memcpy(tkp, fkp, sizeof(*fkp) * numrecs);
 		memcpy(tpp, fpp, sizeof(*fpp) * numrecs);
 	} else {
 		frp = xfs_rtrefcount_droot_rec_addr(dblock, 1);
@@ -703,7 +703,7 @@ xfs_rtrefcountbt_to_disk(
 		fpp = xfs_rtrefcount_broot_ptr_addr(mp, rblock, 1, rblocklen);
 		tpp = xfs_rtrefcount_droot_ptr_addr(dblock, 1, maxrecs);
 		numrecs = be16_to_cpu(rblock->bb_numrecs);
-		memcpy(tkp, fkp, 2 * sizeof(*fkp) * numrecs);
+		memcpy(tkp, fkp, sizeof(*fkp) * numrecs);
 		memcpy(tpp, fpp, sizeof(*fpp) * numrecs);
 	} else {
 		frp = xfs_rtrefcount_rec_addr(rblock, 1);

Re: [PATCH 01/11] xfs: always call xfs_qm_adjust_dqlimits during quotacheck repair

[Christoph Hellwig]

On Mon, Sep 14, 2026 at 10:36:49PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM notices that everywhere else in the kernel, a call to
> xfs_qm_adjust_dqlimits precedes every call to xfs_qm_adjust_dqtimers.
> In particular, mount-time quotacheck does this.
> 
> Looking at xfs_qm_adjust_dqlimits, that function is in charge of
> conveying default limits to a dquot if that dquot's limits have been
> zeroed.  That's quite possible in a repair, so let's follow what
> everyone else does and set it.  This requires us to add a new predicate
> to decide if xfs_qm_adjust_dqlimits is likely to reset any of the
> dquot's limits, and to set the dirty flag to true if so.

I'd vote for merging the two functions as they clearly always should
be called together instead.

> A different LOLLM noticed that we have no way to force xchk_commit_dquot
> to call xfs_qm_adjust_dqtimers, so pack that in at the same time.

This feels completely unrelated, and could also use a much more
detailed explanation why we'd want that.

Re: [PATCH 02/11] xfs: call xfs_dquot_set_prealloc_limits if we installed default rtb limits

[Christoph Hellwig]

On Mon, Sep 14, 2026 at 10:37:05PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Now that we have quotas for the realtime volume, we also have
> precomputed watermark limits for the realtime block counts.  These
> precomputations should be done any time we change the rtb limits, which
> means that xfs_qm_adjust_dqlimits needs to ensure that if we installed
> a default rtb limit.

Looks good:

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

Re: [PATCH 03/11] xfs: fix rtgroup repair estimations

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 04/11] xfs: don't cross reference rmapbt with bitmaps if they're incomplete

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 05/11] xfs: don't let memory failures leak blocks and kill repairs

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 06/11] xfs: don't merge different file IO error types

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 07/11] xfs: fix blockgc group quota scanning when usrquota isn't enforced

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 08/11] xfs: fix cursor and pointer handling when recovering iunlink buckets

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 09/11] xfs: drop dquot flush lock when we can't find a buffer to flush

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 10/11] xfs: don't let hidden_space go negative in xfs_metafile_resv_init

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 11/11] xfs: fix wild memcpy access when formatting ondisk rtrefcount btree roots

[Christoph Hellwig]

On Mon, Sep 14, 2026 at 10:39:25PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that the inode btree root formatting methods copy too many
> bytes -- there's only one set of keys in node blocks, not two.  This
> causes memory corruption of whatever's beyond the buffers.

Eeek.

Looks good:

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

Re: [PATCH 01/11] xfs: always call xfs_qm_adjust_dqlimits during quotacheck repair

[Darrick J. Wong]

On Mon, Sep 14, 2026 at 11:25:16PM -0700, Christoph Hellwig wrote:
> On Mon, Sep 14, 2026 at 10:36:49PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM notices that everywhere else in the kernel, a call to
> > xfs_qm_adjust_dqlimits precedes every call to xfs_qm_adjust_dqtimers.
> > In particular, mount-time quotacheck does this.
> > 
> > Looking at xfs_qm_adjust_dqlimits, that function is in charge of
> > conveying default limits to a dquot if that dquot's limits have been
> > zeroed.  That's quite possible in a repair, so let's follow what
> > everyone else does and set it.  This requires us to add a new predicate
> > to decide if xfs_qm_adjust_dqlimits is likely to reset any of the
> > dquot's limits, and to set the dirty flag to true if so.
> 
> I'd vote for merging the two functions as they clearly always should
> be called together instead.

Ok, I'll combine them into an xfs_qm_adjust_dqenforcement() function as
a separate patch...

> > A different LOLLM noticed that we have no way to force xchk_commit_dquot
> > to call xfs_qm_adjust_dqtimers, so pack that in at the same time.
> 
> This feels completely unrelated, and could also use a much more
> detailed explanation why we'd want that.

...and turn this patch into a justification for code that forces dirty =
true.  cem said he'd like to take the other 10 patches from part 15, so
I'll move these two to the start of part 16.

--D

Re: [PATCH 01/11] xfs: always call xfs_qm_adjust_dqlimits during quotacheck repair

[Carlos Maiolino]

On Tue, Sep 15, 2026 at 10:38:20AM -0700, Darrick J. Wong wrote:
> On Mon, Sep 14, 2026 at 11:25:16PM -0700, Christoph Hellwig wrote:
> > On Mon, Sep 14, 2026 at 10:36:49PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > LOLLM notices that everywhere else in the kernel, a call to
> > > xfs_qm_adjust_dqlimits precedes every call to xfs_qm_adjust_dqtimers.
> > > In particular, mount-time quotacheck does this.
> > > 
> > > Looking at xfs_qm_adjust_dqlimits, that function is in charge of
> > > conveying default limits to a dquot if that dquot's limits have been
> > > zeroed.  That's quite possible in a repair, so let's follow what
> > > everyone else does and set it.  This requires us to add a new predicate
> > > to decide if xfs_qm_adjust_dqlimits is likely to reset any of the
> > > dquot's limits, and to set the dirty flag to true if so.
> > 
> > I'd vote for merging the two functions as they clearly always should
> > be called together instead.
> 
> Ok, I'll combine them into an xfs_qm_adjust_dqenforcement() function as
> a separate patch...
> 
> > > A different LOLLM noticed that we have no way to force xchk_commit_dquot
> > > to call xfs_qm_adjust_dqtimers, so pack that in at the same time.
> > 
> > This feels completely unrelated, and could also use a much more
> > detailed explanation why we'd want that.
> 
> ...and turn this patch into a justification for code that forces dirty =
> true.  cem said he'd like to take the other 10 patches from part 15, so
> I'll move these two to the start of part 16.
> 

Yeah, I've got the other 10 patches already and discarded the first one.
Fell free to include that in the next series =]

Cheers

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

[Carlos Maiolino]

On Mon, 14 Sep 2026 22:36:43 -0700, Darrick J. Wong wrote:
> Here's a fifteenth 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!

[01/11] xfs: always call xfs_qm_adjust_dqlimits during quotacheck repair
        (no commit info)
[02/11] xfs: call xfs_dquot_set_prealloc_limits if we installed default rtb limits
        commit: cb2194c0d7b2b913e171df9601f3b7aba6337525
[03/11] xfs: fix rtgroup repair estimations
        commit: bfcb66fe0df3588614662d5e2172586e068ae812
[04/11] xfs: don't cross reference rmapbt with bitmaps if they're incomplete
        commit: 77b4ee1509046e772192d3955142c542aad023ce
[05/11] xfs: don't let memory failures leak blocks and kill repairs
        commit: 06f5d4c2be3b18b6a3eb5f0e1ff4c4a5b420171f
[06/11] xfs: don't merge different file IO error types
        commit: 509688570a42dd42a4eb656d55cf56c285621528
[07/11] xfs: fix blockgc group quota scanning when usrquota isn't enforced
        commit: 4e07c52ca4bd9191e3718bee30f1000333446217
[08/11] xfs: fix cursor and pointer handling when recovering iunlink buckets
        commit: d3bb71b6c6c249139215f2ee33120e2c8005f104
[09/11] xfs: drop dquot flush lock when we can't find a buffer to flush
        commit: 979dbd93efd51c12e386869e6db5df497656a60e
[10/11] xfs: don't let hidden_space go negative in xfs_metafile_resv_init
        commit: d2d0237773deda949c6cc02e001d5c7301214b86
[11/11] xfs: fix wild memcpy access when formatting ondisk rtrefcount btree roots
        commit: 407b04dca6614a77d3c867d5cc71c1ab31768444

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