[PATCH v2] ocfs2: fix oob in __ocfs2_find_path

Edward Adam Davis posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
fs/ocfs2/alloc.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
[PATCH v2] ocfs2: fix oob in __ocfs2_find_path
Posted by Edward Adam Davis 1 month, 2 weeks ago
Patch 2f26f58df041 modifies the definition of the array l_recs, specifying
the number of its members as l_count. In the path shown in [1], it will
first read the inode block where the system file directory is located
from the disk, and then read the l_count of each extension block from
the disk according to the extension list of the directory. Then the
value is 0 before l_count is read. If the array l_recs member is directly
accessed, the oob in [1] will be triggered.

The loop terminates when l_count is 0, similar to when next_free is 0.

[1]
UBSAN: array-index-out-of-bounds in fs/ocfs2/alloc.c:1838:11
index 0 is out of range for type 'struct ocfs2_extent_rec[] __counted_by(l_count)' (aka 'struct ocfs2_extent_rec[]')
Call Trace:
 __ocfs2_find_path+0x606/0xa40 fs/ocfs2/alloc.c:1838
 ocfs2_find_leaf+0xab/0x1c0 fs/ocfs2/alloc.c:1946
 ocfs2_get_clusters_nocache+0x172/0xc60 fs/ocfs2/extent_map.c:418
 ocfs2_get_clusters+0x505/0xa70 fs/ocfs2/extent_map.c:631
 ocfs2_extent_map_get_blocks+0x202/0x6a0 fs/ocfs2/extent_map.c:678
 ocfs2_read_virt_blocks+0x286/0x930 fs/ocfs2/extent_map.c:1001
 ocfs2_read_dir_block fs/ocfs2/dir.c:521 [inline]
 ocfs2_find_entry_el fs/ocfs2/dir.c:728 [inline]
 ocfs2_find_entry+0x3e4/0x2090 fs/ocfs2/dir.c:1120
 ocfs2_find_files_on_disk+0xdf/0x310 fs/ocfs2/dir.c:2023
 ocfs2_lookup_ino_from_name+0x52/0x100 fs/ocfs2/dir.c:2045
 _ocfs2_get_system_file_inode fs/ocfs2/sysfile.c:136 [inline]
 ocfs2_get_system_file_inode+0x326/0x770 fs/ocfs2/sysfile.c:112
 ocfs2_init_global_system_inodes+0x319/0x660 fs/ocfs2/super.c:461
 ocfs2_initialize_super fs/ocfs2/super.c:2196 [inline]
 ocfs2_fill_super+0x4432/0x65b0 fs/ocfs2/super.c:993
 get_tree_bdev_flags+0x40e/0x4d0 fs/super.c:1691
 vfs_get_tree+0x92/0x2a0 fs/super.c:1751
 fc_mount fs/namespace.c:1199 [inline]

Fixes: 2f26f58df041 ("ocfs2: annotate flexible array members with __counted_by_le()")
Reported-by: syzbot+151afab124dfbc5f15e6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=151afab124dfbc5f15e6
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
v1 -> v2: check l_count and update comments

 fs/ocfs2/alloc.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index 58bf58b68955..44bbc51276de 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -1812,14 +1812,16 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
 			ret = -EROFS;
 			goto out;
 		}
-		if (le16_to_cpu(el->l_next_free_rec) == 0) {
+		if (le16_to_cpu(el->l_next_free_rec) == 0 ||
+		    le16_to_cpu(el->l_count) == 0) {
 			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
-				    "Owner %llu has empty extent list at depth %u\n",
+				    "Owner %llu has empty extent list at depth %u "
+				    "(next free=%u count=%u)\n",
 				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
-				    le16_to_cpu(el->l_tree_depth));
+				    le16_to_cpu(el->l_tree_depth),
+				    le16_to_cpu(el->l_next_free_rec), le16_to_cpu(el->l_count));
 			ret = -EROFS;
 			goto out;
