lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <3080b006-a093-4aad-ae7b-de7d0ae9d6d9@linux.alibaba.com>
Date: Tue, 28 Oct 2025 13:52:25 +0800
From: Joseph Qi <joseph.qi@...ux.alibaba.com>
To: Deepanshu Kartikey <kartikey406@...il.com>, mark@...heh.com,
 jlbec@...lplan.org, heming.zhao@...e.com, akpm <akpm@...ux-foundation.org>
Cc: ocfs2-devel@...ts.linux.dev, linux-kernel@...r.kernel.org,
 syzbot+fd8af97c7227fe605d95@...kaller.appspotmail.com
Subject: Re: [PATCH v3] ocfs2: validate cl_bpc in allocator inodes to prevent
 divide-by-zero



On 2025/10/28 12:29, Deepanshu Kartikey wrote:
> The chain allocator field cl_bpc (blocks per cluster) is read from disk
> and used in division operations without validation. A corrupted filesystem
> image with cl_bpc=0 causes a divide-by-zero crash in the kernel:
> 
>   divide error: 0000 [#1] PREEMPT SMP KASAN
>   RIP: 0010:ocfs2_bg_discontig_add_extent fs/ocfs2/suballoc.c:335 [inline]
>   RIP: 0010:ocfs2_block_group_fill+0x5bd/0xa70 fs/ocfs2/suballoc.c:386
>   Call Trace:
>    ocfs2_block_group_alloc+0x7e9/0x1330 fs/ocfs2/suballoc.c:703
>    ocfs2_reserve_suballoc_bits+0x20a6/0x4640 fs/ocfs2/suballoc.c:834
>    ocfs2_reserve_new_inode+0x4f4/0xcc0 fs/ocfs2/suballoc.c:1074
>    ocfs2_mknod+0x83c/0x2050 fs/ocfs2/namei.c:306
> 
> This patch adds validation in ocfs2_validate_inode_block() to ensure cl_bpc
> matches the expected value calculated from the superblock's cluster size
> and block size for chain allocator inodes (identified by OCFS2_CHAIN_FL).
> 
> Moving the validation to inode validation time (rather than allocation time)
> has several benefits:
> - Validates once when the inode is read, rather than on every allocation
> - Protects all code paths that use cl_bpc (allocation, resize, etc.)
> - Follows the existing pattern of inode validation in OCFS2
> - Centralizes validation logic
> 
> The validation catches both:
> - Zero values that cause divide-by-zero crashes
> - Non-zero but incorrect values indicating filesystem corruption or
>   mismatched filesystem geometry
> 
> With this fix, mounting a corrupted filesystem produces:
>   OCFS2: ERROR (device loop0): ocfs2_validate_inode_block: Inode 74
>          has corrupted cl_bpc: ondisk=0 expected=16
> 
> Instead of a kernel crash.
> 
> Link: https://lore.kernel.org/ocfs2-devel/20251026132625.12348-1-kartikey406@gmail.com/T/#u [v1]
> Link: https://lore.kernel.org/all/20251027124131.10002-1-kartikey406@gmail.com/T/ [v2]
> Reported-by: syzbot+fd8af97c7227fe605d95@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=fd8af97c7227fe605d95
> Tested-by: syzbot+fd8af97c7227fe605d95@...kaller.appspotmail.com
> Suggested-by: Joseph Qi <joseph.qi@...ux.alibaba.com>
> Signed-off-by: Deepanshu Kartikey <kartikey406@...il.com>

Looks fine.
Reviewed-by: Joseph Qi <joseph.qi@...ux.alibaba.com>

> ---
> Changes in v3:
> - Removed unnecessary local osb variable as suggested by Joseph Qi
> - Added blank line before rc = 0 for better readability
> 
> Changes in v2:
> - Moved validation from ocfs2_block_group_alloc() to ocfs2_validate_inode_block()
>   as suggested by Joseph Qi to benefit all code paths
> - Added OCFS2_CHAIN_FL check to only validate chain allocator inodes
> - Updated commit message to reflect the new location
> ---
>  fs/ocfs2/inode.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index fcc89856ab95..dbbfe72b1dc7 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -1502,6 +1502,21 @@ int ocfs2_validate_inode_block(struct super_block *sb,
>  				 le16_to_cpu(di->i_suballoc_slot));
>  		goto bail;
>  	}
> +	/* Validate cl_bpc for chain allocator inodes */
> +	if (le32_to_cpu(di->i_flags) & OCFS2_CHAIN_FL) {
> +		struct ocfs2_chain_list *cl = &di->id2.i_chain;
> +		u16 cl_bpc = le16_to_cpu(cl->cl_bpc);
> +		u16 expected_bpc = 1 << (OCFS2_SB(sb)->s_clustersize_bits -
> +					 sb->s_blocksize_bits);
> +
> +		if (cl_bpc != expected_bpc) {
> +			rc = ocfs2_error(sb,
> +				"Inode %llu has corrupted cl_bpc: ondisk=%u expected=%u\n",
> +				(unsigned long long)bh->b_blocknr,
> +				cl_bpc, expected_bpc);
> +			goto bail;
> +		}
> +	}
>  
>  	rc = 0;
>  


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