[PATCH] xfs : Fix potential null pointer dereference in xfs_exchmaps_dir_to_sf()

[PATCH] xfs : Fix potential null pointer dereference in xfs_exchmaps_dir_to_sf()

[Chelsy Ratnawat]

xfs_dir3_block_read() can return a NULL buffer with no error, but
xfs_exchmaps_dir_to_sf() dereferences bp without checking it.
Fix this by adding a check for NULL and returning -EFSCORRUPTED if bp is
missing, since block-format directories must have a valid data block.

Signed-off-by: Chelsy Ratnawat <chelsyratnawat2001@gmail.com>
---
 fs/xfs/libxfs/xfs_exchmaps.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c
index 932ee4619e9e..e33a401d9766 100644
--- a/fs/xfs/libxfs/xfs_exchmaps.c
+++ b/fs/xfs/libxfs/xfs_exchmaps.c
@@ -475,6 +475,9 @@ xfs_exchmaps_dir_to_sf(
 	if (error)
 		return error;
 
+	if (!bp)
+		return -EFSCORRUPTED;
+
 	size = xfs_dir2_block_sfsize(xmi->xmi_ip2, bp->b_addr, &sfh);
 	if (size > xfs_inode_data_fork_size(xmi->xmi_ip2))
 		return 0;
-- 
2.47.3

Re: [PATCH] xfs : Fix potential null pointer dereference in xfs_exchmaps_dir_to_sf()

[Darrick J. Wong]

On Tue, Nov 25, 2025 at 06:22:05AM -0800, Chelsy Ratnawat wrote:
> xfs_dir3_block_read() can return a NULL buffer with no error, but
> xfs_exchmaps_dir_to_sf() dereferences bp without checking it.
> Fix this by adding a check for NULL and returning -EFSCORRUPTED if bp is
> missing, since block-format directories must have a valid data block.

How did you encounter this bug?  Did you hit it in production?  syzbot
report?  Something else?

Also, are there similar functions in xfs_exchmaps.c that suffer from the
same problem?

> Signed-off-by: Chelsy Ratnawat <chelsyratnawat2001@gmail.com>

Cc: <stable@vger.kernel.org> # v6.10
Fixes: da165fbde23b84 ("xfs: condense directories after a mapping exchange operation")

--D

> ---
>  fs/xfs/libxfs/xfs_exchmaps.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_exchmaps.c b/fs/xfs/libxfs/xfs_exchmaps.c
> index 932ee4619e9e..e33a401d9766 100644
> --- a/fs/xfs/libxfs/xfs_exchmaps.c
> +++ b/fs/xfs/libxfs/xfs_exchmaps.c
> @@ -475,6 +475,9 @@ xfs_exchmaps_dir_to_sf(
>  	if (error)
>  		return error;
>  
> +	if (!bp)
> +		return -EFSCORRUPTED;
> +
>  	size = xfs_dir2_block_sfsize(xmi->xmi_ip2, bp->b_addr, &sfh);
>  	if (size > xfs_inode_data_fork_size(xmi->xmi_ip2))
>  		return 0;
> -- 
> 2.47.3
> 

Re: [PATCH] xfs : Fix potential null pointer dereference in xfs_exchmaps_dir_to_sf()

[Christoph Hellwig]

On Tue, Nov 25, 2025 at 06:22:05AM -0800, Chelsy Ratnawat wrote:
> xfs_dir3_block_read() can return a NULL buffer with no error, but
> xfs_exchmaps_dir_to_sf() dereferences bp without checking it.
> Fix this by adding a check for NULL and returning -EFSCORRUPTED if bp is
> missing, since block-format directories must have a valid data block.

xfs_dir3_block_read is a thin wrapper around xfs_da_read_buf, which
like all the buffer functions should not return a NULL buffer unless
either an error occured, or the caller asked for read-ahead semantics,
which xfs_dir3_block_read never does.  It seems like it currently
could when an invalid mapping is passed in (see the invalid_mapping
case in xfs_dabuf_map).  I think we'd be much better off trying to
fix these inconsistencies than doctoring around them in the callers.

Re: [PATCH] xfs : Fix potential null pointer dereference in xfs_exchmaps_dir_to_sf()

[Carlos Maiolino]

On Tue, Dec 02, 2025 at 10:21:08PM -0800, Christoph Hellwig wrote:
> On Tue, Nov 25, 2025 at 06:22:05AM -0800, Chelsy Ratnawat wrote:
> > xfs_dir3_block_read() can return a NULL buffer with no error, but
> > xfs_exchmaps_dir_to_sf() dereferences bp without checking it.
> > Fix this by adding a check for NULL and returning -EFSCORRUPTED if bp is
> > missing, since block-format directories must have a valid data block.
> 
> xfs_dir3_block_read is a thin wrapper around xfs_da_read_buf, which
> like all the buffer functions should not return a NULL buffer unless
> either an error occured, or the caller asked for read-ahead semantics,
> which xfs_dir3_block_read never does.  It seems like it currently
> could when an invalid mapping is passed in (see the invalid_mapping
> case in xfs_dabuf_map).  I think we'd be much better off trying to
> fix these inconsistencies than doctoring around them in the callers.
> 

+1