[PATCH 0/2] xfs: add missing cursor status check after btree ops

[PATCH 0/2] xfs: add missing cursor status check after btree ops

[Eric Sandeen]

I've been looking at ways to reduce/eliminate the frequency of the:

	if (XFS_IS_CORRUPT(mp, i != 1)) {
		xfs_btree_mark_sick(cnt_cur);
		return -EFSCORRUPTED;
	}

idiom scattered all over the tree, and in doing so I realized we
have a few spots that probably /should/ have this checked but
don't. Sending these first to ease backporting if needed, though
what they should actually get for Fixes: isn't very clear to me as
this error handling has evolved over time I think.

I did run these through xfstests but I can't guarantee the change
was tested in any targeted way, so it's mostly inspection.

After these I'll send a series that floats the idea of removing this
idiom completely from btree insert/delete type fns because "stat" is
never different from "error" in practice, but that's a story for
another day and I need to do a bit more testing first.

Thanks,
-Eric

[PATCH 1/2] xfs: add missing cursor status check after xfs_get_rec

[Eric Sandeen]

The xfs_inobt_get_rec() call in xfs_ialloc_count_ondisk fails to check
the &has_record return - if the record is not found, the next
conditional will read an uninitialized  or stale irec. Fix that.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index e705f681ed44..17699f7c009f 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -2843,6 +2843,11 @@ xfs_ialloc_count_ondisk(
 		error = xfs_inobt_get_rec(cur, &irec, &has_record);
 		if (error)
 			return error;
+		if (XFS_IS_CORRUPT(cur->bc_mp, has_record != 1)) {
+			xfs_btree_mark_sick(cur);
+			return -EFSCORRUPTED;
+		}
+
 		if (irec.ir_startino > high)
 			break;
 
-- 
2.55.0

[PATCH 2/2] xfs: add missing cursor stat checks after xfs_btree_delete

[Eric Sandeen]

Three xfs_btree_delete() calls in xfs_ialloc.c fail to check the status
returns indicating whether a record was actually deleted. If the cursor
was pointing to something that couldn't be deleted or didn't exist,
something has gone wrong, so mark the fs as sick and return
EFSCORRUPTED.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 17699f7c009f..bb6e684f5467 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -1626,12 +1626,20 @@ xfs_dialloc_ag(
 	 */
 	rec.ir_free &= ~XFS_INOBT_MASK(offset);
 	rec.ir_freecount--;
-	if (rec.ir_freecount)
+	if (rec.ir_freecount) {
 		error = xfs_inobt_update(cur, &rec);
-	else
+		if (error)
+			goto error_cur;
+	} else {
 		error = xfs_btree_delete(cur, &i);
-	if (error)
-		goto error_cur;
+		if (error)
+			goto error_cur;
+		if (XFS_IS_CORRUPT(mp, i != 1)) {
+			xfs_btree_mark_sick(cur);
+			error = -EFSCORRUPTED;
+			goto error_cur;
+		}
+	}
 
 	/*
 	 * The finobt has now been updated appropriately. We haven't updated the
@@ -2164,6 +2172,11 @@ xfs_difree_inobt(
 				__func__, error);
 			goto error0;
 		}
+		if (XFS_IS_CORRUPT(mp, i != 1)) {
+			xfs_btree_mark_sick(cur);
+			error = -EFSCORRUPTED;
+			goto error0;
+		}
 
 		error = xfs_difree_inode_chunk(tp, pag, &rec);
 		if (error)
@@ -2290,7 +2303,11 @@ xfs_difree_finobt(
 		error = xfs_btree_delete(cur, &i);
 		if (error)
 			goto error;
-		ASSERT(i == 1);
+		if (XFS_IS_CORRUPT(mp, i != 1)) {
+			xfs_btree_mark_sick(cur);
+			error = -EFSCORRUPTED;
+			goto error;
+		}
 	} else {
 		error = xfs_inobt_update(cur, &rec);
 		if (error)
-- 
2.55.0

Re: [PATCH 1/2] xfs: add missing cursor status check after xfs_get_rec

[Darrick J. Wong]

On Tue, Oct 13, 2026 at 04:07:46PM -0500, Eric Sandeen wrote:
> The xfs_inobt_get_rec() call in xfs_ialloc_count_ondisk fails to check
> the &has_record return - if the record is not found, the next
> conditional will read an uninitialized  or stale irec. Fix that.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Yeah, we had better check has_record before using irec...
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_ialloc.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index e705f681ed44..17699f7c009f 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2843,6 +2843,11 @@ xfs_ialloc_count_ondisk(
>  		error = xfs_inobt_get_rec(cur, &irec, &has_record);
>  		if (error)
>  			return error;
> +		if (XFS_IS_CORRUPT(cur->bc_mp, has_record != 1)) {
> +			xfs_btree_mark_sick(cur);
> +			return -EFSCORRUPTED;
> +		}
> +
>  		if (irec.ir_startino > high)
>  			break;
>  
> -- 
> 2.55.0
> 
> 

Re: [PATCH 2/2] xfs: add missing cursor stat checks after xfs_btree_delete

[Darrick J. Wong]

On Tue, Oct 13, 2026 at 04:07:47PM -0500, Eric Sandeen wrote:
> Three xfs_btree_delete() calls in xfs_ialloc.c fail to check the status
> returns indicating whether a record was actually deleted. If the cursor
> was pointing to something that couldn't be deleted or didn't exist,
> something has gone wrong, so mark the fs as sick and return
> EFSCORRUPTED.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good too!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_ialloc.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 17699f7c009f..bb6e684f5467 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -1626,12 +1626,20 @@ xfs_dialloc_ag(
>  	 */
>  	rec.ir_free &= ~XFS_INOBT_MASK(offset);
>  	rec.ir_freecount--;
> -	if (rec.ir_freecount)
> +	if (rec.ir_freecount) {
>  		error = xfs_inobt_update(cur, &rec);
> -	else
> +		if (error)
> +			goto error_cur;
> +	} else {
>  		error = xfs_btree_delete(cur, &i);
> -	if (error)
> -		goto error_cur;
> +		if (error)
> +			goto error_cur;
> +		if (XFS_IS_CORRUPT(mp, i != 1)) {
> +			xfs_btree_mark_sick(cur);
> +			error = -EFSCORRUPTED;
> +			goto error_cur;
> +		}
> +	}
>  
>  	/*
>  	 * The finobt has now been updated appropriately. We haven't updated the
> @@ -2164,6 +2172,11 @@ xfs_difree_inobt(
>  				__func__, error);
>  			goto error0;
>  		}
> +		if (XFS_IS_CORRUPT(mp, i != 1)) {
> +			xfs_btree_mark_sick(cur);
> +			error = -EFSCORRUPTED;
> +			goto error0;
> +		}
>  
>  		error = xfs_difree_inode_chunk(tp, pag, &rec);
>  		if (error)
> @@ -2290,7 +2303,11 @@ xfs_difree_finobt(
>  		error = xfs_btree_delete(cur, &i);
>  		if (error)
>  			goto error;
> -		ASSERT(i == 1);
> +		if (XFS_IS_CORRUPT(mp, i != 1)) {
> +			xfs_btree_mark_sick(cur);
> +			error = -EFSCORRUPTED;
> +			goto error;
> +		}
>  	} else {
>  		error = xfs_inobt_update(cur, &rec);
>  		if (error)
> -- 
> 2.55.0
> 
> 

[PATCH 1/2 V2] xfs: add missing cursor status check after xfs_inobt_get_rec

[Eric Sandeen]

The xfs_inobt_get_rec() call in xfs_ialloc_count_ondisk fails to check
the &has_record return - if the record is not found, the next
conditional will read an uninitialized  or stale irec. Fix that.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---

V2: oops, fix function name in subject!

 fs/xfs/libxfs/xfs_ialloc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index e705f681ed44..17699f7c009f 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -2843,6 +2843,11 @@ xfs_ialloc_count_ondisk(
 		error = xfs_inobt_get_rec(cur, &irec, &has_record);
 		if (error)
 			return error;
+		if (XFS_IS_CORRUPT(cur->bc_mp, has_record != 1)) {
+			xfs_btree_mark_sick(cur);
+			return -EFSCORRUPTED;
+		}
+
 		if (irec.ir_startino > high)
 			break;
 
-- 
2.55.0

Re: [PATCH 0/2] xfs: add missing cursor status check after btree ops

[Carlos Maiolino]

On Tue, 13 Oct 2026 16:07:45 -0500, Eric Sandeen wrote:
> I've been looking at ways to reduce/eliminate the frequency of the:
> 
> 	if (XFS_IS_CORRUPT(mp, i != 1)) {
> 		xfs_btree_mark_sick(cnt_cur);
> 		return -EFSCORRUPTED;
> 	}
> 
> [...]

Applied to for-next, thanks!

[1/2] xfs: add missing cursor status check after xfs_inobt_get_rec
      commit: 5beb41df46f20d8ab5591c5c1f27e79e8a761616
[2/2] xfs: add missing cursor stat checks after xfs_btree_delete
      commit: b47fb1ed0221fd21e696b468ed9eb40dd3c21f4d

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

Re: [PATCH 1/2 V2] xfs: add missing cursor status check after xfs_inobt_get_rec

[Christoph Hellwig]

On Tue, Oct 13, 2026 at 07:16:39PM -0500, Eric Sandeen wrote:
> The xfs_inobt_get_rec() call in xfs_ialloc_count_ondisk fails to check
> the &has_record return - if the record is not found, the next
> conditional will read an uninitialized  or stale irec. Fix that.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Looks good:

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

Re: [PATCH 2/2] xfs: add missing cursor stat checks after xfs_btree_delete

[Christoph Hellwig]

Looks good:

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