[PATCH 0/4] ocfs2: consolidate extent list validation into block read callbacks

[PATCH 0/4] ocfs2: consolidate extent list validation into block read callbacks

[Joseph Qi]

ocfs2 validates extent list fields (l_count, l_next_free_rec) at various
points during extent tree traversal. This is fragile because each caller
must remember to check for corrupted on-disk data before using it.

This series moves those checks into the block read validation callbacks
(ocfs2_validate_dx_root and ocfs2_validate_extent_block), so corrupted
fields are caught early at block read time. Redundant post-read checks
are then removed.

Patch 1-2 handle the dx_root extent list:
  - Move l_count and l_next_free_rec validation into
    ocfs2_validate_dx_root(), skipping inline dx roots.
  - Remove the now-redundant l_next_free_rec == 0 check in
    ocfs2_dx_dir_lookup_rec() and rework the error message to avoid a
    potential NULL pointer dereference of rec.

Patch 3-4 handle extent blocks:
  - Add l_count and l_next_free_rec validation to
    ocfs2_validate_extent_block().
  - Remove the duplicate l_next_free_rec > l_count check in
    __ocfs2_find_path().

Joseph Qi (4):
  ocfs2: validate dx_root extent list fields during block read
  ocfs2: remove empty extent list check in ocfs2_dx_dir_lookup_rec()
  ocfs2: validate extent block list fields during block read
  ocfs2: remove redundant l_next_free_rec check in __ocfs2_find_path()

 fs/ocfs2/alloc.c | 35 +++++++++++++++++++++-------------
 fs/ocfs2/dir.c   | 49 +++++++++++++++++++++++++++---------------------
 2 files changed, 50 insertions(+), 34 deletions(-)

-- 
2.39.3

[PATCH 1/4] ocfs2: validate dx_root extent list fields during block read

[Joseph Qi]

Move the extent list l_count validation from ocfs2_dx_dir_lookup_rec()
into ocfs2_validate_dx_root(), so that corrupted on-disk fields are
caught early at block read time rather than during directory lookups.

Additionally, add a l_next_free_rec <= l_count check to prevent
out-of-bounds access when iterating over extent records.

Both checks are skipped for inline dx roots (OCFS2_DX_FLAG_INLINE),
which use dr_entries instead of dr_list.

Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/dir.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index 1c8abf2c592c..82e720c8ba32 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -593,7 +593,7 @@ static int ocfs2_validate_dx_root(struct super_block *sb,
 		mlog(ML_ERROR,
 		     "Checksum failed for dir index root block %llu\n",
 		     (unsigned long long)bh->b_blocknr);
-		return ret;
+		goto bail;
 	}
 
 	if (!OCFS2_IS_VALID_DX_ROOT(dx_root)) {
@@ -601,8 +601,32 @@ static int ocfs2_validate_dx_root(struct super_block *sb,
 				  "Dir Index Root # %llu has bad signature %.*s\n",
 				  (unsigned long long)le64_to_cpu(dx_root->dr_blkno),
 				  7, dx_root->dr_signature);
+		goto bail;
+	}
+
+	if (!(dx_root->dr_flags & OCFS2_DX_FLAG_INLINE)) {
+		struct ocfs2_extent_list *el = &dx_root->dr_list;
+
+		if (le16_to_cpu(el->l_count) != ocfs2_extent_recs_per_dx_root(sb)) {
+			ret = ocfs2_error(sb,
+					  "Dir Index Root # %llu has invalid l_count %u (expected %u)\n",
+					  (unsigned long long)le64_to_cpu(dx_root->dr_blkno),
+					  le16_to_cpu(el->l_count),
+					  ocfs2_extent_recs_per_dx_root(sb));
+			goto bail;
+		}
+
+		if (le16_to_cpu(el->l_next_free_rec) > le16_to_cpu(el->l_count)) {
+			ret = ocfs2_error(sb,
+					  "Dir Index Root # %llu has invalid l_next_free_rec %u (l_count %u)\n",
+					  (unsigned long long)le64_to_cpu(dx_root->dr_blkno),
+					  le16_to_cpu(el->l_next_free_rec),
+					  le16_to_cpu(el->l_count));
+			goto bail;
+		}
 	}
 
+bail:
 	return ret;
 }
 
