Re: [PATCH 03/10] xfs: don't validate error tags in the I/O path

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jan 27, 2026 at 05:05:43PM +0100, Christoph Hellwig wrote:
> We can trust XFS developers enough to not pass random stuff to
> XFS_ERROR_TEST/DELAY.  Open code the validity check in xfs_errortag_add,
> which is the only place that receives unvalidated error tag values from
> user space, and drop the now pointless xfs_errortag_enabled helper.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

That seems reasonable; these are debug knobs after all.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_errortag.h |  2 +-
>  fs/xfs/xfs_error.c           | 38 ++++++++++--------------------------
>  fs/xfs/xfs_error.h           |  2 +-
>  3 files changed, 12 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index 57e47077c75a..b7d98471684b 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -53,7 +53,7 @@
>   * Drop-writes support removed because write error handling cannot trash
>   * pre-existing delalloc extents in any useful way anymore. We retain the
>   * definition so that we can reject it as an invalid value in
> - * xfs_errortag_valid().
> + * xfs_errortag_add().
>   */
>  #define XFS_ERRTAG_DROP_WRITES				28
>  #define XFS_ERRTAG_LOG_BAD_CRC				29
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index dfa4abf9fd1a..52a1d51126e3 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -125,30 +125,6 @@ xfs_errortag_del(
>  	xfs_sysfs_del(&mp->m_errortag_kobj);
>  }
>  
> -static bool
> -xfs_errortag_valid(
> -	unsigned int		error_tag)
> -{
> -	if (error_tag >= XFS_ERRTAG_MAX)
> -		return false;
> -
> -	/* Error out removed injection types */
> -	if (error_tag == XFS_ERRTAG_DROP_WRITES)
> -		return false;
> -	return true;
> -}
> -
> -bool
> -xfs_errortag_enabled(
> -	struct xfs_mount	*mp,
> -	unsigned int		tag)
> -{
> -	if (!xfs_errortag_valid(tag))
> -		return false;
> -
> -	return mp->m_errortag[tag] != 0;
> -}
> -
>  bool
>  xfs_errortag_test(
>  	struct xfs_mount	*mp,
> @@ -158,9 +134,6 @@ xfs_errortag_test(
>  {
>  	unsigned int		randfactor;
>  
> -	if (!xfs_errortag_valid(error_tag))
> -		return false;
> -
>  	randfactor = mp->m_errortag[error_tag];
>  	if (!randfactor || get_random_u32_below(randfactor))
>  		return false;
> @@ -178,8 +151,17 @@ xfs_errortag_add(
>  {
>  	BUILD_BUG_ON(ARRAY_SIZE(xfs_errortag_random_default) != XFS_ERRTAG_MAX);
>  
> -	if (!xfs_errortag_valid(error_tag))
> +	if (error_tag >= XFS_ERRTAG_MAX)
> +		return -EINVAL;
> +
> +	/* Error out removed injection types */
> +	switch (error_tag) {
> +	case XFS_ERRTAG_DROP_WRITES:
>  		return -EINVAL;
> +	default:
> +		break;
> +	}
> +
>  	mp->m_errortag[error_tag] = xfs_errortag_random_default[error_tag];
>  	return 0;
>  }
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index 3a78c8dfaec8..ec22546a8ca8 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -44,7 +44,7 @@ bool xfs_errortag_enabled(struct xfs_mount *mp, unsigned int tag);
>  #define XFS_ERRORTAG_DELAY(mp, tag)		\
>  	do { \
>  		might_sleep(); \
> -		if (!xfs_errortag_enabled((mp), (tag))) \
> +		if (!mp->m_errortag[tag]) \
>  			break; \
>  		xfs_warn_ratelimited((mp), \
>  "Injecting %ums delay at file %s, line %d, on filesystem \"%s\"", \
> -- 
> 2.47.3
> 
>