Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
From:   Amir Goldstein <amir73il@gmail.com>
To:     Miklos Szeredi <miklos@szeredi.hu>
Cc:     Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna@kernel.org>,
        Benjamin Coddington <bcodding@redhat.com>,
        Jeff Layton <jlayton@kernel.org>,
        Chuck Lever <chuck.lever@oracle.com>,
        Christian Brauner <brauner@kernel.org>,
        Jan Kara <jack@suse.cz>, linux-fsdevel@vger.kernel.org,
        linux-unionfs@vger.kernel.org, linux-nfs@vger.kernel.org
Subject: [PATCH] fuse: derive f_fsid from s_dev and connection start time
Date:   Wed, 25 Oct 2023 14:42:28 +0300
Message-Id: <20231025114228.23167-1-amir73il@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Use s_dev number and connection start time to report f_fsid in statfs(2).

The s_dev number could be easily recycled, so we use lower 32bits of the
connection start time to try to avoid the recycling of f_fsid.
The anon bdev number is only 20 bits (major is 0), so we could use more
bits from connection start time, but avoiding f_fsid recycling is not
critical, so 32bit is enough.

If the server does not support NFS export, fuse client still advertizes
->s_export_op, but those are non compliant operations that often cannot
decode file handles, or worse, decode file hanldes to wrong objects.

In this case, leave f_fsid zero to signal fanotify and aware users to
avoid exporting this incompliant fuse filesystem to NFS.

This allows fuse client to be monitored by fanotify filesystem watch
for local client file access if server supports NFS export.

For example, with inotify-tools 4.23.8.0, the following command can be
used to watch local client access over entire nfs filesystem:

  fsnotifywatch --filesystem /mnt/fuse

Note that fanotify filesystem watch does not report remote changes on
server.  It provides the same notifications as inotify, but it watches
over the entire filesystem and reports file handle of objects and fsid
with events.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Miklos,

I'd like to explain why I chose to tie setting fuse f_fsid with fuse
server NFS export capability.

Since v6.6-rc7, fanotify permits sb/mount watch only for filesystems
that know how to decode ALL file handles (not only how to encode).
fanotify checks for the ->fh_to_dentry() method, which fuse always
implements regardless of server NFS export support.

At first I considered assigning s_export_op depending on server NFS
export support, but that would break the exising fuse best-effort decode
behavior, whatever it is worth.

Currently, fanotify sb watch does not support fuse because of zero f_fsid,
so I decided to keep it this way for the incomplete NFS export case.

Regardless of my own reasons, I think this behavior also makes sense for
users that want to export fuse to NFS:

In order to export fuse to NFS, user needs to specify an explicit "fsid"
argument in /etc/exports.  Until now, there was no clue from fuse w.r.t
a good value for the export fsid.

With this change, f_fsid would be a good candidate for export fsid.
Naturally, exported file handles would become stale after fuse server
restart, but that is much better that having file handles decode the
wrong object.

Even more important, if users would know that they can use f_fsid as
a valid export fsid, a zero f_fsid is also a good indication that
exporting this fuse fs is not a good idea.

Thanks,
Amir.

 fs/fuse/fuse_i.h |  3 +++
 fs/fuse/inode.c  | 26 +++++++++++++++++++++-----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index bf0b85d0b95c..963a7dbf4224 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -815,6 +815,9 @@ struct fuse_conn {
 	/** Device ID from the root super block */
 	dev_t dev;
 
+	/** Connection start time in jiffies */
+	u64 start_time;
+
 	/** Dentries in the control filesystem */
 	struct dentry *ctl_dentry[FUSE_CTL_NUM_DENTRIES];
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index e63f966698a5..480bd2e7ad37 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -533,7 +533,6 @@ static void fuse_send_destroy(struct fuse_mount *fm)
 
 static void convert_fuse_statfs(struct kstatfs *stbuf, struct fuse_kstatfs *attr)
 {
-	stbuf->f_type    = FUSE_SUPER_MAGIC;
 	stbuf->f_bsize   = attr->bsize;
 	stbuf->f_frsize  = attr->frsize;
 	stbuf->f_blocks  = attr->blocks;
@@ -542,7 +541,22 @@ static void convert_fuse_statfs(struct kstatfs *stbuf, struct fuse_kstatfs *attr
 	stbuf->f_files   = attr->files;
 	stbuf->f_ffree   = attr->ffree;
 	stbuf->f_namelen = attr->namelen;
-	/* fsid is left zero */
+}
+
+static void fuse_get_fsid(struct super_block *sb, __kernel_fsid_t *fsid)
+{
+	struct fuse_mount *fm = get_fuse_mount_super(sb);
+
+	/* fsid is left zero when server does not support NFS export */
+	if (!fm->fc->export_support)
+		return;
+
+	/*
+	 * Using the anon bdev number to avoid local f_fsid collisions with
+	 * lowers bit of connection start time to avoid recycling of f_fsid.
+	 */
+	fsid->val[0] = new_encode_dev(sb->s_dev);
+	fsid->val[1] = (u32) fm->fc->start_time;
 }
 
 static int fuse_statfs(struct dentry *dentry, struct kstatfs *buf)
@@ -553,10 +567,11 @@ static int fuse_statfs(struct dentry *dentry, struct kstatfs *buf)
 	struct fuse_statfs_out outarg;
 	int err;
 
-	if (!fuse_allow_current_process(fm->fc)) {
-		buf->f_type = FUSE_SUPER_MAGIC;
+	buf->f_type = FUSE_SUPER_MAGIC;
+	fuse_get_fsid(sb, &buf->f_fsid);
+
+	if (!fuse_allow_current_process(fm->fc))
 		return 0;
-	}
 
 	memset(&outarg, 0, sizeof(outarg));
 	args.in_numargs = 0;
@@ -874,6 +889,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
 	fc->user_ns = get_user_ns(user_ns);
 	fc->max_pages = FUSE_DEFAULT_MAX_PAGES_PER_REQ;
 	fc->max_pages_limit = FUSE_MAX_MAX_PAGES;
+	fc->start_time = get_jiffies_64();
 
 	INIT_LIST_HEAD(&fc->mounts);
 	list_add(&fm->fc_entry, &fc->mounts);
-- 
2.34.1