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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAB9dFdtak3PsPDZMbsiAEMbCx-UNvwOGFtROhZpcaN6=90HE0A@mail.gmail.com>
Date:   Mon, 30 May 2022 15:45:34 -0300
From:   Marc Dionne <marc.dionne@...istor.com>
To:     David Howells <dhowells@...hat.com>
Cc:     linux-afs@...ts.infradead.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] afs: Fix infinite loop found by xfstest generic/676

On Mon, May 30, 2022 at 11:09 AM David Howells <dhowells@...hat.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@...hat.com>
> cc: Marc Dionne <marc.dionne@...istor.com>
> cc: linux-afs@...ts.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@...istor.com>

Marc