[PATCH V2] xfs: do not propagate ENODATA disk errors into xattr code

[PATCH V2] xfs: do not propagate ENODATA disk errors into xattr code

[Eric Sandeen]

ENODATA (aka ENOATTR) has a very specific meaning in the xfs xattr code;
namely, that the requested attribute name could not be found.

However, a medium error from disk may also return ENODATA. At best,
this medium error may escape to userspace as "attribute not found"
when in fact it's an IO (disk) error.

At worst, we may oops in xfs_attr_leaf_get() when we do:

	error = xfs_attr_leaf_hasname(args, &bp);
	if (error == -ENOATTR)  {
		xfs_trans_brelse(args->trans, bp);
		return error;
	}

because an ENODATA/ENOATTR error from disk leaves us with a null bp,
and the xfs_trans_brelse will then null-deref it.

As discussed on the list, we really need to modify the lower level
IO functions to trap all disk errors and ensure that we don't let
unique errors like this leak up into higher xfs functions - many
like this should be remapped to EIO.

However, this patch directly addresses a reported bug in the xattr
code, and should be safe to backport to stable kernels. A larger-scope
patch to handle more unique errors at lower levels can follow later.

(Note, prior to 07120f1abdff we did not oops, but we did return the
wrong error code to userspace.)

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Fixes: 07120f1abdff ("xfs: Add xfs_has_attr and subroutines")
Cc: <stable@vger.kernel.org> # v5.9+
---

V2: Remove the extraneous trap point as pointed out by djwong, oops.

(I get it that sprinkling this around in 2 places might have an ick
factor but I think it's necessary to make a surgical strike on this bug
before we address the general problem.)

Thanks,
-Eric


diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 4c44ce1c8a64..bff3dc226f81 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -435,6 +435,13 @@ xfs_attr_rmtval_get(
 					0, &bp, &xfs_attr3_rmt_buf_ops);
 			if (xfs_metadata_is_sick(error))
 				xfs_dirattr_mark_sick(args->dp, XFS_ATTR_FORK);
+			/*
+			 * ENODATA from disk implies a disk medium failure;
+			 * ENODATA for xattrs means attribute not found, so
+			 * disambiguate that here.
+			 */
+			if (error == -ENODATA)
+				error = -EIO;
 			if (error)
 				return error;
 
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 17d9e6154f19..723a0643b838 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2833,6 +2833,12 @@ xfs_da_read_buf(
 			&bp, ops);
 	if (xfs_metadata_is_sick(error))
 		xfs_dirattr_mark_sick(dp, whichfork);
+	/*
+	 * ENODATA from disk implies a disk medium failure; ENODATA for
+	 * xattrs means attribute not found, so disambiguate that here.
+	 */
+	if (error == -ENODATA && whichfork == XFS_ATTR_FORK)
+		error = -EIO;
 	if (error)
 		goto out_free;
 

Re: [PATCH V2] xfs: do not propagate ENODATA disk errors into xattr code

[Darrick J. Wong]

On Fri, Aug 22, 2025 at 12:55:56PM -0500, Eric Sandeen wrote:
> ENODATA (aka ENOATTR) has a very specific meaning in the xfs xattr code;
> namely, that the requested attribute name could not be found.
> 
> However, a medium error from disk may also return ENODATA. At best,
> this medium error may escape to userspace as "attribute not found"
> when in fact it's an IO (disk) error.
> 
> At worst, we may oops in xfs_attr_leaf_get() when we do:
> 
> 	error = xfs_attr_leaf_hasname(args, &bp);
> 	if (error == -ENOATTR)  {
> 		xfs_trans_brelse(args->trans, bp);
> 		return error;
> 	}
> 
> because an ENODATA/ENOATTR error from disk leaves us with a null bp,
> and the xfs_trans_brelse will then null-deref it.
> 
> As discussed on the list, we really need to modify the lower level
> IO functions to trap all disk errors and ensure that we don't let
> unique errors like this leak up into higher xfs functions - many
> like this should be remapped to EIO.
> 
> However, this patch directly addresses a reported bug in the xattr
> code, and should be safe to backport to stable kernels. A larger-scope
> patch to handle more unique errors at lower levels can follow later.
> 
> (Note, prior to 07120f1abdff we did not oops, but we did return the
> wrong error code to userspace.)
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Fixes: 07120f1abdff ("xfs: Add xfs_has_attr and subroutines")
> Cc: <stable@vger.kernel.org> # v5.9+

Yeah, seems fine to me.  Thanks for putting this together.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
> 
> V2: Remove the extraneous trap point as pointed out by djwong, oops.
> 
> (I get it that sprinkling this around in 2 places might have an ick
> factor but I think it's necessary to make a surgical strike on this bug
> before we address the general problem.)
> 
> Thanks,
> -Eric
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> index 4c44ce1c8a64..bff3dc226f81 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.c
> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> @@ -435,6 +435,13 @@ xfs_attr_rmtval_get(
>  					0, &bp, &xfs_attr3_rmt_buf_ops);
>  			if (xfs_metadata_is_sick(error))
>  				xfs_dirattr_mark_sick(args->dp, XFS_ATTR_FORK);
> +			/*
> +			 * ENODATA from disk implies a disk medium failure;
> +			 * ENODATA for xattrs means attribute not found, so
> +			 * disambiguate that here.
> +			 */
> +			if (error == -ENODATA)
> +				error = -EIO;
>  			if (error)
>  				return error;
>  
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 17d9e6154f19..723a0643b838 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -2833,6 +2833,12 @@ xfs_da_read_buf(
>  			&bp, ops);
>  	if (xfs_metadata_is_sick(error))
>  		xfs_dirattr_mark_sick(dp, whichfork);
> +	/*
> +	 * ENODATA from disk implies a disk medium failure; ENODATA for
> +	 * xattrs means attribute not found, so disambiguate that here.
> +	 */
> +	if (error == -ENODATA && whichfork == XFS_ATTR_FORK)
> +		error = -EIO;
>  	if (error)
>  		goto out_free;
>  
> 
> 
> 

Re: [PATCH V2] xfs: do not propagate ENODATA disk errors into xattr code

[Carlos Maiolino]

On Fri, 22 Aug 2025 12:55:56 -0500, Eric Sandeen wrote:
> ENODATA (aka ENOATTR) has a very specific meaning in the xfs xattr code;
> namely, that the requested attribute name could not be found.
> 
> However, a medium error from disk may also return ENODATA. At best,
> this medium error may escape to userspace as "attribute not found"
> when in fact it's an IO (disk) error.
> 
> [...]

Applied to for-next, thanks!

[1/1] xfs: do not propagate ENODATA disk errors into xattr code
      commit: ae668cd567a6a7622bc813ee0bb61c42bed61ba7

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