@@ -791,14 +815,6 @@ static int ocfs2_dx_dir_lookup_rec(struct inode *inode,
 	struct ocfs2_extent_block *eb;
 	struct ocfs2_extent_rec *rec = NULL;
 
-	if (le16_to_cpu(el->l_count) !=
-	    ocfs2_extent_recs_per_dx_root(inode->i_sb)) {
-		ret = ocfs2_error(inode->i_sb,
-				  "Inode %lu has invalid extent list length %u\n",
-				  inode->i_ino, le16_to_cpu(el->l_count));
-		goto out;
-	}
-
 	if (el->l_tree_depth) {
 		ret = ocfs2_find_leaf(INODE_CACHE(inode), el, major_hash,
 				      &eb_bh);
-- 
2.39.3

[PATCH 2/4] ocfs2: remove empty extent list check in ocfs2_dx_dir_lookup_rec()

[Joseph Qi]

The full extent list check is introduced by commit 44acc46d182f, which
is to avoid NULL pointer dereference if a dirent is not found.

Reworking the error message to not reference rec. Instead, report
major_hash being looked up and l_next_free_rec, which naturally covers
both failure cases (empty extent list and no matching record) without
needing a separate l_next_free_rec == 0 guard.

Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/dir.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index 82e720c8ba32..d94595a49923 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -835,14 +835,6 @@ static int ocfs2_dx_dir_lookup_rec(struct inode *inode,
 		}
 	}
 
-	if (le16_to_cpu(el->l_next_free_rec) == 0) {
-		ret = ocfs2_error(inode->i_sb,
-				  "Inode %lu has empty extent list at depth %u\n",
-				  inode->i_ino,
-				  le16_to_cpu(el->l_tree_depth));
-		goto out;
-	}
-
 	found = 0;
 	for (i = le16_to_cpu(el->l_next_free_rec) - 1; i >= 0; i--) {
 		rec = &el->l_recs[i];
@@ -855,10 +847,9 @@ static int ocfs2_dx_dir_lookup_rec(struct inode *inode,
 
 	if (!found) {
 		ret = ocfs2_error(inode->i_sb,
-				  "Inode %lu has bad extent record (%u, %u, 0) in btree\n",
-				  inode->i_ino,
-				  le32_to_cpu(rec->e_cpos),
-				  ocfs2_rec_clusters(el, rec));
+				  "Inode %lu has no extent record for hash %u in btree (next_free_rec %u)\n",
+				  inode->i_ino, major_hash,
+				  le16_to_cpu(el->l_next_free_rec));
 		goto out;
 	}
 
-- 
2.39.3

[PATCH 3/4] ocfs2: validate extent block list fields during block read

[Joseph Qi]

Add extent list validation to ocfs2_validate_extent_block() so that
corrupted on-disk fields are caught early at block read time rather
than during extent tree traversal.

Two checks are added:

  - l_count must equal the expected value from
    ocfs2_extent_recs_per_eb(), catching blocks with a corrupted record
    count before any array iteration.

  - l_next_free_rec must not exceed l_count, preventing out-of-bounds
    access when iterating over extent records.

Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/alloc.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index 344fd4d95fbc..8639806bcbb8 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -917,11 +917,32 @@ static int ocfs2_validate_extent_block(struct super_block *sb,
 		goto bail;
 	}
 
-	if (le32_to_cpu(eb->h_fs_generation) != OCFS2_SB(sb)->fs_generation)
+	if (le32_to_cpu(eb->h_fs_generation) != OCFS2_SB(sb)->fs_generation) {
 		rc = ocfs2_error(sb,
 				 "Extent block #%llu has an invalid h_fs_generation of #%u\n",
 				 (unsigned long long)bh->b_blocknr,
 				 le32_to_cpu(eb->h_fs_generation));
+		goto bail;
+	}
+
+	if (le16_to_cpu(eb->h_list.l_count) != ocfs2_extent_recs_per_eb(sb)) {
+		rc = ocfs2_error(sb,
+				 "Extent block #%llu has invalid l_count %u (expected %u)\n",
+				 (unsigned long long)bh->b_blocknr,
+				 le16_to_cpu(eb->h_list.l_count),
+				 ocfs2_extent_recs_per_eb(sb));
+		goto bail;
+	}
+
+	if (le16_to_cpu(eb->h_list.l_next_free_rec) > le16_to_cpu(eb->h_list.l_count)) {
+		rc = ocfs2_error(sb,
+				 "Extent block #%llu has invalid l_next_free_rec %u (l_count %u)\n",
+				 (unsigned long long)bh->b_blocknr,
+				 le16_to_cpu(eb->h_list.l_next_free_rec),
+				 le16_to_cpu(eb->h_list.l_count));
+		goto bail;
+	}
+
 bail:
 	return rc;
 }
-- 
2.39.3

[PATCH 4/4] ocfs2: remove redundant l_next_free_rec check in __ocfs2_find_path()

[Joseph Qi]

The l_next_free_rec > l_count check after ocfs2_read_extent_block() in
__ocfs2_find_path() is now redundant, as ocfs2_validate_extent_block()
already performs this validation at block read time.

Remove the duplicate check to avoid maintaining the same validation in
two places.

Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/alloc.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index 8639806bcbb8..08e137928080 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -1878,18 +1878,6 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
 		eb = (struct ocfs2_extent_block *) bh->b_data;
 		el = &eb->h_list;
 
-		if (le16_to_cpu(el->l_next_free_rec) >
-		    le16_to_cpu(el->l_count)) {
-			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
-				    "Owner %llu has bad count in extent list at block %llu (next free=%u, count=%u)\n",
-				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
-				    (unsigned long long)bh->b_blocknr,
-				    le16_to_cpu(el->l_next_free_rec),
-				    le16_to_cpu(el->l_count));
-			ret = -EROFS;
-			goto out;
-		}
-
 		if (func)
 			func(data, bh);
 	}
