On Wed 10-07-24 12:06:36, libaokun@xxxxxxxxxxxxxxx wrote: > From: Baokun Li <libaokun1@xxxxxxxxxx> > > In ext4_ext_rm_idx() and ext4_ext_correct_indexes(), there is no proper > rollback of already executed updates when updating a level of the extents > path fails, so we may get an inconsistent extents tree, which may trigger > some bad things in errors=continue mode. > > Hence clear the verified bit of modified extents buffers if the tree fails > to be updated in ext4_ext_rm_idx() or ext4_ext_correct_indexes(), which > forces the extents buffers to be checked in ext4_valid_extent_entries(), > ensuring that the extents tree is consistent. > > Signed-off-by: zhanchengbin <zhanchengbin1@xxxxxxxxxx> > Link: https://lore.kernel.org/r/20230213080514.535568-3-zhanchengbin1@xxxxxxxxxx/ > Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@xxxxxxx> Honza > --- > fs/ext4/extents.c | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index bff3666c891a..4d589d34b30e 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -1749,12 +1749,23 @@ static int ext4_ext_correct_indexes(handle_t *handle, struct inode *inode, > break; > err = ext4_ext_get_access(handle, inode, path + k); > if (err) > - break; > + goto clean; > path[k].p_idx->ei_block = border; > err = ext4_ext_dirty(handle, inode, path + k); > if (err) > - break; > + goto clean; > } > + return 0; > + > +clean: > + /* > + * The path[k].p_bh is either unmodified or with no verified bit > + * set (see ext4_ext_get_access()). So just clear the verified bit > + * of the successfully modified extents buffers, which will force > + * these extents to be checked to avoid using inconsistent data. > + */ > + while (++k < depth) > + clear_buffer_verified(path[k].p_bh); > > return err; > } > @@ -2312,12 +2323,24 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode, > break; > err = ext4_ext_get_access(handle, inode, path + k); > if (err) > - break; > + goto clean; > path[k].p_idx->ei_block = path[k + 1].p_idx->ei_block; > err = ext4_ext_dirty(handle, inode, path + k); > if (err) > - break; > + goto clean; > } > + return 0; > + > +clean: > + /* > + * The path[k].p_bh is either unmodified or with no verified bit > + * set (see ext4_ext_get_access()). So just clear the verified bit > + * of the successfully modified extents buffers, which will force > + * these extents to be checked to avoid using inconsistent data. > + */ > + while (++k < depth) > + clear_buffer_verified(path[k].p_bh); > + > return err; > } > > -- > 2.39.2 > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
