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

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

[Darrick J. Wong]

Hi all,

Here's a sixteenth 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-16
---
Commits in this patchset:
 * xfs: fix missing xfs_qm_adjust_dqlimits call in quotacheck repair
 * xfs: online quotacheck must dirty dquot if enforcement adjustments needed
 * xfs: fix buffer overruns in xfs_ioc_attr_list
 * xfs: clean up after failed metafile relinking
 * xfs: pass xfs_trans_resv object to reservation calculation helpers
 * xfs: fix xfs_rename_space_res for non-pptr filesystems
 * xfs: fix ondisk symlink target validation in xrep_dinode_check_dfork
 * xfs: fix maximum atomic cow length computation
 * xfs: add missing healthmon trace strings
 * xfarray: don't crash when sorting if array element crosses a folio
 * xfarray: don't allow users to unset in the middle of an array
 * xfs: don't allow sorting sparse arrays
 * xfs: simply the free space btree repair code
 * xfarray: warn against sorting arrays with identical elements
---
 fs/xfs/libxfs/xfs_metadir.h                        |    5 -
 fs/xfs/libxfs/xfs_symlink_remote.h                 |    2 
 fs/xfs/scrub/xfarray.h                             |    9 +
 fs/xfs/xfs_dquot.h                                 |    3 
 fs/xfs/xfs_trace.h                                 |   28 ++++
 .../filesystems/xfs/xfs-online-fsck-design.rst     |   13 --
 fs/xfs/libxfs/xfs_dquot_buf.c                      |   13 --
 fs/xfs/libxfs/xfs_inode_fork.c                     |    4 -
 fs/xfs/libxfs/xfs_metadir.c                        |   30 ++++-
 fs/xfs/libxfs/xfs_symlink_remote.c                 |    8 +
 fs/xfs/libxfs/xfs_trans_resv.c                     |   38 +++---
 fs/xfs/libxfs/xfs_trans_space.c                    |    6 -
 fs/xfs/scrub/alloc_repair.c                        |   15 +-
 fs/xfs/scrub/inode_repair.c                        |   26 ++++
 fs/xfs/scrub/quota_repair.c                        |    5 -
 fs/xfs/scrub/quotacheck_repair.c                   |   54 ++++++++
 fs/xfs/scrub/xfarray.c                             |  133 ++++++--------------
 fs/xfs/xfs_dquot.c                                 |   27 +++-
 fs/xfs/xfs_handle.c                                |    2 
 fs/xfs/xfs_qm.c                                    |    6 -
 fs/xfs/xfs_qm_syscalls.c                           |   17 +--
 fs/xfs/xfs_reflink.c                               |    7 +
 fs/xfs/xfs_trans_dquot.c                           |    6 -
 23 files changed, 252 insertions(+), 205 deletions(-)

[PATCH 01/14] xfs: fix missing xfs_qm_adjust_dqlimits call in quotacheck repair

[Darrick J. Wong]

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

LOLLM noticed 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, but online quotacheck
does not.

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 we actually need to do
that.

However, there's a pre-existing pattern in the kernel -- for non-root
dquots, first we call xfs_qm_adjust_dqlimits to set the dquot's limits
to the defaults if they are zero, and then xfs_qm_adjust_dqtimers to
start grace periods if the dquot's usage is above the softlimit.  The
grace period decision cannot be made correctly if we forget to import
the default limits.

Therefore, let's combine both into a single xfs_qm_adjust_dqenforcement
helper that takes care of both pieces, which fixes quotacheck and makes
it hard to repeat this mistake.

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/xfs_dquot.h               |    3 +--
 fs/xfs/scrub/quota_repair.c      |    5 +----
 fs/xfs/scrub/quotacheck_repair.c |    3 +--
 fs/xfs/xfs_dquot.c               |   27 +++++++++++++++++----------
 fs/xfs/xfs_qm.c                  |    6 +-----
 fs/xfs/xfs_qm_syscalls.c         |   17 +++++++----------
 fs/xfs/xfs_trans_dquot.c         |    6 +-----
 7 files changed, 29 insertions(+), 38 deletions(-)


diff --git a/fs/xfs/xfs_dquot.h b/fs/xfs/xfs_dquot.h
index bbb824adca82ce..326f85d2b4116c 100644
--- a/fs/xfs/xfs_dquot.h
+++ b/fs/xfs/xfs_dquot.h
@@ -204,8 +204,7 @@ void xfs_dquot_to_disk(struct xfs_disk_dquot *ddqp, struct xfs_dquot *dqp);
 void		xfs_qm_dqdestroy(struct xfs_dquot *dqp);
 int		xfs_qm_dqflush(struct xfs_dquot *dqp, struct xfs_buf *bp);
 void		xfs_qm_dqunpin_wait(struct xfs_dquot *dqp);