-- 
2.39.3

Re: [PATCH 0/4] ocfs2: consolidate extent list validation into block read callbacks

[Andrew Morton]

On Fri,  3 Apr 2026 17:07:59 +0800 Joseph Qi <joseph.qi@linux.alibaba.com> wrote:

> ocfs2 validates extent list fields (l_count, l_next_free_rec) at various
> points during extent tree traversal. This is fragile because each caller
> must remember to check for corrupted on-disk data before using it.
> 
> This series moves those checks into the block read validation callbacks
> (ocfs2_validate_dx_root and ocfs2_validate_extent_block), so corrupted
> fields are caught early at block read time. Redundant post-read checks
> are then removed.

AI review asked a few questions - mainly suggesting additional changes.
https://sashiko.dev/#/patchset/20260403090803.3860971-1-joseph.qi@linux.alibaba.com

Re: [PATCH 0/4] ocfs2: consolidate extent list validation into block read callbacks

[Joseph Qi]

On 4/4/26 3:27 AM, Andrew Morton wrote:
> On Fri,  3 Apr 2026 17:07:59 +0800 Joseph Qi <joseph.qi@linux.alibaba.com> wrote:
> 
>> ocfs2 validates extent list fields (l_count, l_next_free_rec) at various
>> points during extent tree traversal. This is fragile because each caller
>> must remember to check for corrupted on-disk data before using it.
>>
>> This series moves those checks into the block read validation callbacks
>> (ocfs2_validate_dx_root and ocfs2_validate_extent_block), so corrupted
>> fields are caught early at block read time. Redundant post-read checks
>> are then removed.
> 
> AI review asked a few questions - mainly suggesting additional changes.
> https://sashiko.dev/#/patchset/20260403090803.3860971-1-joseph.qi@linux.alibaba.com

Yes, not regressions, actually some improvements.

Thanks,
Joseph

Re: [PATCH 1/4] ocfs2: validate dx_root extent list fields during block read

[Heming Zhao]

On Fri, Apr 03, 2026 at 05:08:00PM +0800, Joseph Qi wrote:
> Move the extent list l_count validation from ocfs2_dx_dir_lookup_rec()
> into ocfs2_validate_dx_root(), so that corrupted on-disk fields are
> caught early at block read time rather than during directory lookups.
> 
> Additionally, add a l_next_free_rec <= l_count check to prevent
> out-of-bounds access when iterating over extent records.
> 
> Both checks are skipped for inline dx roots (OCFS2_DX_FLAG_INLINE),
> which use dr_entries instead of dr_list.
> 
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>

