In ocfs2_get_refcount_rec(), the 'rec' pointer is initialized to NULL.
If the extent list is empty (el->l_next_free_rec == 0), the loop skips
assignment, leaving 'rec' as NULL and 'found' as 0.

Currently, the code skips the 'if (found)' block but proceeds directly to
dereference 'rec' at line 767 (le64_to_cpu(rec->e_blkno)), causing a
NULL pointer dereference panic.

This patch adds an 'else' branch to the 'if (found)' check. If no valid
record is found, it reports a filesystem error and exits, preventing
the invalid memory access.

Fixes: e73a819db9c2 ("ocfs2: Add support for incrementing refcount in the tree.")
Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
---
Changelog:

v1 -> v2:

1. Add a Fixes tag.
---
 fs/ocfs2/refcounttree.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
index c92e0ea85bca..464bdd6e0a8e 100644
--- a/fs/ocfs2/refcounttree.c
+++ b/fs/ocfs2/refcounttree.c
@@ -1122,6 +1122,11 @@ static int ocfs2_get_refcount_rec(struct ocfs2_caching_info *ci,
 
 		if (cpos_end < low_cpos + len)
 			len = cpos_end - low_cpos;
+	} else {
+		ret = ocfs2_error(sb, "Refcount tree %llu has no extent record covering cpos %u\n",
+				  (unsigned long long)ocfs2_metadata_cache_owner(ci),
+				  low_cpos);
+		goto out;
 	}
 
 	ret = ocfs2_read_refcount_block(ci, le64_to_cpu(rec->e_blkno),
-- 
2.25.1

On Sun, Jan 18, 2026 at 07:05:23PM +0000, Jiasheng Jiang wrote:
> In ocfs2_get_refcount_rec(), the 'rec' pointer is initialized to NULL.
> If the extent list is empty (el->l_next_free_rec == 0), the loop skips
> assignment, leaving 'rec' as NULL and 'found' as 0.
> 
> Currently, the code skips the 'if (found)' block but proceeds directly to
> dereference 'rec' at line 767 (le64_to_cpu(rec->e_blkno)), causing a
> NULL pointer dereference panic.

Do you have reproduction steps to support your analysis?

there are two types of 'rb': leaf or tree.
the check 'if (!(le32_to_cpu(rb->rf_flags) & OCFS2_REFCOUNT_TREE_FL))'
handles leaf case and returns to the caller.
the subsequent code handles the 'tree' type, therefore, in theory,
el->l_next_free_rec should be ">= 1", and the execution should enter the
for-loop to assign the 'rec'.

Thanks,
Heming

> 
> This patch adds an 'else' branch to the 'if (found)' check. If no valid
> record is found, it reports a filesystem error and exits, preventing
> the invalid memory access.
> 
> Fixes: e73a819db9c2 ("ocfs2: Add support for incrementing refcount in the tree.")
> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
> ---
> Changelog:
> 
> v1 -> v2:
> 
> 1. Add a Fixes tag.
> ---
>  fs/ocfs2/refcounttree.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
> index c92e0ea85bca..464bdd6e0a8e 100644
> --- a/fs/ocfs2/refcounttree.c
> +++ b/fs/ocfs2/refcounttree.c
> @@ -1122,6 +1122,11 @@ static int ocfs2_get_refcount_rec(struct ocfs2_caching_info *ci,
>  
>  		if (cpos_end < low_cpos + len)
>  			len = cpos_end - low_cpos;
> +	} else {
> +		ret = ocfs2_error(sb, "Refcount tree %llu has no extent record covering cpos %u\n",
> +				  (unsigned long long)ocfs2_metadata_cache_owner(ci),
> +				  low_cpos);
> +		goto out;
>  	}
>  
>  	ret = ocfs2_read_refcount_block(ci, le64_to_cpu(rec->e_blkno),
> -- 
> 2.25.1
> 
>

On Mon, 19 Jan 2026 22:43:26 +0800, Heming Zhao wrote:
> On Sun, Jan 18, 2026 at 07:05:23PM +0000, Jiasheng Jiang wrote:
>> In ocfs2_get_refcount_rec(), the 'rec' pointer is initialized to NULL.
>> If the extent list is empty (el->l_next_free_rec == 0), the loop skips
>> assignment, leaving 'rec' as NULL and 'found' as 0.
>> 
>> Currently, the code skips the 'if (found)' block but proceeds directly to
>> dereference 'rec' at line 767 (le64_to_cpu(rec->e_blkno)), causing a
>> NULL pointer dereference panic.
> 
> Do you have reproduction steps to support your analysis?
> 

Thanks for your review. We found this issue through static analysis and
code review. We do not have a specific reproduction script or a corrupted
filesystem image to trigger this at the moment.

> there are two types of 'rb': leaf or tree.
> the check 'if (!(le32_to_cpu(rb->rf_flags) & OCFS2_REFCOUNT_TREE_FL))'
> handles leaf case and returns to the caller.
> the subsequent code handles the 'tree' type, therefore, in theory,
> el->l_next_free_rec should be ">= 1", and the execution should enter the
> for-loop to assign the 'rec'.
> 
> Thanks,
> Heming
> 

You are absolutely correct. Theoretically, if the OCFS2_REFCOUNT_TREE_FL
flag is set, the extent list should not be empty, and el->l_next_free_rec
should be >= 1.

However, if the filesystem metadata is corrupted (e.g., due to bit rot or
software bugs), it is possible that l_next_free_rec reads as 0 even for a
tree root. In the current implementation, such inconsistency leads to a
NULL pointer dereference and panics the system.

This patch is intended as a hardening measure. Instead of crashing the
kernel when encountering such invalid on-disk data, it is safer to report
a filesystem error via ocfs2_error and abort the operation gracefully.

This approach aligns with recent fixes in OCFS2. For example, Commit
44acc46d182f ("ocfs2: avoid NULL pointer dereference in
dx_dir_lookup_rec()") addressed an identical issue where an empty extent
list left the 'rec' pointer NULL.

I will update the commit message in the next version to explicitly state
that this is a hardening measure against on-disk corruption, ensuring the
rationale is accurate.

>> 
>> This patch adds an 'else' branch to the 'if (found)' check. If no valid
>> record is found, it reports a filesystem error and exits, preventing
>> the invalid memory access.
>> 
>> Fixes: e73a819db9c2 ("ocfs2: Add support for incrementing refcount in the tree.")
>> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
>> ---
>> Changelog:
>> 
>> v1 -> v2:
>> 
>> 1. Add a Fixes tag.
>> ---
>>  fs/ocfs2/refcounttree.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
>> index c92e0ea85bca..464bdd6e0a8e 100644
>> --- a/fs/ocfs2/refcounttree.c
>> +++ b/fs/ocfs2/refcounttree.c
>> @@ -1122,6 +1122,11 @@ static int ocfs2_get_refcount_rec(struct ocfs2_caching_info *ci,
>>  
>>  		if (cpos_end < low_cpos + len)
>>  			len = cpos_end - low_cpos;
>> +	} else {
>> +		ret = ocfs2_error(sb, "Refcount tree %llu has no extent record covering cpos %u\n",
>> +				  (unsigned long long)ocfs2_metadata_cache_owner(ci),
>> +				  low_cpos);
>> +		goto out;
>>  	}
>>  
>>  	ret = ocfs2_read_refcount_block(ci, le64_to_cpu(rec->e_blkno),
>> -- 
>> 2.25.1
>> 
>>

On Mon, Jan 19, 2026 at 05:12:15PM +0000, Jiasheng Jiang wrote:
> On Mon, 19 Jan 2026 22:43:26 +0800, Heming Zhao wrote:
> > On Sun, Jan 18, 2026 at 07:05:23PM +0000, Jiasheng Jiang wrote:
> >> In ocfs2_get_refcount_rec(), the 'rec' pointer is initialized to NULL.
> >> If the extent list is empty (el->l_next_free_rec == 0), the loop skips
> >> assignment, leaving 'rec' as NULL and 'found' as 0.
> >> 
> >> Currently, the code skips the 'if (found)' block but proceeds directly to
> >> dereference 'rec' at line 767 (le64_to_cpu(rec->e_blkno)), causing a
> >> NULL pointer dereference panic.
> > 
> > Do you have reproduction steps to support your analysis?
> > 
> 
> Thanks for your review. We found this issue through static analysis and
> code review. We do not have a specific reproduction script or a corrupted
> filesystem image to trigger this at the moment.
> 
> > there are two types of 'rb': leaf or tree.
> > the check 'if (!(le32_to_cpu(rb->rf_flags) & OCFS2_REFCOUNT_TREE_FL))'
> > handles leaf case and returns to the caller.
> > the subsequent code handles the 'tree' type, therefore, in theory,
> > el->l_next_free_rec should be ">= 1", and the execution should enter the
> > for-loop to assign the 'rec'.
> > 
> > Thanks,
> > Heming
> > 
> 
> You are absolutely correct. Theoretically, if the OCFS2_REFCOUNT_TREE_FL
> flag is set, the extent list should not be empty, and el->l_next_free_rec
> should be >= 1.
> 
> However, if the filesystem metadata is corrupted (e.g., due to bit rot or
> software bugs), it is possible that l_next_free_rec reads as 0 even for a
> tree root. In the current implementation, such inconsistency leads to a
> NULL pointer dereference and panics the system.
> 
> This patch is intended as a hardening measure. Instead of crashing the
> kernel when encountering such invalid on-disk data, it is safer to report
> a filesystem error via ocfs2_error and abort the operation gracefully.
> 
> This approach aligns with recent fixes in OCFS2. For example, Commit
> 44acc46d182f ("ocfs2: avoid NULL pointer dereference in
> dx_dir_lookup_rec()") addressed an identical issue where an empty extent
> list left the 'rec' pointer NULL.
> 
> I will update the commit message in the next version to explicitly state
> that this is a hardening measure against on-disk corruption, ensuring the
> rationale is accurate.

I am quoting the rule from Documentation/process/stable-kernel-rules.rst:

Rules on what kind of patches are accepted, and which ones are not, into the
"-stable" tree:
  ... ...
  - No "This could be a problem..." type of things like a "theoretical race
    condition", unless an explanation of how the bug can be exploited is also
    provided.

This principle applies to your case as well (even though you are targeting
the mainline tree). I don't believe the issues mentioned in these patches
are worth fixing without proof. Please provide reproduction steps for all
your patches before we continue the review.

Thanks,
Heming

> 
> >> 
> >> This patch adds an 'else' branch to the 'if (found)' check. If no valid
> >> record is found, it reports a filesystem error and exits, preventing
> >> the invalid memory access.
> >> 
> >> Fixes: e73a819db9c2 ("ocfs2: Add support for incrementing refcount in the tree.")
> >> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
> >> ---
> >> Changelog:
> >> 
> >> v1 -> v2:
> >> 
> >> 1. Add a Fixes tag.
> >> ---
> >>  fs/ocfs2/refcounttree.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >> 
> >> diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
> >> index c92e0ea85bca..464bdd6e0a8e 100644
> >> --- a/fs/ocfs2/refcounttree.c
> >> +++ b/fs/ocfs2/refcounttree.c
> >> @@ -1122,6 +1122,11 @@ static int ocfs2_get_refcount_rec(struct ocfs2_caching_info *ci,
> >>  
> >>  		if (cpos_end < low_cpos + len)
> >>  			len = cpos_end - low_cpos;
> >> +	} else {
> >> +		ret = ocfs2_error(sb, "Refcount tree %llu has no extent record covering cpos %u\n",
> >> +				  (unsigned long long)ocfs2_metadata_cache_owner(ci),
> >> +				  low_cpos);
> >> +		goto out;
> >>  	}
> >>  
> >>  	ret = ocfs2_read_refcount_block(ci, le64_to_cpu(rec->e_blkno),
> >> -- 
> >> 2.25.1
> >> 
> >>

…> This patch adds an 'else' branch …

Thanks for another contribution.

* Does anything hinder to follow a corresponding wording requirement?
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.19-rc5#n94

* Can it be helpful to append parentheses to function names?


Regards,
Markus

On Mon, Jan 19, 2026 at 10:42:29AM +0100, Markus Elfring wrote:
> …> This patch adds an 'else' branch …
> 
> Thanks for another contribution.
> 
> * Does anything hinder to follow a corresponding wording requirement?
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.19-rc5#n94
> 
> * Can it be helpful to append parentheses to function names?
> 
> 
> Regards,
> Markus
> 

Hi,

This is the semi-friendly patch-bot of Greg Kroah-Hartman.

Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list.  I strongly suggest that you not do this anymore.  Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.

Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all.  The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback.  Please feel free to also ignore emails
from them.

thanks,

greg k-h's patch email bot

On Sun, Jan 18, 2026 at 07:05:23PM +0000, Jiasheng Jiang wrote:
> In ocfs2_get_refcount_rec(), the 'rec' pointer is initialized to NULL.
> If the extent list is empty (el->l_next_free_rec == 0), the loop skips
> assignment, leaving 'rec' as NULL and 'found' as 0.
> 
> Currently, the code skips the 'if (found)' block but proceeds directly to
> dereference 'rec' at line 767 (le64_to_cpu(rec->e_blkno)), causing a
> NULL pointer dereference panic.
> 
> This patch adds an 'else' branch to the 'if (found)' check. If no valid
> record is found, it reports a filesystem error and exits, preventing
> the invalid memory access.
> 
> Fixes: e73a819db9c2 ("ocfs2: Add support for incrementing refcount in the tree.")
> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
> ---
> Changelog:
> 
> v1 -> v2:
> 
> 1. Add a Fixes tag.
> ---
>  fs/ocfs2/refcounttree.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
> index c92e0ea85bca..464bdd6e0a8e 100644
> --- a/fs/ocfs2/refcounttree.c
> +++ b/fs/ocfs2/refcounttree.c
> @@ -1122,6 +1122,11 @@ static int ocfs2_get_refcount_rec(struct ocfs2_caching_info *ci,
>  
>  		if (cpos_end < low_cpos + len)
>  			len = cpos_end - low_cpos;
> +	} else {
> +		ret = ocfs2_error(sb, "Refcount tree %llu has no extent record covering cpos %u\n",
> +				  (unsigned long long)ocfs2_metadata_cache_owner(ci),
> +				  low_cpos);
> +		goto out;
>  	}
>  
>  	ret = ocfs2_read_refcount_block(ci, le64_to_cpu(rec->e_blkno),
> -- 
> 2.25.1
> 
> 

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>