Skip to contentSkip to conversation

afs: Fix endless loop in directory parsing

PATCH Applied David Howells 3 participants 3 messages Feb 23, 2024

Subsystems (2)
VFS Afs filesystem
lore
Share & tools
lore

David Howells

author maintainer
#0 lore raw

PATCH 1/1 afs: Fix endless loop in directory parsing

If a directory has a block with only ".__afsXXXX" files in it (from uncompleted silly-rename), these .__afsXXXX files are skipped but without advancing the file position in the dir_context. This leads to afs_dir_iterate() repeating the block again and again.

Fix this by making the code that skips the .__afsXXXX file also manually advance the file position.

The symptoms are a soft lookup:

        watchdog: BUG: soft lockup - CPU#3 stuck for 52s! [check:5737]
        ...
        RIP: 0010:afs_dir_iterate_block+0x39/0x1fd
        ...
         ? watchdog_timer_fn+0x1a6/0x213
        ...
         ? asm_sysvec_apic_timer_interrupt+0x16/0x20
         ? afs_dir_iterate_block+0x39/0x1fd
         afs_dir_iterate+0x10a/0x148
         afs_readdir+0x30/0x4a
         iterate_dir+0x93/0xd3
         __do_sys_getdents64+0x6b/0xd4

This is almost certainly the actual fix for:

        https://bugzilla.kernel.org/show_bug.cgi?id=218496
Fixes 57e9d49c5452 ("afs: Hide silly-rename files from userspace")
Signed-off-by David Howells <[email protected]>
cc Marc Dionne <[email protected]>
cc Markus Suvanto <[email protected]>
cc [email protected]
---
 fs/afs/dir.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
fs/afs/dir.c +3 -1
index b5b8de521f99..8a67fc427e74 100644
@@ -479,8 +479,10 @@ static int afs_dir_iterate_block(struct afs_vnode *dvnode,
 		    dire->u.name[0] == '.' &&
 		    ctx->actor != afs_lookup_filldir &&
 		    ctx->actor != afs_lookup_one_filldir &&
-		    memcmp(dire->u.name, ".__afs", 6) == 0)
+		    memcmp(dire->u.name, ".__afs", 6) == 0) {
+			ctx->pos = blkoff + next * sizeof(union afs_xdr_dirent);
 			continue;
+		}
 
 		/* found the next entry */
 		if (!dir_emit(ctx, dire->u.name, nlen,
maintainer Reviewed-by on v1 1/1
#1 lore raw
Quoted 44 lines · David Howells

If a directory has a block with only ".__afsXXXX" files in it (from uncompleted silly-rename), these .__afsXXXX files are skipped but without advancing the file position in the dir_context. This leads to afs_dir_iterate() repeating the block again and again.

Fix this by making the code that skips the .__afsXXXX file also manually advance the file position.

The symptoms are a soft lookup:

        watchdog: BUG: soft lockup - CPU#3 stuck for 52s! [check:5737]
        ...
        RIP: 0010:afs_dir_iterate_block+0x39/0x1fd
        ...
         ? watchdog_timer_fn+0x1a6/0x213
        ...
         ? asm_sysvec_apic_timer_interrupt+0x16/0x20
         ? afs_dir_iterate_block+0x39/0x1fd
         afs_dir_iterate+0x10a/0x148
         afs_readdir+0x30/0x4a
         iterate_dir+0x93/0xd3
         __do_sys_getdents64+0x6b/0xd4

This is almost certainly the actual fix for:

        https://bugzilla.kernel.org/show_bug.cgi?id=218496
Fixes 57e9d49c5452 ("afs: Hide silly-rename files from userspace")
Signed-off-by David Howells <[email protected]>
cc Marc Dionne <[email protected]>
cc Markus Suvanto <[email protected]>
cc [email protected]
---
 fs/afs/dir.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
fs/afs/dir.c +0 -0
index b5b8de521f99..8a67fc427e74 100644
@@ -479,8 +479,10 @@ static int afs_dir_iterate_block(struct afs_vnode *dvnode,
                    dire->u.name[0] == '.' &&

fs/afs/dir.c static int afs_dir_iterate_block(struct afs_vnode *dvnode, · line 488

Quoted 10 lines
fs/afs/dir.c +3 -1
                    ctx->actor != afs_lookup_filldir &&
                    ctx->actor != afs_lookup_one_filldir &&
-                   memcmp(dire->u.name, ".__afs", 6) == 0)
+                   memcmp(dire->u.name, ".__afs", 6) == 0) {
+                       ctx->pos = blkoff + next * sizeof(union afs_xdr_dirent);
                        continue;
+               }
 
                /* found the next entry */
                if (!dir_emit(ctx, dire->u.name, nlen,
Reviewed-by Marc Dionne <[email protected]>

Marc

maintainer Applied
#2 lore raw
In reply to David Howells
Quoted 10 lines · David Howells

If a directory has a block with only ".__afsXXXX" files in it (from uncompleted silly-rename), these .__afsXXXX files are skipped but without advancing the file position in the dir_context. This leads to afs_dir_iterate() repeating the block again and again.

Fix this by making the code that skips the .__afsXXXX file also manually advance the file position.

[...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree. Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] afs: Fix endless loop in directory parsing

      https://git.kernel.org/vfs/vfs/c/c7742709248d