Re: [PATCH 5.4.y] ext4: fix bug_on in __es_tree_search

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, May 13, 2024 at 04:51:02PM -0300, Guilherme G. Piccoli wrote:
> CCing the right stable ML address...
> Apologies!
> 
> 
> On 11/05/2024 18:10, Guilherme G. Piccoli wrote:
> > From: Baokun Li <libaokun1@huawei.com>
> > 
> > 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@kernel.org
> > Fixes: 5946d089379a ("ext4: check for overlapping extents in ext4_valid_extent_entries()")
> > Reported-by: Hulk Robot <hulkci@huawei.com>
> > Signed-off-by: Baokun Li <libaokun1@huawei.com>
> > Reviewed-by: Jan Kara <jack@suse.cz>
> > Link: https://lore.kernel.org/r/20220518120816.1541863-1-libaokun1@huawei.com
> > Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> > Reported-by: syzbot+2a58d88f0fb315c85363@syzkaller.appspotmail.com
> > [gpiccoli: Manual backport due to unrelated missing patches.]
> > Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
> > ---
> > 
> > 
> > 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@kroah.com/
> > 
> > 
> >  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;
> 

Having a forwarded patch doesn't really help, can we get the real
backport please?

thanks,

greg k-h