[lvc-project] [PATCH] ocfs2: add extent record check in ocfs2_commit_truncate()

[Glass Su]

> On Dec 17, 2024, at 18:43, Dmitry Antipov <dmantipov at yandex.ru> wrote:
> 
> Syzbot has reported the following BUG():
> 
> Kernel BUG at fs/ocfs2/alloc.c:686!
> ...
> RIP: 0010:ocfs2_new_path fs/ocfs2/alloc.c:686 [inline]
> RIP: 0010:ocfs2_commit_truncate+0x2547/0x2550 fs/ocfs2/alloc.c:7234
> Call Trace:
> <TASK>
> ocfs2_truncate_for_delete fs/ocfs2/inode.c:623 [inline]
> ocfs2_wipe_inode fs/ocfs2/inode.c:790 [inline]
> ocfs2_delete_inode fs/ocfs2/inode.c:1079 [inline]
> ocfs2_evict_inode+0x1c46/0x4630 fs/ocfs2/inode.c:1216
> evict+0x4e8/0x9a0 fs/inode.c:796
> d_delete_notify include/linux/fsnotify.h:332 [inline]
> vfs_rmdir+0x3d7/0x510 fs/namei.c:4407
> do_rmdir+0x3b5/0x580 fs/namei.c:4453
> __do_sys_rmdir fs/namei.c:4472 [inline]
> __se_sys_rmdir fs/namei.c:4470 [inline]
> __x64_sys_rmdir+0x47/0x50 fs/namei.c:4470
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> ...
> </TASK>
> 
> In 'ocfs2_commit_truncate()', add an extra extent tree depth check
> to refuse further operations on (presumably) corrupted on-disk inode.

The BUG_ON is in ocfs2_new_path(). The grace way to avoid further
similar fuzz reports is that change the BUG_ON() to check and return
ERR_PTR(-EIO) but it needs change of all call sites. For now, I think
 the fix is fine.

> Reported-by: syzbot+c16daba279a1161acfb0 at syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c16daba279a1161acfb0
> Fixes: 78f94673d7fa ("Ocfs2: Optimize ocfs2 truncate to use ocfs2_remove_btree_range() instead.")
> Signed-off-by: Dmitry Antipov <dmantipov at yandex.ru>
> ---
> fs/ocfs2/alloc.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 395e23920632..040aa2694f53 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -7219,12 +7219,20 @@ int ocfs2_commit_truncate(struct ocfs2_super *osb,
> struct ocfs2_extent_rec *rec;
> struct ocfs2_path *path = NULL;
> struct ocfs2_dinode *di = (struct ocfs2_dinode *)di_bh->b_data;
> + u16 tree_depth = le16_to_cpu(di->id2.i_list.l_tree_depth);

No need of tree_depth. Just call
If (unlikely(le16_to_cpu(di->id2.i_list.l_tree_depth) >= OCFS2_MAX_PATH_DEPTH))  {
...
> struct ocfs2_extent_list *root_el = &(di->id2.i_list);
> u64 refcount_loc = le64_to_cpu(di->i_refcount_loc);
> struct ocfs2_extent_tree et;
> struct ocfs2_cached_dealloc_ctxt dealloc;
> struct ocfs2_refcount_tree *ref_tree = NULL;
> 
> + if (unlikely(tree_depth >= OCFS2_MAX_PATH_DEPTH)) {
> + ocfs2_error(inode->i_sb,
> +    "dinode %llu has invalid extent record depth %u\n",
> +    (u64)di_bh->b_blocknr, tree_depth);
> + return -EINVAL;

-EIO is better.

— 
Su
> + }
> +
> ocfs2_init_dinode_extent_tree(&et, INODE_CACHE(inode), di_bh);
> ocfs2_init_dealloc_ctxt(&dealloc);
> 
> -- 
> 2.47.1
> 
>