On Tue 14-09-21 19:14:15, yangerkun wrote:
> Or the ls for ext4 dir can run into a deadloop since info->last_pos !=
> ctx->pos which will reset the world and start read the entry which has
> already got before. Details see below:
>
> 1. a dx_dir which has 3 block, block 0 as dx_root block, block 1/2 as
> leaf block which own the ext4_dir_entry_2
> 2. block 1 read ok and call_filldir which will fill the dirent and update
> the ctx->pos
> 3. block 2 read fail, but we has already fill some dirent, so we will
> return back to userspace will a positive return val(see ksys_getdents64)
> 4. the second ext4_dx_readdir will reset the world since info->last_pos
> != ctx->pos, and will also init the curr_hash which pos to block 1
> 5. So we will read block1 too, and once block2 still read fail, we can
> only fill one dirent because the hash of the entry in block1(besides
> the last one) won't greater than curr_hash
> 6. this time, we forget update last_pos too since the read for block2
> will fail, and since we has got the one entry, ksys_getdents64 can
> return success
> 7. Latter we will trapped in a loop with step 4~6
>
> Fix it by update last_pos too once ext4_htree_fill_tree return fail.
>
> Signed-off-by: yangerkun <yangerkun@xxxxxxxxxx>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/ext4/dir.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index ffb295aa891c..74b172a4adda 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -551,7 +551,7 @@ static int ext4_dx_readdir(struct file *file, struct dir_context *ctx)
> struct dir_private_info *info = file->private_data;
> struct inode *inode = file_inode(file);
> struct fname *fname;
> - int ret;
> + int ret = 0;
>
> if (!info) {
> info = ext4_htree_create_dir_info(file, ctx->pos);
> @@ -599,7 +599,7 @@ static int ext4_dx_readdir(struct file *file, struct dir_context *ctx)
> info->curr_minor_hash,
> &info->next_hash);
> if (ret < 0)
> - return ret;
> + goto finished;
> if (ret == 0) {
> ctx->pos = ext4_get_htree_eof(file);
> break;
> @@ -630,7 +630,7 @@ static int ext4_dx_readdir(struct file *file, struct dir_context *ctx)
> }
> finished:
> info->last_pos = ctx->pos;
> - return 0;
> + return ret < 0 ? ret : 0;
> }
>
> static int ext4_release_dir(struct inode *inode, struct file *filp)
> --
> 2.31.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
