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@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;
>
Having a forwarded patch doesn't really help, can we get the real
backport please?
thanks,
greg k-h
