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

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

[David Howells]

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,

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

[Marc Dionne]

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

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

[David Howells]

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>
Reviewed-by: Marc Dionne <marc.dionne@auristor.com>
Tested-by: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
Link: http://lore.kernel.org/r/165391973497.110268.2939296942213894166.stgit@warthog.procyon.org.uk/
---

 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,