-
 		}
 
 		for(i = 0; i < le16_to_cpu(el->l_next_free_rec) - 1; i++) {
-- 
2.43.0
Re: [PATCH v2] ocfs2: fix oob in __ocfs2_find_path
Posted by Heming Zhao 1 month, 2 weeks ago
On Fri, Dec 19, 2025 at 12:55:06PM +0800, Edward Adam Davis wrote:
> Patch 2f26f58df041 modifies the definition of the array l_recs, specifying
> the number of its members as l_count. In the path shown in [1], it will
> first read the inode block where the system file directory is located
> from the disk, and then read the l_count of each extension block from
> the disk according to the extension list of the directory. Then the
> value is 0 before l_count is read. If the array l_recs member is directly
> accessed, the oob in [1] will be triggered.

Please revise the commit log.
I noticed a mistake in my previous email. While __ocfs2_find_path() does issue
IOs via ocfs2_read_extent_block(), the claim that "the value is 0 before
l_count is read" cannot be true in this context. All data for struct
ocfs2_extent_list (el) is fully read from the disk before the OOB issue is
triggered.

> 
> The loop terminates when l_count is 0, similar to when next_free is 0.
> 
> [1]
> UBSAN: array-index-out-of-bounds in fs/ocfs2/alloc.c:1838:11
> index 0 is out of range for type 'struct ocfs2_extent_rec[] __counted_by(l_count)' (aka 'struct ocfs2_extent_rec[]')
> Call Trace:
>  __ocfs2_find_path+0x606/0xa40 fs/ocfs2/alloc.c:1838
>  ocfs2_find_leaf+0xab/0x1c0 fs/ocfs2/alloc.c:1946
>  ocfs2_get_clusters_nocache+0x172/0xc60 fs/ocfs2/extent_map.c:418
>  ocfs2_get_clusters+0x505/0xa70 fs/ocfs2/extent_map.c:631
>  ocfs2_extent_map_get_blocks+0x202/0x6a0 fs/ocfs2/extent_map.c:678
>  ocfs2_read_virt_blocks+0x286/0x930 fs/ocfs2/extent_map.c:1001
>  ocfs2_read_dir_block fs/ocfs2/dir.c:521 [inline]
>  ocfs2_find_entry_el fs/ocfs2/dir.c:728 [inline]
>  ocfs2_find_entry+0x3e4/0x2090 fs/ocfs2/dir.c:1120
>  ocfs2_find_files_on_disk+0xdf/0x310 fs/ocfs2/dir.c:2023
>  ocfs2_lookup_ino_from_name+0x52/0x100 fs/ocfs2/dir.c:2045
>  _ocfs2_get_system_file_inode fs/ocfs2/sysfile.c:136 [inline]
>  ocfs2_get_system_file_inode+0x326/0x770 fs/ocfs2/sysfile.c:112
>  ocfs2_init_global_system_inodes+0x319/0x660 fs/ocfs2/super.c:461
>  ocfs2_initialize_super fs/ocfs2/super.c:2196 [inline]
>  ocfs2_fill_super+0x4432/0x65b0 fs/ocfs2/super.c:993
>  get_tree_bdev_flags+0x40e/0x4d0 fs/super.c:1691
>  vfs_get_tree+0x92/0x2a0 fs/super.c:1751
>  fc_mount fs/namespace.c:1199 [inline]
> 
> Fixes: 2f26f58df041 ("ocfs2: annotate flexible array members with __counted_by_le()")
> Reported-by: syzbot+151afab124dfbc5f15e6@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=151afab124dfbc5f15e6
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> v1 -> v2: check l_count and update comments
> 
>  fs/ocfs2/alloc.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 58bf58b68955..44bbc51276de 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -1812,14 +1812,16 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
>  			ret = -EROFS;
>  			goto out;
>  		}
> -		if (le16_to_cpu(el->l_next_free_rec) == 0) {
> +		if (le16_to_cpu(el->l_next_free_rec) == 0 ||
> +		    le16_to_cpu(el->l_count) == 0) {

please restore the code above to my original patch. Since the value is expected
to be zero, converting from le16 to cpu endian is a redundant operation and
waste of cpu cycles.

- Heming
>  			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
> -				    "Owner %llu has empty extent list at depth %u\n",
> +				    "Owner %llu has empty extent list at depth %u "
> +				    "(next free=%u count=%u)\n",
>  				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
> -				    le16_to_cpu(el->l_tree_depth));
> +				    le16_to_cpu(el->l_tree_depth),
> +				    le16_to_cpu(el->l_next_free_rec), le16_to_cpu(el->l_count));
>  			ret = -EROFS;
>  			goto out;
> -
>  		}
>  
>  		for(i = 0; i < le16_to_cpu(el->l_next_free_rec) - 1; i++) {
> -- 
> 2.43.0
>
Re: [PATCH v2] ocfs2: fix oob in __ocfs2_find_path
Posted by Edward Adam Davis 1 month, 2 weeks ago
On Fri, 19 Dec 2025 14:31:58 +0800, Heming Zhao wrote:
> > Patch 2f26f58df041 modifies the definition of the array l_recs, specifying
> > the number of its members as l_count. In the path shown in [1], it will
> > first read the inode block where the system file directory is located
> > from the disk, and then read the l_count of each extension block from
> > the disk according to the extension list of the directory. Then the
> > value is 0 before l_count is read. If the array l_recs member is directly
> > accessed, the oob in [1] will be triggered.
> 
> Please revise the commit log.
> I noticed a mistake in my previous email. While __ocfs2_find_path() does issue
> IOs via ocfs2_read_extent_block(), the claim that "the value is 0 before
> l_count is read" cannot be true in this context. All data for struct
> ocfs2_extent_list (el) is fully read from the disk before the OOB issue is
> triggered.
Upon closer inspection, yes, the root extent list has been read from
the disk, and e_blkno is being retrieved to read subsequent extension
blocks. However, an error occurred while reading e_blkno because the
l_count value of the root extent list is 0.
Therefore, the Fix tag is incorrect; it shouldn't be 2f26f58df041,
but rather dcd0538ff4e8 ("ocfs2: sparse b-tree support").
> 
> > The loop terminates when l_count is 0, similar to when next_free is 0.
> >
> > [1]
> > UBSAN: array-index-out-of-bounds in fs/ocfs2/alloc.c:1838:11
> > index 0 is out of range for type 'struct ocfs2_extent_rec[] __counted_by(l_count)' (aka 'struct ocfs2_extent_rec[]')
> > Call Trace:
> >  __ocfs2_find_path+0x606/0xa40 fs/ocfs2/alloc.c:1838
> >  ocfs2_find_leaf+0xab/0x1c0 fs/ocfs2/alloc.c:1946
> >  ocfs2_get_clusters_nocache+0x172/0xc60 fs/ocfs2/extent_map.c:418
> >  ocfs2_get_clusters+0x505/0xa70 fs/ocfs2/extent_map.c:631
> >  ocfs2_extent_map_get_blocks+0x202/0x6a0 fs/ocfs2/extent_map.c:678
> >  ocfs2_read_virt_blocks+0x286/0x930 fs/ocfs2/extent_map.c:1001
> >  ocfs2_read_dir_block fs/ocfs2/dir.c:521 [inline]
> >  ocfs2_find_entry_el fs/ocfs2/dir.c:728 [inline]
> >  ocfs2_find_entry+0x3e4/0x2090 fs/ocfs2/dir.c:1120
> >  ocfs2_find_files_on_disk+0xdf/0x310 fs/ocfs2/dir.c:2023
> >  ocfs2_lookup_ino_from_name+0x52/0x100 fs/ocfs2/dir.c:2045
> >  _ocfs2_get_system_file_inode fs/ocfs2/sysfile.c:136 [inline]
> >  ocfs2_get_system_file_inode+0x326/0x770 fs/ocfs2/sysfile.c:112
> >  ocfs2_init_global_system_inodes+0x319/0x660 fs/ocfs2/super.c:461
> >  ocfs2_initialize_super fs/ocfs2/super.c:2196 [inline]
> >  ocfs2_fill_super+0x4432/0x65b0 fs/ocfs2/super.c:993
> >  get_tree_bdev_flags+0x40e/0x4d0 fs/super.c:1691
> >  vfs_get_tree+0x92/0x2a0 fs/super.c:1751
> >  fc_mount fs/namespace.c:1199 [inline]
> >
> > Fixes: 2f26f58df041 ("ocfs2: annotate flexible array members with __counted_by_le()")
> > Reported-by: syzbot+151afab124dfbc5f15e6@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=151afab124dfbc5f15e6
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> > v1 -> v2: check l_count and update comments
> >
> >  fs/ocfs2/alloc.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> > index 58bf58b68955..44bbc51276de 100644
> > --- a/fs/ocfs2/alloc.c
> > +++ b/fs/ocfs2/alloc.c
> > @@ -1812,14 +1812,16 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
> >  			ret = -EROFS;
> >  			goto out;
> >  		}
> > -		if (le16_to_cpu(el->l_next_free_rec) == 0) {
> > +		if (le16_to_cpu(el->l_next_free_rec) == 0 ||
> > +		    le16_to_cpu(el->l_count) == 0) {
> 
> please restore the code above to my original patch. Since the value is expected
> to be zero, converting from le16 to cpu endian is a redundant operation and
> waste of cpu cycles.
My view remains unchanged, because l_count/l_next_free_rec originates
from the disk, not from memory.
Re: [PATCH v2] ocfs2: fix oob in __ocfs2_find_path
Posted by Heming Zhao 1 month, 2 weeks ago
On Fri, Dec 19, 2025 at 03:00:02PM +0800, Edward Adam Davis wrote:
> On Fri, 19 Dec 2025 14:31:58 +0800, Heming Zhao wrote:
> > > Patch 2f26f58df041 modifies the definition of the array l_recs, specifying
> > > the number of its members as l_count. In the path shown in [1], it will
> > > first read the inode block where the system file directory is located
> > > from the disk, and then read the l_count of each extension block from
> > > the disk according to the extension list of the directory. Then the
> > > value is 0 before l_count is read. If the array l_recs member is directly
> > > accessed, the oob in [1] will be triggered.
> > 
> > Please revise the commit log.
> > I noticed a mistake in my previous email. While __ocfs2_find_path() does issue
> > IOs via ocfs2_read_extent_block(), the claim that "the value is 0 before
> > l_count is read" cannot be true in this context. All data for struct
> > ocfs2_extent_list (el) is fully read from the disk before the OOB issue is
> > triggered.
> Upon closer inspection, yes, the root extent list has been read from
> the disk, and e_blkno is being retrieved to read subsequent extension
> blocks. However, an error occurred while reading e_blkno because the
> l_count value of the root extent list is 0.
> Therefore, the Fix tag is incorrect; it shouldn't be 2f26f58df041,
> but rather dcd0538ff4e8 ("ocfs2: sparse b-tree support").
> > 
> > > The loop terminates when l_count is 0, similar to when next_free is 0.
> > >
> > > [1]
> > > UBSAN: array-index-out-of-bounds in fs/ocfs2/alloc.c:1838:11
> > > index 0 is out of range for type 'struct ocfs2_extent_rec[] __counted_by(l_count)' (aka 'struct ocfs2_extent_rec[]')
> > > Call Trace:
> > >  __ocfs2_find_path+0x606/0xa40 fs/ocfs2/alloc.c:1838
> > >  ocfs2_find_leaf+0xab/0x1c0 fs/ocfs2/alloc.c:1946
> > >  ocfs2_get_clusters_nocache+0x172/0xc60 fs/ocfs2/extent_map.c:418
> > >  ocfs2_get_clusters+0x505/0xa70 fs/ocfs2/extent_map.c:631
> > >  ocfs2_extent_map_get_blocks+0x202/0x6a0 fs/ocfs2/extent_map.c:678
> > >  ocfs2_read_virt_blocks+0x286/0x930 fs/ocfs2/extent_map.c:1001
> > >  ocfs2_read_dir_block fs/ocfs2/dir.c:521 [inline]
> > >  ocfs2_find_entry_el fs/ocfs2/dir.c:728 [inline]
> > >  ocfs2_find_entry+0x3e4/0x2090 fs/ocfs2/dir.c:1120
> > >  ocfs2_find_files_on_disk+0xdf/0x310 fs/ocfs2/dir.c:2023
> > >  ocfs2_lookup_ino_from_name+0x52/0x100 fs/ocfs2/dir.c:2045
> > >  _ocfs2_get_system_file_inode fs/ocfs2/sysfile.c:136 [inline]
> > >  ocfs2_get_system_file_inode+0x326/0x770 fs/ocfs2/sysfile.c:112
> > >  ocfs2_init_global_system_inodes+0x319/0x660 fs/ocfs2/super.c:461
> > >  ocfs2_initialize_super fs/ocfs2/super.c:2196 [inline]
> > >  ocfs2_fill_super+0x4432/0x65b0 fs/ocfs2/super.c:993
> > >  get_tree_bdev_flags+0x40e/0x4d0 fs/super.c:1691
> > >  vfs_get_tree+0x92/0x2a0 fs/super.c:1751
> > >  fc_mount fs/namespace.c:1199 [inline]
> > >
> > > Fixes: 2f26f58df041 ("ocfs2: annotate flexible array members with __counted_by_le()")

This commit doesn't introduce any bugs, rather, it introduces stricter sanity
checks for struct usage. Please remove the 'Fixes' tag.

The code logic of this function is correct. This issue is triggered by a
syzbot-crafted image and is very unlikely to occur in the real world. I would
prefer not to add a 'Fixes:' tag, as doing so would increase the backporting
workload for Linux distribution maintainers.

> > > Reported-by: syzbot+151afab124dfbc5f15e6@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=151afab124dfbc5f15e6
> > > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > > ---
> > > v1 -> v2: check l_count and update comments
> > >
> > >  fs/ocfs2/alloc.c | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> > > index 58bf58b68955..44bbc51276de 100644
> > > --- a/fs/ocfs2/alloc.c
> > > +++ b/fs/ocfs2/alloc.c
> > > @@ -1812,14 +1812,16 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
> > >  			ret = -EROFS;
> > >  			goto out;
> > >  		}
> > > -		if (le16_to_cpu(el->l_next_free_rec) == 0) {
> > > +		if (le16_to_cpu(el->l_next_free_rec) == 0 ||
> > > +		    le16_to_cpu(el->l_count) == 0) {
> > 
> > please restore the code above to my original patch. Since the value is expected
> > to be zero, converting from le16 to cpu endian is a redundant operation and
> > waste of cpu cycles.
> My view remains unchanged, because l_count/l_next_free_rec originates
> from the disk, not from memory.
> 

What are the bits representation of zero? Is byte-swapping necessary for
different-endian architectures?" We just need to perform the conversion in the
error msg log. 

- Heming
Re: [PATCH v2] ocfs2: fix oob in __ocfs2_find_path
Posted by Edward Adam Davis 1 month, 2 weeks ago
On Fri, 19 Dec 2025 14:31:58 +0800, Heming Zhao wrote:
> What are the bits representation of zero? Is byte-swapping necessary for
> different-endian architectures?" We just need to perform the conversion in the
> error msg log. 
That's valid point.
[PATCH v3] ocfs2: fix oob in __ocfs2_find_path
Posted by Edward Adam Davis 1 month, 2 weeks ago
syzbot constructed a corrupted image, which resulted in the l_count
value of the root element being 0. Since the length of the l_recs
array depends on l_count, reading its member e_blkno triggered the
out-of-bounds access reported by syzbot in [1].

The loop terminates when l_count is 0, similar to when next_free is 0.

[1]
UBSAN: array-index-out-of-bounds in fs/ocfs2/alloc.c:1838:11
index 0 is out of range for type 'struct ocfs2_extent_rec[] __counted_by(l_count)' (aka 'struct ocfs2_extent_rec[]')
Call Trace:
 __ocfs2_find_path+0x606/0xa40 fs/ocfs2/alloc.c:1838
 ocfs2_find_leaf+0xab/0x1c0 fs/ocfs2/alloc.c:1946
 ocfs2_get_clusters_nocache+0x172/0xc60 fs/ocfs2/extent_map.c:418
 ocfs2_get_clusters+0x505/0xa70 fs/ocfs2/extent_map.c:631
 ocfs2_extent_map_get_blocks+0x202/0x6a0 fs/ocfs2/extent_map.c:678
 ocfs2_read_virt_blocks+0x286/0x930 fs/ocfs2/extent_map.c:1001
 ocfs2_read_dir_block fs/ocfs2/dir.c:521 [inline]
 ocfs2_find_entry_el fs/ocfs2/dir.c:728 [inline]
 ocfs2_find_entry+0x3e4/0x2090 fs/ocfs2/dir.c:1120
 ocfs2_find_files_on_disk+0xdf/0x310 fs/ocfs2/dir.c:2023
 ocfs2_lookup_ino_from_name+0x52/0x100 fs/ocfs2/dir.c:2045
 _ocfs2_get_system_file_inode fs/ocfs2/sysfile.c:136 [inline]
 ocfs2_get_system_file_inode+0x326/0x770 fs/ocfs2/sysfile.c:112
 ocfs2_init_global_system_inodes+0x319/0x660 fs/ocfs2/super.c:461
 ocfs2_initialize_super fs/ocfs2/super.c:2196 [inline]
 ocfs2_fill_super+0x4432/0x65b0 fs/ocfs2/super.c:993
 get_tree_bdev_flags+0x40e/0x4d0 fs/super.c:1691
 vfs_get_tree+0x92/0x2a0 fs/super.c:1751
 fc_mount fs/namespace.c:1199 [inline]

Reported-by: syzbot+151afab124dfbc5f15e6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=151afab124dfbc5f15e6
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
v2 -> v3: remove le16 and Fix tag and update comments
v1 -> v2: check l_count and update comments

 fs/ocfs2/alloc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index 58bf58b68955..b7db177d17d6 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -1812,14 +1812,15 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
 			ret = -EROFS;
 			goto out;
 		}
-		if (le16_to_cpu(el->l_next_free_rec) == 0) {
+		if (!el->l_next_free_rec || !el->l_count) {
 			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
-				    "Owner %llu has empty extent list at depth %u\n",
+				    "Owner %llu has empty extent list at depth %u\n"
+				    "(next free=%u count=%u)\n",
 				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
-				    le16_to_cpu(el->l_tree_depth));
+				    le16_to_cpu(el->l_tree_depth),
+				    le16_to_cpu(el->l_next_free_rec), le16_to_cpu(el->l_count));
 			ret = -EROFS;
 			goto out;
-
 		}
 
 		for(i = 0; i < le16_to_cpu(el->l_next_free_rec) - 1; i++) {
-- 
2.43.0
Re: [PATCH v3] ocfs2: fix oob in __ocfs2_find_path
Posted by Heming Zhao 1 month, 2 weeks ago
On Fri, Dec 19, 2025 at 03:43:32PM +0800, Edward Adam Davis wrote:
> syzbot constructed a corrupted image, which resulted in the l_count
> value of the root element being 0. Since the length of the l_recs
> array depends on l_count, reading its member e_blkno triggered the
> out-of-bounds access reported by syzbot in [1].

for: "which resulted in the l_count of the root element being 0"
This issue occurs not only in the root extent but also in every node of the
b-tree. It's better to use a more general description, such as:
which resulted in el->l_count from the b-tree extent block being 0.

- Heming
> 
> The loop terminates when l_count is 0, similar to when next_free is 0.
> 
> [1]
> UBSAN: array-index-out-of-bounds in fs/ocfs2/alloc.c:1838:11
> index 0 is out of range for type 'struct ocfs2_extent_rec[] __counted_by(l_count)' (aka 'struct ocfs2_extent_rec[]')
> Call Trace:
>  __ocfs2_find_path+0x606/0xa40 fs/ocfs2/alloc.c:1838
>  ocfs2_find_leaf+0xab/0x1c0 fs/ocfs2/alloc.c:1946
>  ocfs2_get_clusters_nocache+0x172/0xc60 fs/ocfs2/extent_map.c:418
>  ocfs2_get_clusters+0x505/0xa70 fs/ocfs2/extent_map.c:631
>  ocfs2_extent_map_get_blocks+0x202/0x6a0 fs/ocfs2/extent_map.c:678
>  ocfs2_read_virt_blocks+0x286/0x930 fs/ocfs2/extent_map.c:1001
>  ocfs2_read_dir_block fs/ocfs2/dir.c:521 [inline]
>  ocfs2_find_entry_el fs/ocfs2/dir.c:728 [inline]
>  ocfs2_find_entry+0x3e4/0x2090 fs/ocfs2/dir.c:1120
>  ocfs2_find_files_on_disk+0xdf/0x310 fs/ocfs2/dir.c:2023
>  ocfs2_lookup_ino_from_name+0x52/0x100 fs/ocfs2/dir.c:2045
>  _ocfs2_get_system_file_inode fs/ocfs2/sysfile.c:136 [inline]
>  ocfs2_get_system_file_inode+0x326/0x770 fs/ocfs2/sysfile.c:112
>  ocfs2_init_global_system_inodes+0x319/0x660 fs/ocfs2/super.c:461
>  ocfs2_initialize_super fs/ocfs2/super.c:2196 [inline]
>  ocfs2_fill_super+0x4432/0x65b0 fs/ocfs2/super.c:993
>  get_tree_bdev_flags+0x40e/0x4d0 fs/super.c:1691
>  vfs_get_tree+0x92/0x2a0 fs/super.c:1751
>  fc_mount fs/namespace.c:1199 [inline]
> 
> Reported-by: syzbot+151afab124dfbc5f15e6@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=151afab124dfbc5f15e6
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> v2 -> v3: remove le16 and Fix tag and update comments
> v1 -> v2: check l_count and update comments
> 
>  fs/ocfs2/alloc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 58bf58b68955..b7db177d17d6 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -1812,14 +1812,15 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
>  			ret = -EROFS;
>  			goto out;
>  		}
> -		if (le16_to_cpu(el->l_next_free_rec) == 0) {
> +		if (!el->l_next_free_rec || !el->l_count) {
>  			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
> -				    "Owner %llu has empty extent list at depth %u\n",
> +				    "Owner %llu has empty extent list at depth %u\n"
> +				    "(next free=%u count=%u)\n",
>  				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
> -				    le16_to_cpu(el->l_tree_depth));
> +				    le16_to_cpu(el->l_tree_depth),
> +				    le16_to_cpu(el->l_next_free_rec), le16_to_cpu(el->l_count));
>  			ret = -EROFS;
>  			goto out;
> -
>  		}
>  
>  		for(i = 0; i < le16_to_cpu(el->l_next_free_rec) - 1; i++) {
> -- 
> 2.43.0
>
[PATCH v4] ocfs2: fix oob in __ocfs2_find_path
Posted by Edward Adam Davis 1 month, 2 weeks ago
syzbot constructed a corrupted image, which resulted in el->l_count
from the b-tree extent block being 0. Since the length of the l_recs
array depends on l_count, reading its member e_blkno triggered the
out-of-bounds access reported by syzbot in [1].

The loop terminates when l_count is 0, similar to when next_free is 0.

[1]
UBSAN: array-index-out-of-bounds in fs/ocfs2/alloc.c:1838:11
index 0 is out of range for type 'struct ocfs2_extent_rec[] __counted_by(l_count)' (aka 'struct ocfs2_extent_rec[]')
Call Trace:
 __ocfs2_find_path+0x606/0xa40 fs/ocfs2/alloc.c:1838
 ocfs2_find_leaf+0xab/0x1c0 fs/ocfs2/alloc.c:1946
 ocfs2_get_clusters_nocache+0x172/0xc60 fs/ocfs2/extent_map.c:418
 ocfs2_get_clusters+0x505/0xa70 fs/ocfs2/extent_map.c:631
 ocfs2_extent_map_get_blocks+0x202/0x6a0 fs/ocfs2/extent_map.c:678
 ocfs2_read_virt_blocks+0x286/0x930 fs/ocfs2/extent_map.c:1001
 ocfs2_read_dir_block fs/ocfs2/dir.c:521 [inline]
 ocfs2_find_entry_el fs/ocfs2/dir.c:728 [inline]
 ocfs2_find_entry+0x3e4/0x2090 fs/ocfs2/dir.c:1120
 ocfs2_find_files_on_disk+0xdf/0x310 fs/ocfs2/dir.c:2023
 ocfs2_lookup_ino_from_name+0x52/0x100 fs/ocfs2/dir.c:2045
 _ocfs2_get_system_file_inode fs/ocfs2/sysfile.c:136 [inline]
 ocfs2_get_system_file_inode+0x326/0x770 fs/ocfs2/sysfile.c:112
 ocfs2_init_global_system_inodes+0x319/0x660 fs/ocfs2/super.c:461
 ocfs2_initialize_super fs/ocfs2/super.c:2196 [inline]
 ocfs2_fill_super+0x4432/0x65b0 fs/ocfs2/super.c:993
 get_tree_bdev_flags+0x40e/0x4d0 fs/super.c:1691
 vfs_get_tree+0x92/0x2a0 fs/super.c:1751
 fc_mount fs/namespace.c:1199 [inline]

Reported-by: syzbot+151afab124dfbc5f15e6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=151afab124dfbc5f15e6
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
v3 -> v4: update comments
v2 -> v3: remove le16 and Fix tag and update comments
v1 -> v2: check l_count and update comments

 fs/ocfs2/alloc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index 58bf58b68955..b7db177d17d6 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -1812,14 +1812,15 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
 			ret = -EROFS;
 			goto out;
 		}