LGTM
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/dir.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index 1c8abf2c592c..82e720c8ba32 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -593,7 +593,7 @@ static int ocfs2_validate_dx_root(struct super_block *sb,
>  		mlog(ML_ERROR,
>  		     "Checksum failed for dir index root block %llu\n",
>  		     (unsigned long long)bh->b_blocknr);
> -		return ret;
> +		goto bail;
>  	}
>  
>  	if (!OCFS2_IS_VALID_DX_ROOT(dx_root)) {
> @@ -601,8 +601,32 @@ static int ocfs2_validate_dx_root(struct super_block *sb,
>  				  "Dir Index Root # %llu has bad signature %.*s\n",
>  				  (unsigned long long)le64_to_cpu(dx_root->dr_blkno),
>  				  7, dx_root->dr_signature);
> +		goto bail;
> +	}
> +
> +	if (!(dx_root->dr_flags & OCFS2_DX_FLAG_INLINE)) {
> +		struct ocfs2_extent_list *el = &dx_root->dr_list;
> +
> +		if (le16_to_cpu(el->l_count) != ocfs2_extent_recs_per_dx_root(sb)) {
> +			ret = ocfs2_error(sb,
> +					  "Dir Index Root # %llu has invalid l_count %u (expected %u)\n",
> +					  (unsigned long long)le64_to_cpu(dx_root->dr_blkno),
> +					  le16_to_cpu(el->l_count),
> +					  ocfs2_extent_recs_per_dx_root(sb));
> +			goto bail;
> +		}
> +
> +		if (le16_to_cpu(el->l_next_free_rec) > le16_to_cpu(el->l_count)) {
> +			ret = ocfs2_error(sb,
> +					  "Dir Index Root # %llu has invalid l_next_free_rec %u (l_count %u)\n",
> +					  (unsigned long long)le64_to_cpu(dx_root->dr_blkno),
> +					  le16_to_cpu(el->l_next_free_rec),
> +					  le16_to_cpu(el->l_count));
> +			goto bail;
> +		}
>  	}
>  
> +bail:
>  	return ret;
>  }
>  
> @@ -791,14 +815,6 @@ static int ocfs2_dx_dir_lookup_rec(struct inode *inode,
>  	struct ocfs2_extent_block *eb;
>  	struct ocfs2_extent_rec *rec = NULL;
>  
> -	if (le16_to_cpu(el->l_count) !=
> -	    ocfs2_extent_recs_per_dx_root(inode->i_sb)) {
> -		ret = ocfs2_error(inode->i_sb,
> -				  "Inode %lu has invalid extent list length %u\n",
> -				  inode->i_ino, le16_to_cpu(el->l_count));
> -		goto out;
> -	}
> -
>  	if (el->l_tree_depth) {
>  		ret = ocfs2_find_leaf(INODE_CACHE(inode), el, major_hash,
>  				      &eb_bh);
> -- 
> 2.39.3
> 

Re: [PATCH 2/4] ocfs2: remove empty extent list check in ocfs2_dx_dir_lookup_rec()

[Heming Zhao]

On Fri, Apr 03, 2026 at 05:08:01PM +0800, Joseph Qi wrote:
> The full extent list check is introduced by commit 44acc46d182f, which
> is to avoid NULL pointer dereference if a dirent is not found.
> 
> Reworking the error message to not reference rec. Instead, report
> major_hash being looked up and l_next_free_rec, which naturally covers
> both failure cases (empty extent list and no matching record) without
> needing a separate l_next_free_rec == 0 guard.
> 
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>

LGTM
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/dir.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index 82e720c8ba32..d94595a49923 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -835,14 +835,6 @@ static int ocfs2_dx_dir_lookup_rec(struct inode *inode,
>  		}
>  	}
>  
> -	if (le16_to_cpu(el->l_next_free_rec) == 0) {
> -		ret = ocfs2_error(inode->i_sb,
> -				  "Inode %lu has empty extent list at depth %u\n",
> -				  inode->i_ino,
> -				  le16_to_cpu(el->l_tree_depth));
> -		goto out;
> -	}
> -
>  	found = 0;
>  	for (i = le16_to_cpu(el->l_next_free_rec) - 1; i >= 0; i--) {
>  		rec = &el->l_recs[i];
> @@ -855,10 +847,9 @@ static int ocfs2_dx_dir_lookup_rec(struct inode *inode,
>  
>  	if (!found) {
>  		ret = ocfs2_error(inode->i_sb,
> -				  "Inode %lu has bad extent record (%u, %u, 0) in btree\n",
> -				  inode->i_ino,
> -				  le32_to_cpu(rec->e_cpos),
> -				  ocfs2_rec_clusters(el, rec));
> +				  "Inode %lu has no extent record for hash %u in btree (next_free_rec %u)\n",
> +				  inode->i_ino, major_hash,
> +				  le16_to_cpu(el->l_next_free_rec));
>  		goto out;
>  	}
>  
> -- 
> 2.39.3
> 

Re: [PATCH 3/4] ocfs2: validate extent block list fields during block read

[Heming Zhao]

On Fri, Apr 03, 2026 at 05:08:02PM +0800, Joseph Qi wrote:
> Add extent list validation to ocfs2_validate_extent_block() so that
> corrupted on-disk fields are caught early at block read time rather
> than during extent tree traversal.
> 
> Two checks are added:
> 
>   - l_count must equal the expected value from
>     ocfs2_extent_recs_per_eb(), catching blocks with a corrupted record
>     count before any array iteration.
> 
>   - l_next_free_rec must not exceed l_count, preventing out-of-bounds
>     access when iterating over extent records.
> 
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>

LGTM
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/alloc.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 344fd4d95fbc..8639806bcbb8 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -917,11 +917,32 @@ static int ocfs2_validate_extent_block(struct super_block *sb,
>  		goto bail;
>  	}
>  
> -	if (le32_to_cpu(eb->h_fs_generation) != OCFS2_SB(sb)->fs_generation)
> +	if (le32_to_cpu(eb->h_fs_generation) != OCFS2_SB(sb)->fs_generation) {
>  		rc = ocfs2_error(sb,
>  				 "Extent block #%llu has an invalid h_fs_generation of #%u\n",
>  				 (unsigned long long)bh->b_blocknr,
>  				 le32_to_cpu(eb->h_fs_generation));
> +		goto bail;
> +	}
> +
> +	if (le16_to_cpu(eb->h_list.l_count) != ocfs2_extent_recs_per_eb(sb)) {
> +		rc = ocfs2_error(sb,
> +				 "Extent block #%llu has invalid l_count %u (expected %u)\n",
> +				 (unsigned long long)bh->b_blocknr,
> +				 le16_to_cpu(eb->h_list.l_count),
> +				 ocfs2_extent_recs_per_eb(sb));
> +		goto bail;
> +	}
> +
> +	if (le16_to_cpu(eb->h_list.l_next_free_rec) > le16_to_cpu(eb->h_list.l_count)) {
> +		rc = ocfs2_error(sb,
> +				 "Extent block #%llu has invalid l_next_free_rec %u (l_count %u)\n",
> +				 (unsigned long long)bh->b_blocknr,
> +				 le16_to_cpu(eb->h_list.l_next_free_rec),
> +				 le16_to_cpu(eb->h_list.l_count));
> +		goto bail;
> +	}
> +
>  bail:
>  	return rc;
>  }
> -- 
> 2.39.3
> 

Re: [PATCH 4/4] ocfs2: remove redundant l_next_free_rec check in __ocfs2_find_path()

[Heming Zhao]

On Fri, Apr 03, 2026 at 05:08:03PM +0800, Joseph Qi wrote:
> The l_next_free_rec > l_count check after ocfs2_read_extent_block() in
> __ocfs2_find_path() is now redundant, as ocfs2_validate_extent_block()
> already performs this validation at block read time.
> 
> Remove the duplicate check to avoid maintaining the same validation in
> two places.
> 
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>

LGTM
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/alloc.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 8639806bcbb8..08e137928080 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -1878,18 +1878,6 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
>  		eb = (struct ocfs2_extent_block *) bh->b_data;
>  		el = &eb->h_list;
>  
> -		if (le16_to_cpu(el->l_next_free_rec) >
> -		    le16_to_cpu(el->l_count)) {
> -			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
> -				    "Owner %llu has bad count in extent list at block %llu (next free=%u, count=%u)\n",
> -				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
> -				    (unsigned long long)bh->b_blocknr,
> -				    le16_to_cpu(el->l_next_free_rec),
> -				    le16_to_cpu(el->l_count));
> -			ret = -EROFS;
> -			goto out;
> -		}
> -
>  		if (func)
>  			func(data, bh);
>  	}
> -- 
> 2.39.3
>