-void		xfs_qm_adjust_dqtimers(struct xfs_dquot *d);
-void		xfs_qm_adjust_dqlimits(struct xfs_dquot *d);
+void		xfs_qm_adjust_dqenforcement(struct xfs_dquot *d);
 xfs_dqid_t	xfs_qm_id_for_quotatype(struct xfs_inode *ip,
 				xfs_dqtype_t type);
 int		xfs_qm_dqget(struct xfs_mount *mp, xfs_dqid_t id,
diff --git a/fs/xfs/scrub/quota_repair.c b/fs/xfs/scrub/quota_repair.c
index 59302e8afc7ef0..89f7ea4f92ef4a 100644
--- a/fs/xfs/scrub/quota_repair.c
+++ b/fs/xfs/scrub/quota_repair.c
@@ -248,10 +248,7 @@ xrep_quota_item(
 
 	dq->q_flags |= XFS_DQFLAG_DIRTY;
 	xfs_trans_dqjoin(sc->tp, dq);
-	if (dq->q_id) {
-		xfs_qm_adjust_dqlimits(dq);
-		xfs_qm_adjust_dqtimers(dq);
-	}
+	xfs_qm_adjust_dqenforcement(dq);
 	xfs_trans_log_dquot(sc->tp, dq);
 	return xfs_trans_roll(&sc->tp);
 
diff --git a/fs/xfs/scrub/quotacheck_repair.c b/fs/xfs/scrub/quotacheck_repair.c
index dbb522e1513b0b..48ee08df302a42 100644
--- a/fs/xfs/scrub/quotacheck_repair.c
+++ b/fs/xfs/scrub/quotacheck_repair.c
@@ -110,8 +110,7 @@ xqcheck_commit_dquot(
 
 	/* Commit the dirty dquot to disk. */
 	dq->q_flags |= XFS_DQFLAG_DIRTY;
-	if (dq->q_id)
-		xfs_qm_adjust_dqtimers(dq);
+	xfs_qm_adjust_dqenforcement(dq);
 	xfs_trans_log_dquot(xqc->sc->tp, dq);
 	return xrep_trans_commit(xqc->sc);
 
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index e696ee36c2e8d2..56d990eb2d02ba 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -115,18 +115,15 @@ xfs_qm_dqdestroy(
  * We overwrite the dquot limits only if they are zero and this
  * is not the root dquot.
  */
-void
+static void
 xfs_qm_adjust_dqlimits(
 	struct xfs_dquot	*dq)
 {
 	struct xfs_mount	*mp = dq->q_mount;
 	struct xfs_quotainfo	*q = mp->m_quotainfo;
-	struct xfs_def_quota	*defq;
+	struct xfs_def_quota	*defq = xfs_get_defquota(q, xfs_dquot_type(dq));
 	int			prealloc = 0;
 
-	ASSERT(dq->q_id);
-	defq = xfs_get_defquota(q, xfs_dquot_type(dq));
-
 	if (!dq->q_blk.softlimit) {
 		dq->q_blk.softlimit = defq->blk.soft;
 		prealloc = 1;
@@ -207,22 +204,32 @@ xfs_qm_adjust_res_timer(
  * get reset to zero, however, when we find the count to be under
  * the soft limit (they are only ever set non-zero via userspace).
  */
-void
+static void
 xfs_qm_adjust_dqtimers(
 	struct xfs_dquot	*dq)
 {
 	struct xfs_mount	*mp = dq->q_mount;
 	struct xfs_quotainfo	*qi = mp->m_quotainfo;
-	struct xfs_def_quota	*defq;
-
-	ASSERT(dq->q_id);
-	defq = xfs_get_defquota(qi, xfs_dquot_type(dq));
+	struct xfs_def_quota	*defq = xfs_get_defquota(qi, xfs_dquot_type(dq));
 
 	xfs_qm_adjust_res_timer(dq->q_mount, &dq->q_blk, &defq->blk);
 	xfs_qm_adjust_res_timer(dq->q_mount, &dq->q_ino, &defq->ino);
 	xfs_qm_adjust_res_timer(dq->q_mount, &dq->q_rtb, &defq->rtb);
 }
 
+/* Adjust enforcement limits and timers after a change in usage. */
+void
+xfs_qm_adjust_dqenforcement(
+	struct xfs_dquot	*dq)
+{
+	if (dq->q_id == 0)
+		return;
+
+	xfs_qm_adjust_dqlimits(dq);
+	xfs_qm_adjust_dqtimers(dq);
+	dq->q_flags |= XFS_DQFLAG_DIRTY;
+}
+
 /*
  * initialize a buffer full of dquots and log the whole thing
  */
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 54d00d543b513a..008fed8624be2c 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1294,11 +1294,7 @@ xfs_qm_quotacheck_dqadjust(
 	 *
 	 * There are no timers for the default values set in the root dquot.
 	 */
-	if (dqp->q_id) {
-		xfs_qm_adjust_dqlimits(dqp);
-		xfs_qm_adjust_dqtimers(dqp);
-	}
-
+	xfs_qm_adjust_dqenforcement(dqp);
 	dqp->q_flags |= XFS_DQFLAG_DIRTY;
 out_unlock:
 	mutex_unlock(&dqp->q_qlock);
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 21a7849868288f..e2c5f57a7ba86c 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -371,16 +371,13 @@ xfs_qm_scall_setqlim(
 	if (newlim->d_fieldmask & QC_INO_TIMER)
 		xfs_setqlim_timer(mp, res, qlim, newlim->d_ino_timer);
 
-	if (id != 0) {
-		/*
-		 * If the user is now over quota, start the timelimit.
-		 * The user will not be 'warned'.
-		 * Note that we keep the timers ticking, whether enforcement
-		 * is on or off. We don't really want to bother with iterating
-		 * over all ondisk dquots and turning the timers on/off.
-		 */
-		xfs_qm_adjust_dqtimers(dqp);
-	}
+	/*
+	 * If the user is now over quota, start the timelimit.  The user will
+	 * not be 'warned'.  Note that we keep the timers ticking, whether
+	 * enforcement is on or off. We don't really want to bother with
+	 * iterating over all ondisk dquots and turning the timers on/off.
+	 */
+	xfs_qm_adjust_dqenforcement(dqp);
 	dqp->q_flags |= XFS_DQFLAG_DIRTY;
 	xfs_trans_log_dquot(tp, dqp);
 
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index 1606c614f205ae..93ec876792cc76 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -566,11 +566,7 @@ xfs_trans_apply_dquot_deltas(
 			 * Get any default limits in use.
 			 * Start/reset the timer(s) if needed.
 			 */
-			if (dqp->q_id) {
-				xfs_qm_adjust_dqlimits(dqp);
-				xfs_qm_adjust_dqtimers(dqp);
-			}
-
+			xfs_qm_adjust_dqenforcement(dqp);
 			dqp->q_flags |= XFS_DQFLAG_DIRTY;
 			/*
 			 * add this to the list of items to get logged

Re: [PATCH 01/14] xfs: fix missing xfs_qm_adjust_dqlimits call in quotacheck repair

[Christoph Hellwig]

Looks good:

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

[PATCH 02/14] xfs: online quotacheck must dirty dquot if enforcement adjustments needed

[Darrick J. Wong]

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

LOLLM noticed that we have no way to force xchk_commit_dquot to call
xfs_qm_adjust_dqenforcement if nothing else is wrong with the dquot.
Therefore, add a new predicate to force the dirty flag if the dquot has
zero limits and there are default limits; or if the grace period timer
needs adjusting.

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 |   51 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)


diff --git a/fs/xfs/scrub/quotacheck_repair.c b/fs/xfs/scrub/quotacheck_repair.c
index 48ee08df302a42..e2208510939f08 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) {

Re: [PATCH 02/14] xfs: online quotacheck must dirty dquot if enforcement adjustments needed

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 02/14] xfs: online quotacheck must dirty dquot if enforcement adjustments needed

[Darrick J. Wong]

On Sun, Sep 20, 2026 at 11:15:30PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that we have no way to force xchk_commit_dquot to call
> xfs_qm_adjust_dqenforcement if nothing else is wrong with the dquot.
> Therefore, add a new predicate to force the dirty flag if the dquot has
> zero limits and there are default limits; or if the grace period timer
> needs adjusting.
> 
> 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 |   51 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> 
> diff --git a/fs/xfs/scrub/quotacheck_repair.c b/fs/xfs/scrub/quotacheck_repair.c
> index 48ee08df302a42..e2208510939f08 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)

Shashiko says:

"Does this skip repairing missing grace period timers when the default grace
period is configured to 0?
If a quota usage exceeds its limit but has a missing or corrupt timer
(res->timer == 0) and the default grace period (qlim->time) is 0, this
condition evaluates to false. This causes the repair code to skip dirtying
the dquot, bypassing xfs_qm_adjust_dqenforcement().
When the unfixed dquot is subsequently flushed (e.g., via background
writeback or a log force), xfs_qm_dqflush_check() will find count > softlimit
but timer == 0. This is treated as a fatal inconsistency and triggers
SHUTDOWN_CORRUPT_INCORE, effectively turning a reparable condition into a
denial of service."

Yes, that should just be "if (!res->timer) return true;" since we want
to force xfs_qm_adjust_dqtimers to reset it.  I forgot that !qlim->time
just means "expires immediately", not "expires never".

Will fix in next revision.

--D

> +			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) {
> 
> 

[PATCH 03/14] xfs: fix buffer overruns in xfs_ioc_attr_list

[Darrick J. Wong]

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

When we converted the al_offset array in struct xfs_attrlist into a VLA,
the size of the object shrank by 4 bytes.  Unfortunately, the buffer
size validation in the attrlist ioctl wasn't updated to notice this, so
the al_offset[0] assignment blindly writes off the end of the buffer.
LOLLM noticed the omitted check and complained.  Probably should've left
working code alone but for everyone wanting these ***n static checkers.

Cc: <stable@vger.kernel.org> # v6.5
Fixes: 371baf5c9750a2 ("xfs: convert flex-array declarations in struct xfs_attrlist*")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_handle.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_handle.c b/fs/xfs/xfs_handle.c
index 0689cade8f74c2..fd9d4d8258fff2 100644
--- a/fs/xfs/xfs_handle.c
+++ b/fs/xfs/xfs_handle.c
@@ -409,7 +409,7 @@ xfs_ioc_attr_list(
 	void				*buffer;
 	int				error;
 
-	if (bufsize < sizeof(struct xfs_attrlist) ||
+	if (bufsize < struct_size(alist, al_offset, 1) ||
 	    bufsize > XFS_XATTR_LIST_MAX)
 		return -EINVAL;
 

Re: [PATCH 03/14] xfs: fix buffer overruns in xfs_ioc_attr_list

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:15:45PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> When we converted the al_offset array in struct xfs_attrlist into a VLA,
> the size of the object shrank by 4 bytes.  Unfortunately, the buffer
> size validation in the attrlist ioctl wasn't updated to notice this, so
> the al_offset[0] assignment blindly writes off the end of the buffer.
> LOLLM noticed the omitted check and complained.  Probably should've left
> working code alone but for everyone wanting these ***n static checkers.

It's not really static checkers, but fundamental semantics.  Arrays of
size 1 used for VLAs always were a bad idea and should have never been
used.

The fix looks good:

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

Re: [PATCH 03/14] xfs: fix buffer overruns in xfs_ioc_attr_list

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:15:32PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:15:45PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > When we converted the al_offset array in struct xfs_attrlist into a VLA,
> > the size of the object shrank by 4 bytes.  Unfortunately, the buffer
> > size validation in the attrlist ioctl wasn't updated to notice this, so
> > the al_offset[0] assignment blindly writes off the end of the buffer.
> > LOLLM noticed the omitted check and complained.  Probably should've left
> > working code alone but for everyone wanting these ***n static checkers.
> 
> It's not really static checkers, but fundamental semantics.  Arrays of
> size 1 used for VLAs always were a bad idea and should have never been
> used.

Ok I'll just delete the whole sentence.

> The fix looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks!

--D

[PATCH 04/14] xfs: clean up after failed metafile relinking

[Darrick J. Wong]

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

Once we start a metadir update to link in a file, we have to commit or
cancel it, just like any other operation.  LOLLM pointed out that I
forgot that, so fix it.  This fixes a bug in xfs_repair.

However, in commit e80fbe1ad8eff7, we made xfs_metadir_cancel a static
function within xfs_metadir.c, so we can't just add a xfs_metadir_cancel
call to xfs_dqinode_metadir_link.

Instead, create a new xfs_metadir_link_file helper in xfs_metadir.c that
takes only the xfs_metadir_update object, and handles everything from
start to finish.  This enables us to make xfs_metadir_commit a static
function too.

Cc: <stable@vger.kernel.org> # v6.13
Fixes: e80fbe1ad8eff7 ("xfs: use metadir for quota inodes")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_metadir.h   |    5 +----
 fs/xfs/libxfs/xfs_dquot_buf.c |   13 +------------
 fs/xfs/libxfs/xfs_metadir.c   |   30 +++++++++++++++++++++++++++---
 3 files changed, 29 insertions(+), 19 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_metadir.h b/fs/xfs/libxfs/xfs_metadir.h
index e434b9d1c93200..b64f9fc5ca7867 100644
--- a/fs/xfs/libxfs/xfs_metadir.h
+++ b/fs/xfs/libxfs/xfs_metadir.h
@@ -38,10 +38,7 @@ int xfs_metadir_create_file(struct xfs_metadir_update *upd, umode_t mode,
 		xfs_metadir_createfn create, void *priv,
 		struct xfs_inode **ipp);
 
-int xfs_metadir_start_link(struct xfs_metadir_update *upd);
-int xfs_metadir_link(struct xfs_metadir_update *upd);
-
-int xfs_metadir_commit(struct xfs_metadir_update *upd);
+int xfs_metadir_link_file(struct xfs_metadir_update *upd);
 
 int xfs_metadir_mkdir(struct xfs_inode *dp, const char *path,
 		struct xfs_inode **ipp);
diff --git a/fs/xfs/libxfs/xfs_dquot_buf.c b/fs/xfs/libxfs/xfs_dquot_buf.c
index 77954d1d924cc3..6c8a9afdfb1c0f 100644
--- a/fs/xfs/libxfs/xfs_dquot_buf.c
+++ b/fs/xfs/libxfs/xfs_dquot_buf.c
@@ -454,19 +454,8 @@ xfs_dqinode_metadir_link(
 		.path			= xfs_dqinode_path(type),
 		.ip			= ip,
 	};
-	int				error;
 
-	error = xfs_metadir_start_link(&upd);
-	if (error)
-		return error;
-
-	error = xfs_metadir_link(&upd);
-	if (error)
-		return error;
-
-	xfs_trans_log_inode(upd.tp, upd.ip, XFS_ILOG_CORE);
-
-	return xfs_metadir_commit(&upd);
+	return xfs_metadir_link_file(&upd);
 }
 #endif /* __KERNEL__ */
 
diff --git a/fs/xfs/libxfs/xfs_metadir.c b/fs/xfs/libxfs/xfs_metadir.c
index 7c6b086b73db61..1ff26e55b1864f 100644
--- a/fs/xfs/libxfs/xfs_metadir.c
+++ b/fs/xfs/libxfs/xfs_metadir.c
@@ -317,7 +317,7 @@ xfs_metadir_create(
  * Begin the process of linking a metadata file by allocating transactions
  * and locking whatever resources we're going to need.
  */
-int
+static int
 xfs_metadir_start_link(
 	struct xfs_metadir_update	*upd)
 {
@@ -364,7 +364,7 @@ xfs_metadir_start_link(
  * The path (up to the final component) must already exist, but the final
  * component must not already exist.
  */
-int
+static int
 xfs_metadir_link(
 	struct xfs_metadir_update	*upd)
 {
@@ -409,7 +409,7 @@ xfs_metadir_link(
 #endif /* ! __KERNEL__ */
 
 /* Commit a metadir update and unlock/drop all resources. */
-int
+static int
 xfs_metadir_commit(
 	struct xfs_metadir_update	*upd)
 {
@@ -499,3 +499,27 @@ xfs_metadir_mkdir(
 
 	return xfs_metadir_create_file(&upd, S_IFDIR, NULL, NULL, ipp);
 }
+
+#ifndef __KERNEL__
+/* Link a metadata file into a metadata directory. */
+int
+xfs_metadir_link_file(
+	struct xfs_metadir_update	*upd)
+{
+	int				error;
+
+	error = xfs_metadir_start_link(upd);
+	if (error)
+		return error;
+
+	error = xfs_metadir_link(upd);
+	if (error) {
+		xfs_metadir_cancel(upd, error);
+		return error;
+	}
+
+	xfs_trans_log_inode(upd->tp, upd->ip, XFS_ILOG_CORE);
+
+	return xfs_metadir_commit(upd);
+}
+#endif /* ! __KERNEL__ */

Re: [PATCH 04/14] xfs: clean up after failed metafile relinking

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:16:01PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Once we start a metadir update to link in a file, we have to commit or
> cancel it, just like any other operation.  LOLLM pointed out that I
> forgot that, so fix it.  This fixes a bug in xfs_repair.
> 
> However, in commit e80fbe1ad8eff7, we made xfs_metadir_cancel a static
> function within xfs_metadir.c, so we can't just add a xfs_metadir_cancel
> call to xfs_dqinode_metadir_link.
> 
> Instead, create a new xfs_metadir_link_file helper in xfs_metadir.c that
> takes only the xfs_metadir_update object, and handles everything from
> start to finish.  This enables us to make xfs_metadir_commit a static
> function too.
> 
> Cc: <stable@vger.kernel.org> # v6.13
> Fixes: e80fbe1ad8eff7 ("xfs: use metadir for quota inodes

AFAIK all this isn not used at all in the kernel, so a stable tag
feels a bit odd.

Otherwise looks good:

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

Re: [PATCH 04/14] xfs: clean up after failed metafile relinking

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:17:11PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:16:01PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Once we start a metadir update to link in a file, we have to commit or
> > cancel it, just like any other operation.  LOLLM pointed out that I
> > forgot that, so fix it.  This fixes a bug in xfs_repair.
> > 
> > However, in commit e80fbe1ad8eff7, we made xfs_metadir_cancel a static
> > function within xfs_metadir.c, so we can't just add a xfs_metadir_cancel
> > call to xfs_dqinode_metadir_link.
> > 
> > Instead, create a new xfs_metadir_link_file helper in xfs_metadir.c that
> > takes only the xfs_metadir_update object, and handles everything from
> > start to finish.  This enables us to make xfs_metadir_commit a static
> > function too.
> > 
> > Cc: <stable@vger.kernel.org> # v6.13
> > Fixes: e80fbe1ad8eff7 ("xfs: use metadir for quota inodes
> 
> AFAIK all this isn not used at all in the kernel, so a stable tag
> feels a bit odd.

Will remove.

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

Thanks!

--D

[PATCH 05/14] xfs: pass xfs_trans_resv object to reservation calculation helpers

[Darrick J. Wong]

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

xfs_calc_namespace_reservations computes the directory tree related
transaction reservations for a given xfs_trans_resv object.  The helpers
it relies on, however, read the live one from the xfs_mount even if
we're doing this for minlogsize calculations.  In practice this
shouldn't be a big deal since the minlogsize and live reservation
objects don't differ in a meaningful way, but LOLLM complained about the
inconsistency so let's fix it anyway.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 7dba4a5fe1c5cd ("xfs: extend transaction reservations for parent attributes")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/libxfs/xfs_trans_resv.c |   38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index 3151e97ca8ff6d..1c20a7c27aa1bf 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -606,10 +606,10 @@ static inline unsigned int xfs_calc_pptr_replace_overhead(void)
  */
 STATIC uint
 xfs_calc_rename_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
 	unsigned int		overhead = XFS_DQUOT_LOGRES;
-	struct xfs_trans_resv	*resp = M_RES(mp);
 	unsigned int		t1, t2, t3 = 0;
 
 	t1 = xfs_calc_inode_res(mp, 5) +
@@ -715,10 +715,10 @@ xfs_link_log_count(
  */
 STATIC uint
 xfs_calc_link_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
 	unsigned int		overhead = XFS_DQUOT_LOGRES;
-	struct xfs_trans_resv	*resp = M_RES(mp);
 	unsigned int		t1, t2, t3 = 0;
 
 	overhead += xfs_calc_iunlink_remove_reservation(mp);
@@ -777,10 +777,10 @@ xfs_remove_log_count(
  */
 STATIC uint
 xfs_calc_remove_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
 	unsigned int            overhead = XFS_DQUOT_LOGRES;
-	struct xfs_trans_resv   *resp = M_RES(mp);
 	unsigned int            t1, t2, t3 = 0;
 
 	overhead += xfs_calc_iunlink_add_reservation(mp);
@@ -862,9 +862,9 @@ xfs_icreate_log_count(
 
 STATIC uint
 xfs_calc_icreate_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
-	struct xfs_trans_resv	*resp = M_RES(mp);
 	unsigned int		overhead = XFS_DQUOT_LOGRES;
 	unsigned int		t1, t2, t3 = 0;
 
@@ -911,9 +911,10 @@ xfs_mkdir_log_count(
  */
 STATIC uint
 xfs_calc_mkdir_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
-	return xfs_calc_icreate_reservation(mp);
+	return xfs_calc_icreate_reservation(mp, resp);
 }
 
 static inline unsigned int
@@ -940,9 +941,10 @@ xfs_symlink_log_count(
  */
 STATIC uint
 xfs_calc_symlink_reservation(
-	struct xfs_mount	*mp)
+	struct xfs_mount	*mp,
+	struct xfs_trans_resv	*resp)
 {
-	return xfs_calc_icreate_reservation(mp) +
+	return xfs_calc_icreate_reservation(mp, resp) +
 	       xfs_calc_buf_res(1, XFS_SYMLINK_MAXLEN);
 }
 
@@ -1265,27 +1267,27 @@ xfs_calc_namespace_reservations(
 {
 	ASSERT(resp->tr_attrsetm.tr_logres > 0);
 
-	resp->tr_rename.tr_logres = xfs_calc_rename_reservation(mp);
+	resp->tr_rename.tr_logres = xfs_calc_rename_reservation(mp, resp);
 	resp->tr_rename.tr_logcount = xfs_rename_log_count(mp, resp);
 	resp->tr_rename.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
-	resp->tr_link.tr_logres = xfs_calc_link_reservation(mp);
+	resp->tr_link.tr_logres = xfs_calc_link_reservation(mp, resp);
 	resp->tr_link.tr_logcount = xfs_link_log_count(mp, resp);
 	resp->tr_link.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
-	resp->tr_remove.tr_logres = xfs_calc_remove_reservation(mp);
+	resp->tr_remove.tr_logres = xfs_calc_remove_reservation(mp, resp);
 	resp->tr_remove.tr_logcount = xfs_remove_log_count(mp, resp);
 	resp->tr_remove.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
-	resp->tr_symlink.tr_logres = xfs_calc_symlink_reservation(mp);
+	resp->tr_symlink.tr_logres = xfs_calc_symlink_reservation(mp, resp);
 	resp->tr_symlink.tr_logcount = xfs_symlink_log_count(mp, resp);
 	resp->tr_symlink.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
-	resp->tr_create.tr_logres = xfs_calc_icreate_reservation(mp);
+	resp->tr_create.tr_logres = xfs_calc_icreate_reservation(mp, resp);
 	resp->tr_create.tr_logcount = xfs_icreate_log_count(mp, resp);
 	resp->tr_create.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 
-	resp->tr_mkdir.tr_logres = xfs_calc_mkdir_reservation(mp);
+	resp->tr_mkdir.tr_logres = xfs_calc_mkdir_reservation(mp, resp);
 	resp->tr_mkdir.tr_logcount = xfs_mkdir_log_count(mp, resp);
 	resp->tr_mkdir.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
 }

Re: [PATCH 05/14] xfs: pass xfs_trans_resv object to reservation calculation helpers

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:16:17PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> xfs_calc_namespace_reservations computes the directory tree related
> transaction reservations for a given xfs_trans_resv object.  The helpers
> it relies on, however, read the live one from the xfs_mount even if
> we're doing this for minlogsize calculations.  In practice this
> shouldn't be a big deal since the minlogsize and live reservation
> objects don't differ in a meaningful way, but LOLLM complained about the
> inconsistency so let's fix it anyway.
> 
> Cc: <stable@vger.kernel.org> # v6.10

Is this really a stable candidate?  Same for the current merge windos,
at some point we need to split these fixes into those that could
cause real issues and those who don't to not flood later -rcs with
lots of updates.  And this looks like a very clear candidate to not
rush.

Otherwise looks good:

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

Re: [PATCH 05/14] xfs: pass xfs_trans_resv object to reservation calculation helpers

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:18:52PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:16:17PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > xfs_calc_namespace_reservations computes the directory tree related
> > transaction reservations for a given xfs_trans_resv object.  The helpers
> > it relies on, however, read the live one from the xfs_mount even if
> > we're doing this for minlogsize calculations.  In practice this
> > shouldn't be a big deal since the minlogsize and live reservation
> > objects don't differ in a meaningful way, but LOLLM complained about the
> > inconsistency so let's fix it anyway.
> > 
> > Cc: <stable@vger.kernel.org> # v6.10
> 
> Is this really a stable candidate?  Same for the current merge windos,
> at some point we need to split these fixes into those that could
> cause real issues and those who don't to not flood later -rcs with
> lots of updates.  And this looks like a very clear candidate to not
> rush.

I asked cem if we could just queue them all for 7.4 this morning and it
sounded like he was ok with that, what with ALPS and LPC coming up soon
anyway.

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

Thanks!

--D

Re: [PATCH 05/14] xfs: pass xfs_trans_resv object to reservation calculation helpers

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:18:52PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:16:17PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > xfs_calc_namespace_reservations computes the directory tree related
> > transaction reservations for a given xfs_trans_resv object.  The helpers
> > it relies on, however, read the live one from the xfs_mount even if
> > we're doing this for minlogsize calculations.  In practice this
> > shouldn't be a big deal since the minlogsize and live reservation
> > objects don't differ in a meaningful way, but LOLLM complained about the
> > inconsistency so let's fix it anyway.
> > 
> > Cc: <stable@vger.kernel.org> # v6.10
> 
> Is this really a stable candidate?  Same for the current merge windos,
> at some point we need to split these fixes into those that could
> cause real issues and those who don't to not flood later -rcs with
> lots of updates.  And this looks like a very clear candidate to not
> rush.

<nod> I'll drop this trailer then.

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

Thanks!

--D

[PATCH 06/14] xfs: fix xfs_rename_space_res for non-pptr filesystems

[Darrick J. Wong]

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

We don't need to reserve space for a parent pointer update for an
existing target if parent pointers are disabled.  Fix this regression
(which LOLLM noticed) so that rename reservations go back to what they
were before parent pointers.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: 5a8338c88284df ("xfs: Add parent pointers to rename")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/libxfs/xfs_trans_space.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_trans_space.c b/fs/xfs/libxfs/xfs_trans_space.c
index c4cd547033e584..7edd0d86f0bdb2 100644
--- a/fs/xfs/libxfs/xfs_trans_space.c
+++ b/fs/xfs/libxfs/xfs_trans_space.c
@@ -127,10 +127,10 @@ xfs_rename_space_res(
 		if (has_whiteout)
 			ret += xfs_parent_calc_space_res(mp, src_namelen);
 		ret += 2 * xfs_parent_calc_space_res(mp, target_namelen);
+
+		if (target_exists)
+			ret += xfs_parent_calc_space_res(mp, target_namelen);
 	}
 
-	if (target_exists)
-		ret += xfs_parent_calc_space_res(mp, target_namelen);
-
 	return ret;
 }

Re: [PATCH 06/14] xfs: fix xfs_rename_space_res for non-pptr filesystems

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:16:32PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> We don't need to reserve space for a parent pointer update for an
> existing target if parent pointers are disabled.  Fix this regression
> (which LOLLM noticed) so that rename reservations go back to what they
> were before parent pointers.
> 
> Cc: <stable@vger.kernel.org> # v6.10
> Fixes: 5a8338c88284df ("xfs: Add parent pointers to rename")

This just relaxed a reservation.  I don't think this is a stable/7.3
candidate and probably should not have a fixes tag that causes folks
to backport it.

Otherwise looks good:

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

Re: [PATCH 06/14] xfs: fix xfs_rename_space_res for non-pptr filesystems

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:20:01PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:16:32PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > We don't need to reserve space for a parent pointer update for an
> > existing target if parent pointers are disabled.  Fix this regression
> > (which LOLLM noticed) so that rename reservations go back to what they
> > were before parent pointers.
> > 
> > Cc: <stable@vger.kernel.org> # v6.10
> > Fixes: 5a8338c88284df ("xfs: Add parent pointers to rename")
> 
> This just relaxed a reservation.  I don't think this is a stable/7.3
> candidate and probably should not have a fixes tag that causes folks
> to backport it.

Yes it relaxes a reservation, but only for non-parent pointers
filesystems running on newer kernels.  IOWs, it reduces the reservation
back to what it was before parent pointers, so it actually has some
performance implications for old filesystems.

OTOH nobody's complained about the dip in performance so <shrug> I don't
really care that much either way.

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

Thanks!

--D

[PATCH 07/14] xfs: fix ondisk symlink target validation in xrep_dinode_check_dfork

[Darrick J. Wong]

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

LOLLM noticed that online repair of a broken symlink file could fail
unnecessarily if a local-format symlink target isn't null terminated.
The ondisk target isn't required to be null terminated, but repair
enforces that anyway because it uses the validator for the incore
symlink target.  (The incore buffer is always null-terminated).  Fix
this by reverting the changes to xfs_symlink_shortform_verify and adding
an ondisk-specific helper in inode_repair.c.

Cc: <stable@vger.kernel.org> # v6.8
Fixes: e744cef2060559 ("xfs: zap broken inode forks")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/libxfs/xfs_symlink_remote.h |    2 +-
 fs/xfs/libxfs/xfs_inode_fork.c     |    4 +---
 fs/xfs/libxfs/xfs_symlink_remote.c |    8 ++++++--
 fs/xfs/scrub/inode_repair.c        |   26 +++++++++++++++++++++++++-
 4 files changed, 33 insertions(+), 7 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_symlink_remote.h b/fs/xfs/libxfs/xfs_symlink_remote.h
index c1672fe1f17bb2..3f6590602473d0 100644
--- a/fs/xfs/libxfs/xfs_symlink_remote.h
+++ b/fs/xfs/libxfs/xfs_symlink_remote.h
@@ -18,7 +18,7 @@ bool xfs_symlink_hdr_ok(xfs_ino_t ino, uint32_t offset,
 void xfs_symlink_local_to_remote(struct xfs_trans *tp, struct xfs_buf *bp,
 				 struct xfs_inode *ip, struct xfs_ifork *ifp,
 				 void *priv);
-xfs_failaddr_t xfs_symlink_shortform_verify(void *sfp, int64_t size);
+xfs_failaddr_t xfs_symlink_shortform_verify(struct xfs_inode *ip);
 int xfs_symlink_remote_read(struct xfs_inode *ip, char *link);
 int xfs_symlink_write_target(struct xfs_trans *tp, struct xfs_inode *ip,
 		xfs_ino_t owner, const char *target_path, int pathlen,
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 606a36526ce245..486fe7ab8b6110 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -683,9 +683,7 @@ xfs_ifork_verify_local_data(
 		break;
 	}
 	case S_IFLNK: {
-		struct xfs_ifork	*ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK);
-
-		fa = xfs_symlink_shortform_verify(ifp->if_data, ifp->if_bytes);
+		fa = xfs_symlink_shortform_verify(ip);
 		break;
 	}
 	default:
diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
index b0dc3888bf1b40..0201a3d59b1a24 100644
--- a/fs/xfs/libxfs/xfs_symlink_remote.c
+++ b/fs/xfs/libxfs/xfs_symlink_remote.c
@@ -208,11 +208,15 @@ xfs_symlink_local_to_remote(
  */
 xfs_failaddr_t
 xfs_symlink_shortform_verify(
-	void			*sfp,
-	int64_t			size)
+	struct xfs_inode	*ip)
 {
+	struct xfs_ifork	*ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK);
+	char			*sfp = (char *)ifp->if_data;
+	int			size = ifp->if_bytes;
 	char			*endp = sfp + size;
 
+	ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
+
 	/*
 	 * Zero length symlinks should never occur in memory as they are
 	 * never allowed to exist on disk.
diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index b87c2214623383..fab4015f6a9506 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -1024,6 +1024,30 @@ xrep_dinode_bad_metabt_fork(
 	return false;
 }
 
+static xfs_failaddr_t
+xrep_symlink_shortform_verify(
+	void			*sfp,
+	int64_t			size)
+{
+	/*
+	 * Zero length symlinks should never occur in memory as they are
+	 * never allowed to exist on disk.
+	 */
+	if (!size)
+		return __this_address;
+
+	/* No negative sizes or overly long symlink targets. */
+	if (size < 0 || size > XFS_SYMLINK_MAXLEN)
+		return __this_address;
+
+	/* No NULLs in the target either. */
+	if (memchr(sfp, 0, size))
+		return __this_address;
+
+	/* ondisk symlink target isn't null terminated, unlike incore */
+	return NULL;
+}
+
 /*
  * Check the data fork for things that will fail the ifork verifiers or the
  * ifork formatters.
@@ -1099,7 +1123,7 @@ xrep_dinode_check_dfork(
 			return true;
 		/* symlink structure must pass verification. */
 		if (S_ISLNK(mode) &&
-		    xfs_symlink_shortform_verify(dfork_ptr, data_size) != NULL)
+		    xrep_symlink_shortform_verify(dfork_ptr, data_size) != NULL)
 			return true;
 		break;
 	case XFS_DINODE_FMT_EXTENTS:

Re: [PATCH 07/14] xfs: fix ondisk symlink target validation in xrep_dinode_check_dfork

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 07/14] xfs: fix ondisk symlink target validation in xrep_dinode_check_dfork

[Dave Chinner]

On Sun, Sep 20, 2026 at 11:16:48PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that online repair of a broken symlink file could fail
> unnecessarily if a local-format symlink target isn't null terminated.
> The ondisk target isn't required to be null terminated, but repair
> enforces that anyway because it uses the validator for the incore
> symlink target.  (The incore buffer is always null-terminated).  Fix
> this by reverting the changes to xfs_symlink_shortform_verify and adding
> an ondisk-specific helper in inode_repair.c.
> 
> Cc: <stable@vger.kernel.org> # v6.8
> Fixes: e744cef2060559 ("xfs: zap broken inode forks")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---
>  fs/xfs/libxfs/xfs_symlink_remote.h |    2 +-
>  fs/xfs/libxfs/xfs_inode_fork.c     |    4 +---
>  fs/xfs/libxfs/xfs_symlink_remote.c |    8 ++++++--
>  fs/xfs/scrub/inode_repair.c        |   26 +++++++++++++++++++++++++-
>  4 files changed, 33 insertions(+), 7 deletions(-)

Just tripped over this independently looking at local format symlink
verification for inode log item recovery. Fix looks good.

Reviewed-by: Dave Chinner <dgc@kernel.org>

-- 
Dave Chinner
dgc@kernel.org

[PATCH 08/14] xfs: fix maximum atomic cow length computation

[Darrick J. Wong]

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

LOLLM says rounddown_pow_of_two acts funny if you pass it 0, so we need
to catch this possibility because maybe the filesystem doesn't support
software atomic writes at all.

Cc: <stable@vger.kernel.org> # v6.16
Fixes: 0c438dcc31504b ("xfs: add xfs_calc_atomic_write_unit_max()")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/xfs_reflink.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 4801361366359b..489ecd4cf10aff 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -1081,6 +1081,8 @@ xfs_extlen_t
 xfs_reflink_max_atomic_cow(
 	struct xfs_mount	*mp)
 {
+	xfs_extlen_t		fsb;
+
 	/* We cannot do any atomic writes without out of place writes. */
 	if (!xfs_can_sw_atomic_write(mp))
 		return 0;
@@ -1089,7 +1091,10 @@ xfs_reflink_max_atomic_cow(
 	 * Atomic write limits must always be a power-of-2, according to
 	 * generic_atomic_write_valid.
 	 */
-	return rounddown_pow_of_two(xfs_calc_max_atomic_write_fsblocks(mp));
+	fsb = xfs_calc_max_atomic_write_fsblocks(mp);
+	if (!fsb)
+		return 0;
+	return rounddown_pow_of_two(fsb);
 }
 
 /*

Re: [PATCH 08/14] xfs: fix maximum atomic cow length computation

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:17:04PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM says rounddown_pow_of_two acts funny if you pass it 0, so we need
> to catch this possibility because maybe the filesystem doesn't support
> software atomic writes at all.

Can this happen?  Shouldn't the xfs_can_sw_atomic_write() above guard
against this?

Re: [PATCH 08/14] xfs: fix maximum atomic cow length computation

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:21:21PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:17:04PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM says rounddown_pow_of_two acts funny if you pass it 0, so we need
> > to catch this possibility because maybe the filesystem doesn't support
> > software atomic writes at all.
> 
> Can this happen?  Shouldn't the xfs_can_sw_atomic_write() above guard
> against this?

Hmm.  Yes it can.  I think I'll drop this one.

--D

[PATCH 10/14] xfarray: don't crash when sorting if array element crosses a folio

[Darrick J. Wong]

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

LOLLM points out that if an array element crosses a folio boundary,
xfile_get_folio returns a NULL folio pointer.  If this happens,
si->folio is also set to NULL, and calling folio_pos/folio_address will
just crash the kernel.  Teach this function to handle this condition by
falling back to reading the array element into scratchpad memory.

Cc: <stable@vger.kernel.org> # v6.6
Fixes: cf36f4f64c2d4e ("xfs: cache pages used for xfarray quicksort convergence")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 fs/xfs/scrub/xfarray.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)


diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
index 2ce24bfe4c0fab..30a58e9d4378e4 100644
--- a/fs/xfs/scrub/xfarray.c
+++ b/fs/xfs/scrub/xfarray.c
@@ -830,22 +830,24 @@ xfarray_sort_scan(
 			return PTR_ERR(folio);
 		si->folio = folio;
 
-		si->first_folio_idx = xfarray_idx(si->array,
-				folio_pos(si->folio) + si->array->obj_size - 1);
+		if (si->folio) {
+			si->first_folio_idx = xfarray_idx(si->array,
+					folio_pos(si->folio) + si->array->obj_size - 1);
 
-		next_pos = folio_next_pos(si->folio);
-		si->last_folio_idx = xfarray_idx(si->array, next_pos - 1);
-		if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)
-			si->last_folio_idx--;
+			next_pos = folio_next_pos(si->folio);
+			si->last_folio_idx = xfarray_idx(si->array, next_pos - 1);
+			if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)
+				si->last_folio_idx--;
 
-		trace_xfarray_sort_scan(si, idx);
+			trace_xfarray_sort_scan(si, idx);
+		}
 	}
 
 	/*
 	 * If this folio still doesn't cover the desired element, it must cross
 	 * a folio boundary.  Read into the scratchpad and we're done.
 	 */
-	if (idx < si->first_folio_idx || idx > si->last_folio_idx) {
+	if (!si->folio || idx < si->first_folio_idx || idx > si->last_folio_idx) {
 		void		*temp = xfarray_scratch(si->array);
 
 		error = xfile_load(si->array->xfile, temp, si->array->obj_size,

Re: [PATCH 10/14] xfarray: don't crash when sorting if array element crosses a folio

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:17:35PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM points out that if an array element crosses a folio boundary,
> xfile_get_folio returns a NULL folio pointer.  If this happens,
> si->folio is also set to NULL, and calling folio_pos/folio_address will
> just crash the kernel.  Teach this function to handle this condition by
> falling back to reading the array element into scratchpad memory.
> 
> Cc: <stable@vger.kernel.org> # v6.6
> Fixes: cf36f4f64c2d4e ("xfs: cache pages used for xfarray quicksort convergence")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---
>  fs/xfs/scrub/xfarray.c |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> 
> diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
> index 2ce24bfe4c0fab..30a58e9d4378e4 100644
> --- a/fs/xfs/scrub/xfarray.c
> +++ b/fs/xfs/scrub/xfarray.c
> @@ -830,22 +830,24 @@ xfarray_sort_scan(
>  			return PTR_ERR(folio);
>  		si->folio = folio;
>  
> -		si->first_folio_idx = xfarray_idx(si->array,
> -				folio_pos(si->folio) + si->array->obj_size - 1);
> +		if (si->folio) {
> +			si->first_folio_idx = xfarray_idx(si->array,
> +					folio_pos(si->folio) + si->array->obj_size - 1);

Overly long line.

> +			if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)

Another one.

> +	if (!si->folio || idx < si->first_folio_idx || idx > si->last_folio_idx) {

And one more.

Re: [PATCH 10/14] xfarray: don't crash when sorting if array element crosses a folio

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:22:38PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:17:35PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM points out that if an array element crosses a folio boundary,
> > xfile_get_folio returns a NULL folio pointer.  If this happens,
> > si->folio is also set to NULL, and calling folio_pos/folio_address will
> > just crash the kernel.  Teach this function to handle this condition by
> > falling back to reading the array element into scratchpad memory.
> > 
> > Cc: <stable@vger.kernel.org> # v6.6
> > Fixes: cf36f4f64c2d4e ("xfs: cache pages used for xfarray quicksort convergence")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > Assisted-by: LOLLM # finding obvious bugs
> > ---
> >  fs/xfs/scrub/xfarray.c |   18 ++++++++++--------
> >  1 file changed, 10 insertions(+), 8 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/scrub/xfarray.c b/fs/xfs/scrub/xfarray.c
> > index 2ce24bfe4c0fab..30a58e9d4378e4 100644
> > --- a/fs/xfs/scrub/xfarray.c
> > +++ b/fs/xfs/scrub/xfarray.c
> > @@ -830,22 +830,24 @@ xfarray_sort_scan(
> >  			return PTR_ERR(folio);
> >  		si->folio = folio;
> >  
> > -		si->first_folio_idx = xfarray_idx(si->array,
> > -				folio_pos(si->folio) + si->array->obj_size - 1);
> > +		if (si->folio) {
> > +			si->first_folio_idx = xfarray_idx(si->array,
> > +					folio_pos(si->folio) + si->array->obj_size - 1);
> 
> Overly long line.
> 
> > +			if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)
> 
> Another one.
> 
> > +	if (!si->folio || idx < si->first_folio_idx || idx > si->last_folio_idx) {
> 
> And one more.

On second glance, I think a cleaner way to fix this is to move the logic
that loads the folio, computes the new {first,last}_folio_idx, and
validates them into a new helper:

static int
xfarray_sort_load_folio(
	struct xfarray_sortinfo	*si,
	xfarray_idx_t		idx,
	loff_t			idx_pos)
{
	struct folio		*folio;
	loff_t			next_pos;

	folio = xfile_get_folio(si->array->xfile, idx_pos, si->array->obj_size,
			XFILE_ALLOC);
	if (IS_ERR(folio))
		return PTR_ERR(folio);
	si->folio = folio;

	/* No folio?  Get the caller to read into the scratchpad. */
	if (!si->folio)
		return 0;

	si->first_folio_idx = xfarray_idx(si->array,
			folio_pos(si->folio) + si->array->obj_size - 1);

	next_pos = folio_next_pos(si->folio);
	si->last_folio_idx = xfarray_idx(si->array, next_pos - 1);
	if (xfarray_pos(si->array, si->last_folio_idx + 1) > next_pos)
		si->last_folio_idx--;

	/*
	 * If this folio still doesn't cover the desired element, it must cross
	 * a folio boundary.  Get the caller to read into the scratchpad.
	 */
	if (idx < si->first_folio_idx || idx > si->last_folio_idx) {
		xfarray_sort_scan_done(si);
		return 0;
	}

	trace_xfarray_sort_scan(si, idx);
	return 0;
}

--D

Re: [PATCH 10/14] xfarray: don't crash when sorting if array element crosses a folio

[Christoph Hellwig]

On Tue, Sep 22, 2026 at 10:53:13AM -0700, Darrick J. Wong wrote:
> On second glance, I think a cleaner way to fix this is to move the logic
> that loads the folio, computes the new {first,last}_folio_idx, and
> validates them into a new helper:

Heh, I was about to suggest a new helper, but didn't feel like I'd
want to cause you too much work..