-		if (le16_to_cpu(el->l_next_free_rec) == 0) {
+		if (!el->l_next_free_rec || !el->l_count) {
 			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
-				    "Owner %llu has empty extent list at depth %u\n",
+				    "Owner %llu has empty extent list at depth %u\n"
+				    "(next free=%u count=%u)\n",
 				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
-				    le16_to_cpu(el->l_tree_depth));
+				    le16_to_cpu(el->l_tree_depth),
+				    le16_to_cpu(el->l_next_free_rec), le16_to_cpu(el->l_count));
 			ret = -EROFS;
 			goto out;
-
 		}
 
 		for(i = 0; i < le16_to_cpu(el->l_next_free_rec) - 1; i++) {
-- 
2.43.0
Re: [PATCH v4] ocfs2: fix oob in __ocfs2_find_path
Posted by Heming Zhao 1 month, 2 weeks ago
On Fri, Dec 19, 2025 at 04:31:52PM +0800, Edward Adam Davis wrote:
> syzbot constructed a corrupted image, which resulted in el->l_count
> from the b-tree extent block being 0. Since the length of the l_recs
> array depends on l_count, reading its member e_blkno triggered the
> out-of-bounds access reported by syzbot in [1].
> 
> The loop terminates when l_count is 0, similar to when next_free is 0.
> 
> [1]
> UBSAN: array-index-out-of-bounds in fs/ocfs2/alloc.c:1838:11
> index 0 is out of range for type 'struct ocfs2_extent_rec[] __counted_by(l_count)' (aka 'struct ocfs2_extent_rec[]')
> Call Trace:
>  __ocfs2_find_path+0x606/0xa40 fs/ocfs2/alloc.c:1838
>  ocfs2_find_leaf+0xab/0x1c0 fs/ocfs2/alloc.c:1946
>  ocfs2_get_clusters_nocache+0x172/0xc60 fs/ocfs2/extent_map.c:418
>  ocfs2_get_clusters+0x505/0xa70 fs/ocfs2/extent_map.c:631
>  ocfs2_extent_map_get_blocks+0x202/0x6a0 fs/ocfs2/extent_map.c:678
>  ocfs2_read_virt_blocks+0x286/0x930 fs/ocfs2/extent_map.c:1001
>  ocfs2_read_dir_block fs/ocfs2/dir.c:521 [inline]
>  ocfs2_find_entry_el fs/ocfs2/dir.c:728 [inline]
>  ocfs2_find_entry+0x3e4/0x2090 fs/ocfs2/dir.c:1120
>  ocfs2_find_files_on_disk+0xdf/0x310 fs/ocfs2/dir.c:2023
>  ocfs2_lookup_ino_from_name+0x52/0x100 fs/ocfs2/dir.c:2045
>  _ocfs2_get_system_file_inode fs/ocfs2/sysfile.c:136 [inline]
>  ocfs2_get_system_file_inode+0x326/0x770 fs/ocfs2/sysfile.c:112
>  ocfs2_init_global_system_inodes+0x319/0x660 fs/ocfs2/super.c:461
>  ocfs2_initialize_super fs/ocfs2/super.c:2196 [inline]
>  ocfs2_fill_super+0x4432/0x65b0 fs/ocfs2/super.c:993
>  get_tree_bdev_flags+0x40e/0x4d0 fs/super.c:1691
>  vfs_get_tree+0x92/0x2a0 fs/super.c:1751
>  fc_mount fs/namespace.c:1199 [inline]
> 
> Reported-by: syzbot+151afab124dfbc5f15e6@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=151afab124dfbc5f15e6
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>

LGTM.
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
> v3 -> v4: update comments
> v2 -> v3: remove le16 and Fix tag and update comments
> v1 -> v2: check l_count and update comments
> 
>  fs/ocfs2/alloc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 58bf58b68955..b7db177d17d6 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -1812,14 +1812,15 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
>  			ret = -EROFS;
>  			goto out;
>  		}
> -		if (le16_to_cpu(el->l_next_free_rec) == 0) {
> +		if (!el->l_next_free_rec || !el->l_count) {
>  			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
> -				    "Owner %llu has empty extent list at depth %u\n",
> +				    "Owner %llu has empty extent list at depth %u\n"
> +				    "(next free=%u count=%u)\n",
>  				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
> -				    le16_to_cpu(el->l_tree_depth));
> +				    le16_to_cpu(el->l_tree_depth),
> +				    le16_to_cpu(el->l_next_free_rec), le16_to_cpu(el->l_count));
>  			ret = -EROFS;
>  			goto out;
> -
>  		}
>  
>  		for(i = 0; i < le16_to_cpu(el->l_next_free_rec) - 1; i++) {
> -- 
> 2.43.0
>
Re: [PATCH v4] ocfs2: fix oob in __ocfs2_find_path
Posted by Joseph Qi 1 month, 2 weeks ago

On 2025/12/19 18:07, Heming Zhao wrote:
> On Fri, Dec 19, 2025 at 04:31:52PM +0800, Edward Adam Davis wrote:
>> syzbot constructed a corrupted image, which resulted in el->l_count
>> from the b-tree extent block being 0. Since the length of the l_recs
>> array depends on l_count, reading its member e_blkno triggered the
>> out-of-bounds access reported by syzbot in [1].
>>
>> The loop terminates when l_count is 0, similar to when next_free is 0.
>>
>> [1]
>> UBSAN: array-index-out-of-bounds in fs/ocfs2/alloc.c:1838:11
>> index 0 is out of range for type 'struct ocfs2_extent_rec[] __counted_by(l_count)' (aka 'struct ocfs2_extent_rec[]')
>> Call Trace:
>>  __ocfs2_find_path+0x606/0xa40 fs/ocfs2/alloc.c:1838
>>  ocfs2_find_leaf+0xab/0x1c0 fs/ocfs2/alloc.c:1946
>>  ocfs2_get_clusters_nocache+0x172/0xc60 fs/ocfs2/extent_map.c:418
>>  ocfs2_get_clusters+0x505/0xa70 fs/ocfs2/extent_map.c:631
>>  ocfs2_extent_map_get_blocks+0x202/0x6a0 fs/ocfs2/extent_map.c:678
>>  ocfs2_read_virt_blocks+0x286/0x930 fs/ocfs2/extent_map.c:1001
>>  ocfs2_read_dir_block fs/ocfs2/dir.c:521 [inline]
>>  ocfs2_find_entry_el fs/ocfs2/dir.c:728 [inline]
>>  ocfs2_find_entry+0x3e4/0x2090 fs/ocfs2/dir.c:1120
>>  ocfs2_find_files_on_disk+0xdf/0x310 fs/ocfs2/dir.c:2023
>>  ocfs2_lookup_ino_from_name+0x52/0x100 fs/ocfs2/dir.c:2045
>>  _ocfs2_get_system_file_inode fs/ocfs2/sysfile.c:136 [inline]
>>  ocfs2_get_system_file_inode+0x326/0x770 fs/ocfs2/sysfile.c:112
>>  ocfs2_init_global_system_inodes+0x319/0x660 fs/ocfs2/super.c:461
>>  ocfs2_initialize_super fs/ocfs2/super.c:2196 [inline]
>>  ocfs2_fill_super+0x4432/0x65b0 fs/ocfs2/super.c:993
>>  get_tree_bdev_flags+0x40e/0x4d0 fs/super.c:1691
>>  vfs_get_tree+0x92/0x2a0 fs/super.c:1751
>>  fc_mount fs/namespace.c:1199 [inline]
>>
>> Reported-by: syzbot+151afab124dfbc5f15e6@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=151afab124dfbc5f15e6
>> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> 
> LGTM.
> Reviewed-by: Heming Zhao <heming.zhao@suse.com>

Acked-by: Joseph Qi <joseph.qi@linux.alibaba.com>
>> ---
>> v3 -> v4: update comments
>> v2 -> v3: remove le16 and Fix tag and update comments
>> v1 -> v2: check l_count and update comments
>>
>>  fs/ocfs2/alloc.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
>> index 58bf58b68955..b7db177d17d6 100644
>> --- a/fs/ocfs2/alloc.c
>> +++ b/fs/ocfs2/alloc.c
>> @@ -1812,14 +1812,15 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci,
>>  			ret = -EROFS;
>>  			goto out;
>>  		}
>> -		if (le16_to_cpu(el->l_next_free_rec) == 0) {
>> +		if (!el->l_next_free_rec || !el->l_count) {
>>  			ocfs2_error(ocfs2_metadata_cache_get_super(ci),
>> -				    "Owner %llu has empty extent list at depth %u\n",
>> +				    "Owner %llu has empty extent list at depth %u\n"
>> +				    "(next free=%u count=%u)\n",
>>  				    (unsigned long long)ocfs2_metadata_cache_owner(ci),
>> -				    le16_to_cpu(el->l_tree_depth));
>> +				    le16_to_cpu(el->l_tree_depth),
>> +				    le16_to_cpu(el->l_next_free_rec), le16_to_cpu(el->l_count));
>>  			ret = -EROFS;
>>  			goto out;
>> -
>>  		}
>>  
>>  		for(i = 0; i < le16_to_cpu(el->l_next_free_rec) - 1; i++) {
>> -- 
>> 2.43.0
>>