CCing the right stable ML address...
Apologies!
On 11/05/2024 18:10, Guilherme G. Piccoli wrote:
> From: Baokun Li <libaokun1@xxxxxxxxxx>
>
> commit d36f6ed761b53933b0b4126486c10d3da7751e7f upstream.
>
> Hulk Robot reported a BUG_ON:
> ==================================================================
> kernel BUG at fs/ext4/extents_status.c:199!
> [...]
> RIP: 0010:ext4_es_end fs/ext4/extents_status.c:199 [inline]
> RIP: 0010:__es_tree_search+0x1e0/0x260 fs/ext4/extents_status.c:217
> [...]
> Call Trace:
> ext4_es_cache_extent+0x109/0x340 fs/ext4/extents_status.c:766
> ext4_cache_extents+0x239/0x2e0 fs/ext4/extents.c:561
> ext4_find_extent+0x6b7/0xa20 fs/ext4/extents.c:964
> ext4_ext_map_blocks+0x16b/0x4b70 fs/ext4/extents.c:4384
> ext4_map_blocks+0xe26/0x19f0 fs/ext4/inode.c:567
> ext4_getblk+0x320/0x4c0 fs/ext4/inode.c:980
> ext4_bread+0x2d/0x170 fs/ext4/inode.c:1031
> ext4_quota_read+0x248/0x320 fs/ext4/super.c:6257
> v2_read_header+0x78/0x110 fs/quota/quota_v2.c:63
> v2_check_quota_file+0x76/0x230 fs/quota/quota_v2.c:82
> vfs_load_quota_inode+0x5d1/0x1530 fs/quota/dquot.c:2368
> dquot_enable+0x28a/0x330 fs/quota/dquot.c:2490
> ext4_quota_enable fs/ext4/super.c:6137 [inline]
> ext4_enable_quotas+0x5d7/0x960 fs/ext4/super.c:6163
> ext4_fill_super+0xa7c9/0xdc00 fs/ext4/super.c:4754
> mount_bdev+0x2e9/0x3b0 fs/super.c:1158
> mount_fs+0x4b/0x1e4 fs/super.c:1261
> [...]
> ==================================================================
>
> Above issue may happen as follows:
> -------------------------------------
> ext4_fill_super
> ext4_enable_quotas
> ext4_quota_enable
> ext4_iget
> __ext4_iget
> ext4_ext_check_inode
> ext4_ext_check
> __ext4_ext_check
> ext4_valid_extent_entries
> Check for overlapping extents does't take effect
> dquot_enable
> vfs_load_quota_inode
> v2_check_quota_file
> v2_read_header
> ext4_quota_read
> ext4_bread
> ext4_getblk
> ext4_map_blocks
> ext4_ext_map_blocks
> ext4_find_extent
> ext4_cache_extents
> ext4_es_cache_extent
> ext4_es_cache_extent
> __es_tree_search
> ext4_es_end
> BUG_ON(es->es_lblk + es->es_len < es->es_lblk)
>
> The error ext4 extents is as follows:
> 0af3 0300 0400 0000 00000000 extent_header
> 00000000 0100 0000 12000000 extent1
> 00000000 0100 0000 18000000 extent2
> 02000000 0400 0000 14000000 extent3
>
> In the ext4_valid_extent_entries function,
> if prev is 0, no error is returned even if lblock<=prev.
> This was intended to skip the check on the first extent, but
> in the error image above, prev=0+1-1=0 when checking the second extent,
> so even though lblock<=prev, the function does not return an error.
> As a result, bug_ON occurs in __es_tree_search and the system panics.
>
> To solve this problem, we only need to check that:
> 1. The lblock of the first extent is not less than 0.
> 2. The lblock of the next extent is not less than
> the next block of the previous extent.
> The same applies to extent_idx.
>
> Cc: stable@xxxxxxxxxx
> Fixes: 5946d089379a ("ext4: check for overlapping extents in ext4_valid_extent_entries()")
> Reported-by: Hulk Robot <hulkci@xxxxxxxxxx>
> Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>
> Reviewed-by: Jan Kara <jack@xxxxxxx>
> Link: https://lore.kernel.org/r/20220518120816.1541863-1-libaokun1@xxxxxxxxxx
> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> Reported-by: syzbot+2a58d88f0fb315c85363@xxxxxxxxxxxxxxxxxxxxxxxxx
> [gpiccoli: Manual backport due to unrelated missing patches.]
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@xxxxxxxxxx>
> ---
>
>
> Hey folks, this one should have been backported but due to merge
> issues [0], it ended-up not being on 5.4.y . So here is a working version!
> Cheers,
>
> Guilherme
>
> [0] https://lore.kernel.org/stable/165451751147179@xxxxxxxxx/
>
>
> fs/ext4/extents.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 98e1b1ddb4ec..90b12c7c0f20 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -409,7 +409,7 @@ static int ext4_valid_extent_entries(struct inode *inode,
> {
> unsigned short entries;
> ext4_lblk_t lblock = 0;
> - ext4_lblk_t prev = 0;
> + ext4_lblk_t cur = 0;
>
> if (eh->eh_entries == 0)
> return 1;
> @@ -435,12 +435,12 @@ static int ext4_valid_extent_entries(struct inode *inode,
>
> /* Check for overlapping extents */
> lblock = le32_to_cpu(ext->ee_block);
> - if ((lblock <= prev) && prev) {
> + if (lblock < cur) {
> pblock = ext4_ext_pblock(ext);
> es->s_last_error_block = cpu_to_le64(pblock);
> return 0;
> }
> - prev = lblock + ext4_ext_get_actual_len(ext) - 1;
> + cur = lblock + ext4_ext_get_actual_len(ext);
> ext++;
> entries--;
> }
> @@ -460,13 +460,13 @@ static int ext4_valid_extent_entries(struct inode *inode,
>
> /* Check for overlapping index extents */
> lblock = le32_to_cpu(ext_idx->ei_block);
> - if ((lblock <= prev) && prev) {
> + if (lblock < cur) {
> *pblk = ext4_idx_pblock(ext_idx);
> return 0;
> }
> ext_idx++;
> entries--;
> - prev = lblock;
> + cur = lblock + 1;
> }
> }
> return 1;
