Bug 259974

Summary:	[fusefs]: Looking up .. is TODO
Product:	Base System	Reporter:	Alan Somers <asomers>
Component:	kern	Assignee:	Alan Somers <asomers>
Status:	Closed FIXED
Severity:	Affects Only Me	CC:	emaste
Priority:	---	Flags:	asomers: mfc-stable13+ asomers: mfc-stable12+
Version:	CURRENT
Hardware:	Any
OS:	Any

Description Alan Somers freebsd_committer

2021-11-21 18:42:34 UTC

If the VFS calls VOP_LOOKUP for ".." where dvp is the root of a fuse file system, fusefs will panic with the message "Looking up .. is TODO" (the panic message is not completely accurate, because looking up ".." DOES work for any other directory).

This panic has always been present, though before SVN r348135 it was a page fault rather than an explicit panic.  I never knew any way to trigger this behavior until now.  The following sequence of operations will trigger the panic:

First install necessary packages

$ sudo pkg install fish fusefs-ext2

Start the fuse server in one terminal

$ rm -f /tmp/ext2.img && truncate -s 128m /tmp/ext2.img && mkfs.ext2 /tmp/ext2.img && sudo fuse-ext2 /tmp/ext2.img /tmp/mnt -o allow_other,rw+,debug

Access the fuse file system and trigger the bug from another terminal

$ sudo su
# fish
# cd /tmp/mnt
# mkdir -p a/b
# cd a/b
# /bin/sh
# cd /
# umount /tmp/mnt
umount: unmount of /tmp/mnt failed: Device busy
# exit

Comment 1 Alan Somers freebsd_committer

2021-12-02 03:45:39 UTC

So the root cause of this bug turns out to be pretty stupid.  fusefs special cases lookups for ".." by storing some extra information in each vnode.  But that fails for a directory whose parent has been reclaimed.

The stupid part is that there's no good reason to special-case ".." ... except that it's the lowest common denominator for FUSE file systems :facepalm:.  The FUSE documentation is pretty explicit that file systems aren't required to implement lookups for ".." unless they opt into it.

For file systems that do opt-in, the solution to this bug is easy: just ask the FUSE server to lookup "..".  In fact, we should be doing that anyway in cases where the server specifies a time-limited entry cache and the parent's cache entry has expired.

For file systems that don't opt-int, however, I'm not sure that there is a good solution.  We could return EIO or ESTALE, perhaps.

Comment 2 commit-hook freebsd_committer

2022-01-01 03:39:48 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=1613087a8127122b03a3730046d051adf4edd14f

commit 1613087a8127122b03a3730046d051adf4edd14f
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-12-02 02:50:47 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-01-01 03:38:27 +0000

    fusefs: fix .. lookups when the parent has been reclaimed.

    By default, FUSE file systems are assumed not to support lookups for "."
    and "..".  They must opt-in to that.  To cope with this limitation, the
    fusefs kernel module caches every fuse vnode's parent's inode number,
    and uses that during VOP_LOOKUP for "..".  But if the parent's vnode has
    been reclaimed that won't be possible.  Previously we paniced in this
    situation.  Now, we'll return ESTALE instead.  Or, if the file system
    has opted into ".." lookups, we'll just do that instead.

    This commit also fixes VOP_LOOKUP to respect the cache timeout for ".."
    lookups, if the FUSE file system specified a finite timeout.

    PR:             259974
    MFC after:      2 weeks
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D33239

 sys/fs/fuse/fuse_vnops.c      |  26 ++++--
 tests/sys/fs/fusefs/lookup.cc | 203 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 221 insertions(+), 8 deletions(-)

Comment 3 commit-hook freebsd_committer

2022-01-18 01:13:22 UTC

A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=bfffd351080752652c9f850d1b5540f1097bc7c8

commit bfffd351080752652c9f850d1b5540f1097bc7c8
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-12-02 02:38:04 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-01-18 01:08:25 +0000

    fusefs: in the tests, always assume debug.try_reclaim_vnode is available

    In an earlier version of the revision that created that sysctl (D20519)
    the sysctl was gated by INVARIANTS, so the test had to check for it.
    But in the committed version it is always available.

    (cherry picked from commit 19ab361045343bb777176bb08468f7706d7649c4)

    fusefs: move common code from forget.cc to utils.cc

    (cherry picked from commit 8d99a6b91b788b7ddf88f975f288f7c6479f4be3)

    fusefs: fix .. lookups when the parent has been reclaimed.

    By default, FUSE file systems are assumed not to support lookups for "."
    and "..".  They must opt-in to that.  To cope with this limitation, the
    fusefs kernel module caches every fuse vnode's parent's inode number,
    and uses that during VOP_LOOKUP for "..".  But if the parent's vnode has
    been reclaimed that won't be possible.  Previously we paniced in this
    situation.  Now, we'll return ESTALE instead.  Or, if the file system
    has opted into ".." lookups, we'll just do that instead.

    This commit also fixes VOP_LOOKUP to respect the cache timeout for ".."
    lookups, if the FUSE file system specified a finite timeout.

    PR:             259974
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D33239

    (cherry picked from commit 1613087a8127122b03a3730046d051adf4edd14f)

 sys/fs/fuse/fuse_vnops.c      |  26 ++++--
 tests/sys/fs/fusefs/forget.cc |  14 +--
 tests/sys/fs/fusefs/lookup.cc | 203 ++++++++++++++++++++++++++++++++++++++++++
 tests/sys/fs/fusefs/utils.cc  |   9 ++
 tests/sys/fs/fusefs/utils.hh  |   4 +
 5 files changed, 236 insertions(+), 20 deletions(-)

Comment 4 commit-hook freebsd_committer

2022-01-18 02:29:38 UTC

A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=542dbacf596e1693694cde865e880404b16b8be9

commit 542dbacf596e1693694cde865e880404b16b8be9
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-12-02 02:38:04 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-01-18 02:20:28 +0000

    fusefs: in the tests, always assume debug.try_reclaim_vnode is available

    In an earlier version of the revision that created that sysctl (D20519)
    the sysctl was gated by INVARIANTS, so the test had to check for it.
    But in the committed version it is always available.

    (cherry picked from commit 19ab361045343bb777176bb08468f7706d7649c4)

    fusefs: move common code from forget.cc to utils.cc

    (cherry picked from commit 8d99a6b91b788b7ddf88f975f288f7c6479f4be3)

    fusefs: fix .. lookups when the parent has been reclaimed.

    By default, FUSE file systems are assumed not to support lookups for "."
    and "..".  They must opt-in to that.  To cope with this limitation, the
    fusefs kernel module caches every fuse vnode's parent's inode number,
    and uses that during VOP_LOOKUP for "..".  But if the parent's vnode has
    been reclaimed that won't be possible.  Previously we paniced in this
    situation.  Now, we'll return ESTALE instead.  Or, if the file system
    has opted into ".." lookups, we'll just do that instead.

    This commit also fixes VOP_LOOKUP to respect the cache timeout for ".."
    lookups, if the FUSE file system specified a finite timeout.

    PR:             259974
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D33239

    (cherry picked from commit 1613087a8127122b03a3730046d051adf4edd14f)

 sys/fs/fuse/fuse_vnops.c      |  26 ++++--
 tests/sys/fs/fusefs/forget.cc |  14 +--
 tests/sys/fs/fusefs/lookup.cc | 203 ++++++++++++++++++++++++++++++++++++++++++
 tests/sys/fs/fusefs/utils.cc  |   9 ++
 tests/sys/fs/fusefs/utils.hh  |   4 +
 5 files changed, 236 insertions(+), 20 deletions(-)