ocfs2: fix NULL pointer dereference in ocfs2_xattr_get_rec

[PATCH] ocfs2: fix NULL pointer dereference in ocfs2_xattr_get_rec

Posted by Jiasheng Jiang 2 weeks, 6 days ago

In ocfs2_xattr_get_rec, the variable 'rec' is initialized to NULL.
If the extent list 'el' is empty (l_next_free_rec == 0), the loop
iterating over the records is skipped, leaving 'rec' as NULL.

Since 'e_blkno' is initialized to 0, the function enters the error
handling block 'if (!e_blkno)'. Inside this block, the function calls
ocfs2_error() and attempts to dereference 'rec' via
'le32_to_cpu(rec->e_cpos)' and 'ocfs2_rec_clusters(el, rec)'. This
results in a NULL pointer dereference and a kernel panic.

Fix this by ensuring 'rec' is not NULL before accessing its members
within the error handling path, or by checking for an empty list
explicitly.

Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
---
 fs/ocfs2/xattr.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 1b21fbc16d73..b018c84dbc05 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -3757,10 +3757,16 @@ static int ocfs2_xattr_get_rec(struct inode *inode,
 	}
 
 	if (!e_blkno) {
-		ret = ocfs2_error(inode->i_sb, "Inode %lu has bad extent record (%u, %u, 0) in xattr\n",
-				  inode->i_ino,
-				  le32_to_cpu(rec->e_cpos),
-				  ocfs2_rec_clusters(el, rec));
+		if (rec)
+			ret = ocfs2_error(inode->i_sb,
+					  "Inode %lu has bad extent record (%u, %u, 0) in xattr\n",
+					  inode->i_ino,
+					  le32_to_cpu(rec->e_cpos),
+					  ocfs2_rec_clusters(el, rec));
+		else
+			ret = ocfs2_error(inode->i_sb,
+					  "Inode %lu has bad extent record (NULL) in xattr\n",
+					  inode->i_ino);
 		goto out;
 	}
 
-- 
2.25.1

Re: [PATCH] ocfs2: fix NULL pointer dereference in ocfs2_xattr_get_rec

Posted by Heming Zhao 2 weeks, 5 days ago

On Sat, Jan 17, 2026 at 05:35:56PM +0000, Jiasheng Jiang wrote:
> In ocfs2_xattr_get_rec, the variable 'rec' is initialized to NULL.
> If the extent list 'el' is empty (l_next_free_rec == 0), the loop
> iterating over the records is skipped, leaving 'rec' as NULL.
> 
> Since 'e_blkno' is initialized to 0, the function enters the error
> handling block 'if (!e_blkno)'. Inside this block, the function calls
> ocfs2_error() and attempts to dereference 'rec' via
> 'le32_to_cpu(rec->e_cpos)' and 'ocfs2_rec_clusters(el, rec)'. This
> results in a NULL pointer dereference and a kernel panic.

Do you have reproduction steps to support your analysis?

- Heming
> 
> Fix this by ensuring 'rec' is not NULL before accessing its members
> within the error handling path, or by checking for an empty list
> explicitly.
> 
> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
> ---
>  fs/ocfs2/xattr.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index 1b21fbc16d73..b018c84dbc05 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -3757,10 +3757,16 @@ static int ocfs2_xattr_get_rec(struct inode *inode,
>  	}
>  
>  	if (!e_blkno) {
> -		ret = ocfs2_error(inode->i_sb, "Inode %lu has bad extent record (%u, %u, 0) in xattr\n",
> -				  inode->i_ino,
> -				  le32_to_cpu(rec->e_cpos),
> -				  ocfs2_rec_clusters(el, rec));
> +		if (rec)
> +			ret = ocfs2_error(inode->i_sb,
> +					  "Inode %lu has bad extent record (%u, %u, 0) in xattr\n",
> +					  inode->i_ino,
> +					  le32_to_cpu(rec->e_cpos),
> +					  ocfs2_rec_clusters(el, rec));
> +		else
> +			ret = ocfs2_error(inode->i_sb,
> +					  "Inode %lu has bad extent record (NULL) in xattr\n",
> +					  inode->i_ino);
>  		goto out;
>  	}
>  
> -- 
> 2.25.1
> 
>

Re: [PATCH] ocfs2: fix NULL pointer dereference in ocfs2_xattr_get_rec

Posted by Jiasheng Jiang 2 weeks, 4 days ago

On Mon, 19 Jan 2026 22:49:16 +0800, Heming Zhao wrote:
> On Sat, Jan 17, 2026 at 05:35:56PM +0000, Jiasheng Jiang wrote:
>> In ocfs2_xattr_get_rec, the variable 'rec' is initialized to NULL.
>> If the extent list 'el' is empty (l_next_free_rec == 0), the loop
>> iterating over the records is skipped, leaving 'rec' as NULL.
>> 
>> Since 'e_blkno' is initialized to 0, the function enters the error
>> handling block 'if (!e_blkno)'. Inside this block, the function calls
>> ocfs2_error() and attempts to dereference 'rec' via
>> 'le32_to_cpu(rec->e_cpos)' and 'ocfs2_rec_clusters(el, rec)'. This
>> results in a NULL pointer dereference and a kernel panic.
> 
> Do you have reproduction steps to support your analysis?
> 
> - Heming

Regarding the reproduction steps, we detected this issue through static
analysis. We do not have a specific script to reproduce this as it
requires a corrupted filesystem image.

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. 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.

- Jiasheng

>> 
>> Fix this by ensuring 'rec' is not NULL before accessing its members
>> within the error handling path, or by checking for an empty list
>> explicitly.
>> 
>> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
>> ---
>>  fs/ocfs2/xattr.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>> 
>> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
>> index 1b21fbc16d73..b018c84dbc05 100644
>> --- a/fs/ocfs2/xattr.c
>> +++ b/fs/ocfs2/xattr.c
>> @@ -3757,10 +3757,16 @@ static int ocfs2_xattr_get_rec(struct inode *inode,
>>  	}
>>  
>>  	if (!e_blkno) {
>> -		ret = ocfs2_error(inode->i_sb, "Inode %lu has bad extent record (%u, %u, 0) in xattr\n",
>> -				  inode->i_ino,
>> -				  le32_to_cpu(rec->e_cpos),
>> -				  ocfs2_rec_clusters(el, rec));
>> +		if (rec)
>> +			ret = ocfs2_error(inode->i_sb,
>> +					  "Inode %lu has bad extent record (%u, %u, 0) in xattr\n",
>> +					  inode->i_ino,
>> +					  le32_to_cpu(rec->e_cpos),
>> +					  ocfs2_rec_clusters(el, rec));
>> +		else
>> +			ret = ocfs2_error(inode->i_sb,
>> +					  "Inode %lu has bad extent record (NULL) in xattr\n",
>> +					  inode->i_ino);
>>  		goto out;
>>  	}
>>  
>> -- 
>> 2.25.1
>> 
>>