Re: [PATCH] afs: Fix infinite loop found by xfstest generic/676

[Marc Dionne <marc.dionne@auristor.com>]

On Mon, May 30, 2022 at 11:09 AM David Howells <dhowells@redhat.com> wrote:
>
> In AFS, a directory is handled as a file that the client downloads and
> parses locally for the purposes of performing lookup and getdents
> operations.  The in-kernel afs filesystem has a number of functions that do
> this.  A directory file is arranged as a series of 2K blocks divided into
> 32-byte slots, where a directory entry occupies one or more slots, plus
> each block starts with one or more metadata blocks.
>
> When parsing a block, if the last slots are occupied by a dirent that
> occupies more than a single slot and the file position points at a slot
> that's not the initial one, the logic in afs_dir_iterate_block() that skips
> over it won't advance the file pointer to the end of it.  This will cause
> an infinite loop in getdents() as it will keep retrying that block and
> failing to advance beyond the final entry.
>
> Fix this by advancing the file pointer if the next entry will be beyond it
> when we skip a block.
>
> This was found by the generic/676 xfstest but can also be triggered with
> something like:
>
>         ~/xfstests-dev/src/t_readdir_3 /xfstest.test/z 4000 1
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Marc Dionne <marc.dionne@auristor.com>
> cc: linux-afs@lists.infradead.org
> ---
>
>  fs/afs/dir.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/afs/dir.c b/fs/afs/dir.c
> index 932e61e28e5d..bdac73554e6e 100644
> --- a/fs/afs/dir.c
> +++ b/fs/afs/dir.c
> @@ -463,8 +463,11 @@ static int afs_dir_iterate_block(struct afs_vnode *dvnode,
>                 }
>
>                 /* skip if starts before the current position */
> -               if (offset < curr)
> +               if (offset < curr) {
> +                       if (next > curr)
> +                               ctx->pos = blkoff + next * sizeof(union afs_xdr_dirent);
>                         continue;
> +               }
>
>                 /* found the next entry */
>                 if (!dir_emit(ctx, dire->u.name, nlen,

Looks good, and fixes the hang with generic/676.

Reviewed-and-tested-by: Marc Dionne <marc.dionne@auristor.com>

Marc