Re: [PATCH RFC] xfs: remap block layer ENODATA read errors to EIO

[Dave Chinner <david@fromorbit.com>]

On Mon, Aug 18, 2025 at 03:22:02PM -0500, Eric Sandeen wrote:
> We had a report that a failing scsi disk was oopsing XFS when an xattr
> read encountered a media error. This is because the media error returned
> -ENODATA, which we map in xattr code to -ENOATTR and treat specially.
> 
> In this particular case, it looked like:
> 
> xfs_attr_leaf_get()
> 	error = xfs_attr_leaf_hasname(args, &bp);
> 	// here bp is NULL, error == -ENODATA from disk failure
> 	// but we define ENOATTR as ENODATA, so ...
> 	if (error == -ENOATTR)  {
> 		// whoops, surprise! bp is NULL, OOPS here
> 		xfs_trans_brelse(args->trans, bp);
> 		return error;
> 	} ...
> 
> To avoid whack-a-mole "test for null bp" or "which -ENODATA do we really
> mean in this function?" throughout the xattr code, my first thought is
> that we should simply map -ENODATA in lower level read functions back to
> -EIO, which is unambiguous, even if we lose the nuance of the underlying
> error code. (The block device probably already squawked.) Thoughts?
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index f9ef3b2a332a..6ba57ccaa25f 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -747,6 +747,9 @@ xfs_buf_read_map(
>  		/* bad CRC means corrupted metadata */
>  		if (error == -EFSBADCRC)
>  			error = -EFSCORRUPTED;
> +		/* ENODATA == ENOATTR which confuses xattr layers */
> +		if (error == -ENODATA)
> +			error = -EIO;

Not sure this is the right place to map this. It is only relevant to
the XFS xattr layer in the kernel, so mapping it for everything
seems like overkill.

I suspect that this error mapping should really be in
xfs_attr3_leaf_read() and xfs_da_read_buf() so it is done for
xattr fork metadata read IO only...

-Dave.
-- 
Dave Chinner
david@fromorbit.com