[PATCH 0/5] Support more filesystems with FAN_REPORT_FID

[PATCH 0/5] Support more filesystems with FAN_REPORT_FID

[Amir Goldstein]

Jan,

Following up on the plan laid out in this discussion, this patch set
implements the simpler and less controversial part of the plan to
enable AT_HANDLE_FID for all filesystems.

One filesystem that I tested which gained FAN_REPORT_FID support is 9p,
but there are many other filesystem for whom fanotify will become mostly
inotify drop-in replacement after this change.

Since the main goal of this change is to progress fanotify towards being
an inotify drop-in replacement, support for FAN_REPORT_FID with sb/mount
mark is at lower priority.

Because I think that support for FAN_REPORT_FID with sb/mount mark is
controversial with non-decodeable (AT_HANDLE_FID) file handles, I have
also disabled this feature that was added in v6.6-rc1 to ovelrayfs.

If you agree to this retroactive change, the path #1 should be fast
tracked into v6.6.

The rest of the changes should probably go in via the vfs tree after
review from you and nfsd maintainers.

Thanks,
Amir.

[1] https://lore.kernel.org/r/20230920110429.f4wkfuls73pd55pv@quack3/

Amir Goldstein (5):
  fanotify: limit reporting of event with non-decodeable file handles
  exportfs: add helpers to check if filesystem can encode/decode file
    handles
  exportfs: make ->encode_fh() a mandatory method for NFS export
  exportfs: define FILEID_INO64_GEN* file handle types
  exportfs: support encoding non-decodeable file handles by default

 Documentation/filesystems/nfs/exporting.rst |  7 +--
 Documentation/filesystems/porting.rst       |  9 ++++
 fs/affs/namei.c                             |  1 +
 fs/befs/linuxvfs.c                          |  1 +
 fs/efs/super.c                              |  1 +
 fs/erofs/super.c                            |  1 +
 fs/exportfs/expfs.c                         | 50 +++++++++++++++------
 fs/ext2/super.c                             |  1 +
 fs/ext4/super.c                             |  1 +
 fs/f2fs/super.c                             |  1 +
 fs/fat/nfs.c                                |  1 +
 fs/fhandle.c                                |  6 +--
 fs/fuse/inode.c                             |  7 +--
 fs/jffs2/super.c                            |  1 +
 fs/jfs/super.c                              |  1 +
 fs/nfsd/export.c                            |  3 +-
 fs/notify/fanotify/fanotify_user.c          | 25 +++++++----
 fs/ntfs/namei.c                             |  1 +
 fs/ntfs3/super.c                            |  1 +
 fs/overlayfs/util.c                         |  2 +-
 fs/smb/client/export.c                      |  9 ++--
 fs/squashfs/export.c                        |  1 +
 fs/ufs/super.c                              |  1 +
 include/linux/exportfs.h                    | 46 ++++++++++++++++++-
 24 files changed, 133 insertions(+), 45 deletions(-)

-- 
2.34.1

[PATCH 1/5] fanotify: limit reporting of event with non-decodeable file handles

[Amir Goldstein]

Commit a95aef69a740 ("fanotify: support reporting non-decodeable file
handles") merged in v6.5-rc1, added the ability to use an fanotify group
with FAN_REPORT_FID mode to watch filesystems that do not support nfs
export, but do know how to encode non-decodeable file handles, with the
newly introduced AT_HANDLE_FID flag.

At the time that this commit was merged, there were no filesystems
in-tree with those traits.

Commit 16aac5ad1fa9 ("ovl: support encoding non-decodable file handles"),
merged in v6.6-rc1, added this trait to overlayfs, thus allowing fanotify
watching of overlayfs with FAN_REPORT_FID mode.

In retrospect, allowing an fanotify filesystem/mount mark on such
filesystem in FAN_REPORT_FID mode will result in getting events with
file handles, without the ability to resolve the filesystem objects from
those file handles (i.e. no open_by_handle_at() support).

For v6.6, the safer option would be to allow this mode for inode marks
only, where the caller has the opportunity to use name_to_handle_at() at
the time of setting the mark. In the future we can revise this decision.

Fixes: a95aef69a740 ("fanotify: support reporting non-decodeable file handles")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify_user.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index f69c451018e3..537c70beaad0 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1585,16 +1585,25 @@ static int fanotify_test_fsid(struct dentry *dentry, __kernel_fsid_t *fsid)
 }
 
 /* Check if filesystem can encode a unique fid */
-static int fanotify_test_fid(struct dentry *dentry)
+static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
 {
+	unsigned int mark_type = flags & FANOTIFY_MARK_TYPE_BITS;
+	const struct export_operations *nop = dentry->d_sb->s_export_op;
+
+	/*
+	 * We need to make sure that the filesystem supports encoding of
+	 * file handles so user can use name_to_handle_at() to compare fids
+	 * reported with events to the file handle of watched objects.
+	 */
+	if (!nop)
+		return -EOPNOTSUPP;
+
 	/*
-	 * We need to make sure that the file system supports at least
-	 * encoding a file handle so user can use name_to_handle_at() to
-	 * compare fid returned with event to the file handle of watched
-	 * objects. However, even the relaxed AT_HANDLE_FID flag requires
-	 * at least empty export_operations for ecoding unique file ids.
+	 * For sb/mount mark, we also need to make sure that the filesystem
+	 * supports decoding file handles, so user has a way to map back the
+	 * reported fids to filesystem objects.
 	 */
-	if (!dentry->d_sb->s_export_op)
+	if (mark_type != FAN_MARK_INODE && !nop->fh_to_dentry)
 		return -EOPNOTSUPP;
 
 	return 0;
@@ -1812,7 +1821,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 		if (ret)
 			goto path_put_and_out;
 
-		ret = fanotify_test_fid(path.dentry);
+		ret = fanotify_test_fid(path.dentry, flags);
 		if (ret)
 			goto path_put_and_out;
 
-- 
2.34.1

[PATCH 2/5] exportfs: add helpers to check if filesystem can encode/decode file handles

[Amir Goldstein]

The logic of whether filesystem can encode/decode file handles is open
coded in many places.

In preparation to changing the logic, move the open coded logic into
inline helpers.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/exportfs/expfs.c                |  8 ++------
 fs/fhandle.c                       |  6 +-----
 fs/nfsd/export.c                   |  3 +--
 fs/notify/fanotify/fanotify_user.c |  4 ++--
 fs/overlayfs/util.c                |  2 +-
 include/linux/exportfs.h           | 27 +++++++++++++++++++++++++++
 6 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index c20704aa21b3..9ee205df8fa7 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -396,11 +396,7 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
 {
 	const struct export_operations *nop = inode->i_sb->s_export_op;
 
-	/*
-	 * If a decodeable file handle was requested, we need to make sure that
-	 * filesystem can decode file handles.
-	 */
-	if (nop && !(flags & EXPORT_FH_FID) && !nop->fh_to_dentry)
+	if (!exportfs_can_encode_fh(nop, flags))
 		return -EOPNOTSUPP;
 
 	if (nop && nop->encode_fh)
@@ -456,7 +452,7 @@ exportfs_decode_fh_raw(struct vfsmount *mnt, struct fid *fid, int fh_len,
 	/*
 	 * Try to get any dentry for the given file handle from the filesystem.
 	 */
-	if (!nop || !nop->fh_to_dentry)
+	if (!exportfs_can_decode_fh(nop))
 		return ERR_PTR(-ESTALE);
 	result = nop->fh_to_dentry(mnt->mnt_sb, fid, fh_len, fileid_type);
 	if (IS_ERR_OR_NULL(result))
diff --git a/fs/fhandle.c b/fs/fhandle.c
index 6ea8d35a9382..18b3ba8dc8ea 100644
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -26,12 +26,8 @@ static long do_sys_name_to_handle(const struct path *path,
 	/*
 	 * We need to make sure whether the file system support decoding of
 	 * the file handle if decodeable file handle was requested.
-	 * Otherwise, even empty export_operations are sufficient to opt-in
-	 * to encoding FIDs.
 	 */
-	if (!path->dentry->d_sb->s_export_op ||
-	    (!(fh_flags & EXPORT_FH_FID) &&
-	     !path->dentry->d_sb->s_export_op->fh_to_dentry))
+	if (!exportfs_can_encode_fh(path->dentry->d_sb->s_export_op, fh_flags))
 		return -EOPNOTSUPP;
 
 	if (copy_from_user(&f_handle, ufh, sizeof(struct file_handle)))
diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 11a0eaa2f914..dc99dfc1d411 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -421,8 +421,7 @@ static int check_export(struct path *path, int *flags, unsigned char *uuid)
 		return -EINVAL;
 	}
 
-	if (!inode->i_sb->s_export_op ||
-	    !inode->i_sb->s_export_op->fh_to_dentry) {
+	if (!exportfs_can_decode_fh(inode->i_sb->s_export_op)) {
 		dprintk("exp_export: export of invalid fs type.\n");
 		return -EINVAL;
 	}
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 537c70beaad0..ce926eb9feea 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1595,7 +1595,7 @@ static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
 	 * file handles so user can use name_to_handle_at() to compare fids
 	 * reported with events to the file handle of watched objects.
 	 */
-	if (!nop)
+	if (!exportfs_can_encode_fid(nop))
 		return -EOPNOTSUPP;
 
 	/*
@@ -1603,7 +1603,7 @@ static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
 	 * supports decoding file handles, so user has a way to map back the
 	 * reported fids to filesystem objects.
 	 */
-	if (mark_type != FAN_MARK_INODE && !nop->fh_to_dentry)
+	if (mark_type != FAN_MARK_INODE && !exportfs_can_decode_fh(nop))
 		return -EOPNOTSUPP;
 
 	return 0;
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 89e0d60d35b6..f0a712214ec2 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -55,7 +55,7 @@ int ovl_can_decode_fh(struct super_block *sb)
 	if (!capable(CAP_DAC_READ_SEARCH))
 		return 0;
 
-	if (!sb->s_export_op || !sb->s_export_op->fh_to_dentry)
+	if (!exportfs_can_decode_fh(sb->s_export_op))
 		return 0;
 
 	return sb->s_export_op->encode_fh ? -1 : FILEID_INO32_GEN;
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 11fbd0ee1370..5b3c9f30b422 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -233,6 +233,33 @@ extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
 extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
 			      int *max_len, int flags);
 
+static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
+{
+	return nop;
+}
+
+static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
+{
+	return nop && nop->fh_to_dentry;
+}
+
+static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
+					  int fh_flags)
+{
+	/*
+	 * If a non-decodeable file handle was requested, we only need to make
+	 * sure that filesystem can encode file handles.
+	 */
+	if (fh_flags & EXPORT_FH_FID)
+		return exportfs_can_encode_fid(nop);
+
+	/*
+	 * If a decodeable file handle was requested, we need to make sure that
+	 * filesystem can also decode file handles.
+	 */
+	return exportfs_can_decode_fh(nop);
+}
+
 static inline int exportfs_encode_fid(struct inode *inode, struct fid *fid,
 				      int *max_len)
 {
-- 
2.34.1

[PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

[Amir Goldstein]

export_operations ->encode_fh() no longer has a default implementation to
encode FILEID_INO32_GEN* file handles.

Rename the default helper for encoding FILEID_INO32_GEN* file handles to
generic_encode_ino32_fh() and convert the filesystems that used the
default implementation to use the generic helper explicitly.

This is a step towards allowing filesystems to encode non-decodeable file
handles for fanotify without having to implement any export_operations.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 Documentation/filesystems/nfs/exporting.rst |  7 ++-----
 Documentation/filesystems/porting.rst       |  9 +++++++++
 fs/affs/namei.c                             |  1 +
 fs/befs/linuxvfs.c                          |  1 +
 fs/efs/super.c                              |  1 +
 fs/erofs/super.c                            |  1 +
 fs/exportfs/expfs.c                         | 14 ++++++++------
 fs/ext2/super.c                             |  1 +
 fs/ext4/super.c                             |  1 +
 fs/f2fs/super.c                             |  1 +
 fs/fat/nfs.c                                |  1 +
 fs/jffs2/super.c                            |  1 +
 fs/jfs/super.c                              |  1 +
 fs/ntfs/namei.c                             |  1 +
 fs/ntfs3/super.c                            |  1 +
 fs/smb/client/export.c                      |  9 +++------
 fs/squashfs/export.c                        |  1 +
 fs/ufs/super.c                              |  1 +
 include/linux/exportfs.h                    |  4 +++-
 19 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/Documentation/filesystems/nfs/exporting.rst b/Documentation/filesystems/nfs/exporting.rst
index 4b30daee399a..de64d2d002a2 100644
--- a/Documentation/filesystems/nfs/exporting.rst
+++ b/Documentation/filesystems/nfs/exporting.rst
@@ -122,12 +122,9 @@ are exportable by setting the s_export_op field in the struct
 super_block.  This field must point to a "struct export_operations"
 struct which has the following members:
 
-  encode_fh (optional)
+  encode_fh (mandatory)
     Takes a dentry and creates a filehandle fragment which may later be used
-    to find or create a dentry for the same object.  The default
-    implementation creates a filehandle fragment that encodes a 32bit inode
-    and generation number for the inode encoded, and if necessary the
-    same information for the parent.
+    to find or create a dentry for the same object.
 
   fh_to_dentry (mandatory)
     Given a filehandle fragment, this should find the implied object and
diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
index 4d05b9862451..197ef78a5014 100644
--- a/Documentation/filesystems/porting.rst
+++ b/Documentation/filesystems/porting.rst
@@ -1045,3 +1045,12 @@ filesystem type is now moved to a later point when the devices are closed:
 As this is a VFS level change it has no practical consequences for filesystems
 other than that all of them must use one of the provided kill_litter_super(),
 kill_anon_super(), or kill_block_super() helpers.
+
+---
+
+**mandatory**
+
+export_operations ->encode_fh() no longer has a default implementation to
+encode FILEID_INO32_GEN* file handles.
+Fillesystems that used the default implementation may use the generic helper
+generic_encode_ino32_fh() explicitly.
diff --git a/fs/affs/namei.c b/fs/affs/namei.c
index 2fe4a5832fcf..d6b9758ee23d 100644
--- a/fs/affs/namei.c
+++ b/fs/affs/namei.c
@@ -568,6 +568,7 @@ static struct dentry *affs_fh_to_parent(struct super_block *sb, struct fid *fid,
 }
 
 const struct export_operations affs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = affs_fh_to_dentry,
 	.fh_to_parent = affs_fh_to_parent,
 	.get_parent = affs_get_parent,
diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index 9a16a51fbb88..410dcaffd5ab 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -96,6 +96,7 @@ static const struct address_space_operations befs_symlink_aops = {
 };
 
 static const struct export_operations befs_export_operations = {
+	.encode_fh	= generic_encode_ino32_fh,
 	.fh_to_dentry	= befs_fh_to_dentry,
 	.fh_to_parent	= befs_fh_to_parent,
 	.get_parent	= befs_get_parent,
diff --git a/fs/efs/super.c b/fs/efs/super.c
index b287f47c165b..f17fdac76b2e 100644
--- a/fs/efs/super.c
+++ b/fs/efs/super.c
@@ -123,6 +123,7 @@ static const struct super_operations efs_superblock_operations = {
 };
 
 static const struct export_operations efs_export_ops = {
+	.encode_fh	= generic_encode_ino32_fh,
 	.fh_to_dentry	= efs_fh_to_dentry,
 	.fh_to_parent	= efs_fh_to_parent,
 	.get_parent	= efs_get_parent,
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 3700af9ee173..edbe07a24156 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -626,6 +626,7 @@ static struct dentry *erofs_get_parent(struct dentry *child)
 }
 
 static const struct export_operations erofs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = erofs_fh_to_dentry,
 	.fh_to_parent = erofs_fh_to_parent,
 	.get_parent = erofs_get_parent,
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 9ee205df8fa7..30da4539e257 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -343,20 +343,21 @@ static int get_name(const struct path *path, char *name, struct dentry *child)
 }
 
 /**
- * export_encode_fh - default export_operations->encode_fh function
+ * generic_encode_ino32_fh - generic export_operations->encode_fh function
  * @inode:   the object to encode
- * @fid:     where to store the file handle fragment
+ * @fh:      where to store the file handle fragment
  * @max_len: maximum length to store there
  * @parent:  parent directory inode, if wanted
  *
- * This default encode_fh function assumes that the 32 inode number
+ * This generic encode_fh function assumes that the 32 inode number
  * is suitable for locating an inode, and that the generation number
  * can be used to check that it is still valid.  It places them in the
  * filehandle fragment where export_decode_fh expects to find them.
  */
-static int export_encode_fh(struct inode *inode, struct fid *fid,
-		int *max_len, struct inode *parent)
+int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
+			    struct inode *parent)
 {
+	struct fid *fid = (void *)fh;
 	int len = *max_len;
 	int type = FILEID_INO32_GEN;
 
@@ -380,6 +381,7 @@ static int export_encode_fh(struct inode *inode, struct fid *fid,
 	*max_len = len;
 	return type;
 }
+EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
 
 /**
  * exportfs_encode_inode_fh - encode a file handle from inode
@@ -402,7 +404,7 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
 	if (nop && nop->encode_fh)
 		return nop->encode_fh(inode, fid->raw, max_len, parent);
 
-	return export_encode_fh(inode, fid, max_len, parent);
+	return -EOPNOTSUPP;
 }
 EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
 
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index aaf3e3e88cb2..b9f158a34997 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -397,6 +397,7 @@ static struct dentry *ext2_fh_to_parent(struct super_block *sb, struct fid *fid,
 }
 
 static const struct export_operations ext2_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = ext2_fh_to_dentry,
 	.fh_to_parent = ext2_fh_to_parent,
 	.get_parent = ext2_get_parent,
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index dbebd8b3127e..c44db1915437 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1646,6 +1646,7 @@ static const struct super_operations ext4_sops = {
 };
 
 static const struct export_operations ext4_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = ext4_fh_to_dentry,
 	.fh_to_parent = ext4_fh_to_parent,
 	.get_parent = ext4_get_parent,
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a8c8232852bb..60cfa11f65bf 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3282,6 +3282,7 @@ static struct dentry *f2fs_fh_to_parent(struct super_block *sb, struct fid *fid,
 }
 
 static const struct export_operations f2fs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = f2fs_fh_to_dentry,
 	.fh_to_parent = f2fs_fh_to_parent,
 	.get_parent = f2fs_get_parent,
diff --git a/fs/fat/nfs.c b/fs/fat/nfs.c
index 3626eb585a98..c52e63e10d35 100644
--- a/fs/fat/nfs.c
+++ b/fs/fat/nfs.c
@@ -279,6 +279,7 @@ static struct dentry *fat_get_parent(struct dentry *child_dir)
 }
 
 const struct export_operations fat_export_ops = {
+	.encode_fh	= generic_encode_ino32_fh,
 	.fh_to_dentry   = fat_fh_to_dentry,
 	.fh_to_parent   = fat_fh_to_parent,
 	.get_parent     = fat_get_parent,
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 7ea37f49f1e1..f99591a634b4 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -150,6 +150,7 @@ static struct dentry *jffs2_get_parent(struct dentry *child)
 }
 
 static const struct export_operations jffs2_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.get_parent = jffs2_get_parent,
 	.fh_to_dentry = jffs2_fh_to_dentry,
 	.fh_to_parent = jffs2_fh_to_parent,
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index 2e2f7f6d36a0..2cc2632f3c47 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -896,6 +896,7 @@ static const struct super_operations jfs_super_operations = {
 };
 
 static const struct export_operations jfs_export_operations = {
+	.encode_fh	= generic_encode_ino32_fh,
 	.fh_to_dentry	= jfs_fh_to_dentry,
 	.fh_to_parent	= jfs_fh_to_parent,
 	.get_parent	= jfs_get_parent,
diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
index ab44f2db533b..d7498ddc4a72 100644
--- a/fs/ntfs/namei.c
+++ b/fs/ntfs/namei.c
@@ -384,6 +384,7 @@ static struct dentry *ntfs_fh_to_parent(struct super_block *sb, struct fid *fid,
  * and due to using iget() whereas NTFS needs ntfs_iget().
  */
 const struct export_operations ntfs_export_ops = {
+	.encode_fh	= generic_encode_ino32_fh,
 	.get_parent	= ntfs_get_parent,	/* Find the parent of a given
 						   directory. */
 	.fh_to_dentry	= ntfs_fh_to_dentry,
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 5661a363005e..661ffb5aa1e0 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -789,6 +789,7 @@ static int ntfs_nfs_commit_metadata(struct inode *inode)
 }
 
 static const struct export_operations ntfs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = ntfs_fh_to_dentry,
 	.fh_to_parent = ntfs_fh_to_parent,
 	.get_parent = ntfs3_get_parent,
diff --git a/fs/smb/client/export.c b/fs/smb/client/export.c
index 37c28415df1e..834e9c9197b4 100644
--- a/fs/smb/client/export.c
+++ b/fs/smb/client/export.c
@@ -41,13 +41,10 @@ static struct dentry *cifs_get_parent(struct dentry *dentry)
 }
 
 const struct export_operations cifs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.get_parent = cifs_get_parent,
-/*	Following five export operations are unneeded so far and can default:
-	.get_dentry =
-	.get_name =
-	.find_exported_dentry =
-	.decode_fh =
-	.encode_fs =  */
+/*	Following export operations are mandatory for NFS export support:
+	.fh_to_dentry = */
 };
 
 #endif /* CONFIG_CIFS_NFSD_EXPORT */
diff --git a/fs/squashfs/export.c b/fs/squashfs/export.c
index 723763746238..62972f0ff868 100644
--- a/fs/squashfs/export.c
+++ b/fs/squashfs/export.c
@@ -173,6 +173,7 @@ __le64 *squashfs_read_inode_lookup_table(struct super_block *sb,
 
 
 const struct export_operations squashfs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry = squashfs_fh_to_dentry,
 	.fh_to_parent = squashfs_fh_to_parent,
 	.get_parent = squashfs_get_parent
diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 23377c1baed9..a480810cd4e3 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -137,6 +137,7 @@ static struct dentry *ufs_get_parent(struct dentry *child)
 }
 
 static const struct export_operations ufs_export_ops = {
+	.encode_fh = generic_encode_ino32_fh,
 	.fh_to_dentry	= ufs_fh_to_dentry,
 	.fh_to_parent	= ufs_fh_to_parent,
 	.get_parent	= ufs_get_parent,
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 5b3c9f30b422..6b6e01321405 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -235,7 +235,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
 
 static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
 {
-	return nop;
+	return nop && nop->encode_fh;
 }
 
 static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
@@ -279,6 +279,8 @@ extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
 /*
  * Generic helpers for filesystems.
  */
+int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
+			    struct inode *parent);
 extern struct dentry *generic_fh_to_dentry(struct super_block *sb,
 	struct fid *fid, int fh_len, int fh_type,
 	struct inode *(*get_inode) (struct super_block *sb, u64 ino, u32 gen));
-- 
2.34.1

[PATCH 4/5] exportfs: define FILEID_INO64_GEN* file handle types

[Amir Goldstein]

Similar to the common FILEID_INO32* file handle types, define common
FILEID_INO64* file handle types.

The type values of FILEID_INO64_GEN and FILEID_INO64_GEN_PARENT are the
values returned by fuse and xfs for 64bit ino encoded file handle types.

Note that these type value are filesystem specific and they do not define
a universal file handle format, for example:
fuse encodes FILEID_INO64_GEN as [ino-hi32,ino-lo32,gen] and xfs encodes
FILEID_INO64_GEN as [hostr-order-ino64,gen] (a.k.a xfs_fid64).

The FILEID_INO64_GEN fhandle type is going to be used for file ids for
fanotify from filesystems that do not support NFS export.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/fuse/inode.c          |  7 ++++---
 include/linux/exportfs.h | 11 +++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 2e4eb7cf26fb..e63f966698a5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1002,7 +1002,7 @@ static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
 	}
 
 	*max_len = len;
-	return parent ? 0x82 : 0x81;
+	return parent ? FILEID_INO64_GEN_PARENT : FILEID_INO64_GEN;
 }
 
 static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
@@ -1010,7 +1010,8 @@ static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
 {
 	struct fuse_inode_handle handle;
 
-	if ((fh_type != 0x81 && fh_type != 0x82) || fh_len < 3)
+	if ((fh_type != FILEID_INO64_GEN &&
+	     fh_type != FILEID_INO64_GEN_PARENT) || fh_len < 3)
 		return NULL;
 
 	handle.nodeid = (u64) fid->raw[0] << 32;
@@ -1024,7 +1025,7 @@ static struct dentry *fuse_fh_to_parent(struct super_block *sb,
 {
 	struct fuse_inode_handle parent;
 
-	if (fh_type != 0x82 || fh_len < 6)
+	if (fh_type != FILEID_INO64_GEN_PARENT || fh_len < 6)
 		return NULL;
 
 	parent.nodeid = (u64) fid->raw[3] << 32;
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 6b6e01321405..21eeb9f6bdbd 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -98,6 +98,17 @@ enum fid_type {
 	 */
 	FILEID_FAT_WITH_PARENT = 0x72,
 
+	/*
+	 * 64 bit inode number, 32 bit generation number.
+	 */
+	FILEID_INO64_GEN = 0x81,
+
+	/*
+	 * 64 bit inode number, 32 bit generation number,
+	 * 64 bit parent inode number, 32 bit parent generation.
+	 */
+	FILEID_INO64_GEN_PARENT = 0x82,
+
 	/*
 	 * 128 bit child FID (struct lu_fid)
 	 * 128 bit parent FID (struct lu_fid)
-- 
2.34.1

[PATCH 5/5] exportfs: support encoding non-decodeable file handles by default

[Amir Goldstein]

AT_HANDLE_FID was added as an API for name_to_handle_at() that request
the encoding of a file id, which is not intended to be decoded.

This file id is used by fanotify to describe objects in events.

So far, overlayfs is the only filesystem that supports encoding
non-decodeable file ids, by providing export_operations with an
->encode_fh() method and without a ->decode_fh() method.

Add support for encoding non-decodeable file ids to all the filesystems
that do not provide export_operations, by encoding a file id of type
FILEID_INO64_GEN from { i_ino, i_generation }.

A filesystem may that does not support NFS export, can opt-out of
encoding non-decodeable file ids for fanotify by defining an empty
export_operations struct (i.e. with a NULL ->encode_fh() method).

This allows the use of fanotify events with file ids on filesystems
like 9p which do not support NFS export to bring fanotify in feature
parity with inotify on those filesystems.

Note that fanotify also requires that the filesystems report a non-null
fsid.  Currently, many simple filesystems that have support for inotify
(e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
used with fanotify in file id reporting mode.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
 include/linux/exportfs.h | 10 +++++++---
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 30da4539e257..34e7d835d4ef 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
 }
 EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
 
+/**
+ * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
+ * @inode:   the object to encode
+ * @fid:     where to store the file handle fragment
+ * @max_len: maximum length to store there
+ *
+ * This generic function is used to encode a non-decodeable file id for
+ * fanotify for filesystems that do not support NFS export.
+ */
+static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
+				     int *max_len)
+{
+	if (*max_len < 3) {
+		*max_len = 3;
+		return FILEID_INVALID;
+	}
+
+	fid->i64.ino = inode->i_ino;
+	fid->i64.gen = inode->i_generation;
+	*max_len = 3;
+
+	return FILEID_INO64_GEN;
+}
+
 /**
  * exportfs_encode_inode_fh - encode a file handle from inode
  * @inode:   the object to encode
@@ -401,10 +425,10 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
 	if (!exportfs_can_encode_fh(nop, flags))
 		return -EOPNOTSUPP;
 
-	if (nop && nop->encode_fh)
-		return nop->encode_fh(inode, fid->raw, max_len, parent);
+	if (!nop && (flags & EXPORT_FH_FID))
+		return exportfs_encode_ino64_fid(inode, fid, max_len);
 
-	return -EOPNOTSUPP;
+	return nop->encode_fh(inode, fid->raw, max_len, parent);
 }
 EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
 
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 21eeb9f6bdbd..6688e457da64 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -134,7 +134,11 @@ struct fid {
 			u32 parent_ino;
 			u32 parent_gen;
 		} i32;
- 		struct {
+		struct {
+			u64 ino;
+			u32 gen;
+		} __packed i64;
+		struct {
  			u32 block;
  			u16 partref;
  			u16 parent_partref;
@@ -246,7 +250,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
 
 static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
 {
-	return nop && nop->encode_fh;
+	return !nop || nop->encode_fh;
 }
 
 static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
@@ -259,7 +263,7 @@ static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
 {
 	/*
 	 * If a non-decodeable file handle was requested, we only need to make
-	 * sure that filesystem can encode file handles.
+	 * sure that filesystem did not opt-out of encoding fid.
 	 */
 	if (fh_flags & EXPORT_FH_FID)
 		return exportfs_can_encode_fid(nop);
-- 
2.34.1

Re: [PATCH 2/5] exportfs: add helpers to check if filesystem can encode/decode file handles

[Jeff Layton]

On Wed, 2023-10-18 at 12:59 +0300, Amir Goldstein wrote:
> The logic of whether filesystem can encode/decode file handles is open
> coded in many places.
> 
> In preparation to changing the logic, move the open coded logic into
> inline helpers.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/exportfs/expfs.c                |  8 ++------
>  fs/fhandle.c                       |  6 +-----
>  fs/nfsd/export.c                   |  3 +--
>  fs/notify/fanotify/fanotify_user.c |  4 ++--
>  fs/overlayfs/util.c                |  2 +-
>  include/linux/exportfs.h           | 27 +++++++++++++++++++++++++++
>  6 files changed, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index c20704aa21b3..9ee205df8fa7 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -396,11 +396,7 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  {
>  	const struct export_operations *nop = inode->i_sb->s_export_op;
>  
> -	/*
> -	 * If a decodeable file handle was requested, we need to make sure that
> -	 * filesystem can decode file handles.
> -	 */
> -	if (nop && !(flags & EXPORT_FH_FID) && !nop->fh_to_dentry)
> +	if (!exportfs_can_encode_fh(nop, flags))
>  		return -EOPNOTSUPP;
>  
>  	if (nop && nop->encode_fh)
> @@ -456,7 +452,7 @@ exportfs_decode_fh_raw(struct vfsmount *mnt, struct fid *fid, int fh_len,
>  	/*
>  	 * Try to get any dentry for the given file handle from the filesystem.
>  	 */
> -	if (!nop || !nop->fh_to_dentry)
> +	if (!exportfs_can_decode_fh(nop))
>  		return ERR_PTR(-ESTALE);
>  	result = nop->fh_to_dentry(mnt->mnt_sb, fid, fh_len, fileid_type);
>  	if (IS_ERR_OR_NULL(result))
> diff --git a/fs/fhandle.c b/fs/fhandle.c
> index 6ea8d35a9382..18b3ba8dc8ea 100644
> --- a/fs/fhandle.c
> +++ b/fs/fhandle.c
> @@ -26,12 +26,8 @@ static long do_sys_name_to_handle(const struct path *path,
>  	/*
>  	 * We need to make sure whether the file system support decoding of
>  	 * the file handle if decodeable file handle was requested.
> -	 * Otherwise, even empty export_operations are sufficient to opt-in
> -	 * to encoding FIDs.
>  	 */
> -	if (!path->dentry->d_sb->s_export_op ||
> -	    (!(fh_flags & EXPORT_FH_FID) &&
> -	     !path->dentry->d_sb->s_export_op->fh_to_dentry))
> +	if (!exportfs_can_encode_fh(path->dentry->d_sb->s_export_op, fh_flags))
>  		return -EOPNOTSUPP;
>  
>  	if (copy_from_user(&f_handle, ufh, sizeof(struct file_handle)))
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 11a0eaa2f914..dc99dfc1d411 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -421,8 +421,7 @@ static int check_export(struct path *path, int *flags, unsigned char *uuid)
>  		return -EINVAL;
>  	}
>  
> -	if (!inode->i_sb->s_export_op ||
> -	    !inode->i_sb->s_export_op->fh_to_dentry) {
> +	if (!exportfs_can_decode_fh(inode->i_sb->s_export_op)) {
>  		dprintk("exp_export: export of invalid fs type.\n");
>  		return -EINVAL;
>  	}
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 537c70beaad0..ce926eb9feea 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1595,7 +1595,7 @@ static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
>  	 * file handles so user can use name_to_handle_at() to compare fids
>  	 * reported with events to the file handle of watched objects.
>  	 */
> -	if (!nop)
> +	if (!exportfs_can_encode_fid(nop))
>  		return -EOPNOTSUPP;
>  
>  	/*
> @@ -1603,7 +1603,7 @@ static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
>  	 * supports decoding file handles, so user has a way to map back the
>  	 * reported fids to filesystem objects.
>  	 */
> -	if (mark_type != FAN_MARK_INODE && !nop->fh_to_dentry)
> +	if (mark_type != FAN_MARK_INODE && !exportfs_can_decode_fh(nop))
>  		return -EOPNOTSUPP;
>  
>  	return 0;
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 89e0d60d35b6..f0a712214ec2 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -55,7 +55,7 @@ int ovl_can_decode_fh(struct super_block *sb)
>  	if (!capable(CAP_DAC_READ_SEARCH))
>  		return 0;
>  
> -	if (!sb->s_export_op || !sb->s_export_op->fh_to_dentry)
> +	if (!exportfs_can_decode_fh(sb->s_export_op))
>  		return 0;
>  
>  	return sb->s_export_op->encode_fh ? -1 : FILEID_INO32_GEN;
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 11fbd0ee1370..5b3c9f30b422 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -233,6 +233,33 @@ extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>  			      int *max_len, int flags);
>  
> +static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
> +{
> +	return nop;
> +}
> +
> +static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> +{
> +	return nop && nop->fh_to_dentry;
> +}
> +
> +static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
> +					  int fh_flags)
> +{
> +	/*
> +	 * If a non-decodeable file handle was requested, we only need to make
> +	 * sure that filesystem can encode file handles.
> +	 */
> +	if (fh_flags & EXPORT_FH_FID)
> +		return exportfs_can_encode_fid(nop);
> +
> +	/*
> +	 * If a decodeable file handle was requested, we need to make sure that
> +	 * filesystem can also decode file handles.
> +	 */
> +	return exportfs_can_decode_fh(nop);
> +}
> +
>  static inline int exportfs_encode_fid(struct inode *inode, struct fid *fid,
>  				      int *max_len)
>  {

Nice cleanup.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

[Jeff Layton]

On Wed, 2023-10-18 at 12:59 +0300, Amir Goldstein wrote:
> export_operations ->encode_fh() no longer has a default implementation to
> encode FILEID_INO32_GEN* file handles.
> 
> Rename the default helper for encoding FILEID_INO32_GEN* file handles to
> generic_encode_ino32_fh() and convert the filesystems that used the
> default implementation to use the generic helper explicitly.
> 
> This is a step towards allowing filesystems to encode non-decodeable file
> handles for fanotify without having to implement any export_operations.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  Documentation/filesystems/nfs/exporting.rst |  7 ++-----
>  Documentation/filesystems/porting.rst       |  9 +++++++++
>  fs/affs/namei.c                             |  1 +
>  fs/befs/linuxvfs.c                          |  1 +
>  fs/efs/super.c                              |  1 +
>  fs/erofs/super.c                            |  1 +
>  fs/exportfs/expfs.c                         | 14 ++++++++------
>  fs/ext2/super.c                             |  1 +
>  fs/ext4/super.c                             |  1 +
>  fs/f2fs/super.c                             |  1 +
>  fs/fat/nfs.c                                |  1 +
>  fs/jffs2/super.c                            |  1 +
>  fs/jfs/super.c                              |  1 +
>  fs/ntfs/namei.c                             |  1 +
>  fs/ntfs3/super.c                            |  1 +
>  fs/smb/client/export.c                      |  9 +++------
>  fs/squashfs/export.c                        |  1 +
>  fs/ufs/super.c                              |  1 +
>  include/linux/exportfs.h                    |  4 +++-
>  19 files changed, 39 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/filesystems/nfs/exporting.rst b/Documentation/filesystems/nfs/exporting.rst
> index 4b30daee399a..de64d2d002a2 100644
> --- a/Documentation/filesystems/nfs/exporting.rst
> +++ b/Documentation/filesystems/nfs/exporting.rst
> @@ -122,12 +122,9 @@ are exportable by setting the s_export_op field in the struct
>  super_block.  This field must point to a "struct export_operations"
>  struct which has the following members:
>  
> -  encode_fh (optional)
> +  encode_fh (mandatory)
>      Takes a dentry and creates a filehandle fragment which may later be used
> -    to find or create a dentry for the same object.  The default
> -    implementation creates a filehandle fragment that encodes a 32bit inode
> -    and generation number for the inode encoded, and if necessary the
> -    same information for the parent.
> +    to find or create a dentry for the same object.
>  
>    fh_to_dentry (mandatory)
>      Given a filehandle fragment, this should find the implied object and
> diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
> index 4d05b9862451..197ef78a5014 100644
> --- a/Documentation/filesystems/porting.rst
> +++ b/Documentation/filesystems/porting.rst
> @@ -1045,3 +1045,12 @@ filesystem type is now moved to a later point when the devices are closed:
>  As this is a VFS level change it has no practical consequences for filesystems
>  other than that all of them must use one of the provided kill_litter_super(),
>  kill_anon_super(), or kill_block_super() helpers.
> +
> +---
> +
> +**mandatory**
> +
> +export_operations ->encode_fh() no longer has a default implementation to
> +encode FILEID_INO32_GEN* file handles.
> +Fillesystems that used the default implementation may use the generic helper
> +generic_encode_ino32_fh() explicitly.
> diff --git a/fs/affs/namei.c b/fs/affs/namei.c
> index 2fe4a5832fcf..d6b9758ee23d 100644
> --- a/fs/affs/namei.c
> +++ b/fs/affs/namei.c
> @@ -568,6 +568,7 @@ static struct dentry *affs_fh_to_parent(struct super_block *sb, struct fid *fid,
>  }
>  
>  const struct export_operations affs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = affs_fh_to_dentry,
>  	.fh_to_parent = affs_fh_to_parent,
>  	.get_parent = affs_get_parent,
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index 9a16a51fbb88..410dcaffd5ab 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -96,6 +96,7 @@ static const struct address_space_operations befs_symlink_aops = {
>  };
>  
>  static const struct export_operations befs_export_operations = {
> +	.encode_fh	= generic_encode_ino32_fh,
>  	.fh_to_dentry	= befs_fh_to_dentry,
>  	.fh_to_parent	= befs_fh_to_parent,
>  	.get_parent	= befs_get_parent,
> diff --git a/fs/efs/super.c b/fs/efs/super.c
> index b287f47c165b..f17fdac76b2e 100644
> --- a/fs/efs/super.c
> +++ b/fs/efs/super.c
> @@ -123,6 +123,7 @@ static const struct super_operations efs_superblock_operations = {
>  };
>  
>  static const struct export_operations efs_export_ops = {
> +	.encode_fh	= generic_encode_ino32_fh,
>  	.fh_to_dentry	= efs_fh_to_dentry,
>  	.fh_to_parent	= efs_fh_to_parent,
>  	.get_parent	= efs_get_parent,
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 3700af9ee173..edbe07a24156 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -626,6 +626,7 @@ static struct dentry *erofs_get_parent(struct dentry *child)
>  }
>  
>  static const struct export_operations erofs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = erofs_fh_to_dentry,
>  	.fh_to_parent = erofs_fh_to_parent,
>  	.get_parent = erofs_get_parent,
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 9ee205df8fa7..30da4539e257 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -343,20 +343,21 @@ static int get_name(const struct path *path, char *name, struct dentry *child)
>  }
>  
>  /**
> - * export_encode_fh - default export_operations->encode_fh function
> + * generic_encode_ino32_fh - generic export_operations->encode_fh function
>   * @inode:   the object to encode
> - * @fid:     where to store the file handle fragment
> + * @fh:      where to store the file handle fragment
>   * @max_len: maximum length to store there
>   * @parent:  parent directory inode, if wanted
>   *
> - * This default encode_fh function assumes that the 32 inode number
> + * This generic encode_fh function assumes that the 32 inode number
>   * is suitable for locating an inode, and that the generation number
>   * can be used to check that it is still valid.  It places them in the
>   * filehandle fragment where export_decode_fh expects to find them.
>   */
> -static int export_encode_fh(struct inode *inode, struct fid *fid,
> -		int *max_len, struct inode *parent)
> +int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> +			    struct inode *parent)
>  {
> +	struct fid *fid = (void *)fh;
>  	int len = *max_len;
>  	int type = FILEID_INO32_GEN;
>  
> @@ -380,6 +381,7 @@ static int export_encode_fh(struct inode *inode, struct fid *fid,
>  	*max_len = len;
>  	return type;
>  }
> +EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
>  
>  /**
>   * exportfs_encode_inode_fh - encode a file handle from inode
> @@ -402,7 +404,7 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  	if (nop && nop->encode_fh)
>  		return nop->encode_fh(inode, fid->raw, max_len, parent);
>  
> -	return export_encode_fh(inode, fid, max_len, parent);
> +	return -EOPNOTSUPP;
>  }
>  EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
>  
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index aaf3e3e88cb2..b9f158a34997 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -397,6 +397,7 @@ static struct dentry *ext2_fh_to_parent(struct super_block *sb, struct fid *fid,
>  }
>  
>  static const struct export_operations ext2_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = ext2_fh_to_dentry,
>  	.fh_to_parent = ext2_fh_to_parent,
>  	.get_parent = ext2_get_parent,
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index dbebd8b3127e..c44db1915437 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1646,6 +1646,7 @@ static const struct super_operations ext4_sops = {
>  };
>  
>  static const struct export_operations ext4_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = ext4_fh_to_dentry,
>  	.fh_to_parent = ext4_fh_to_parent,
>  	.get_parent = ext4_get_parent,
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index a8c8232852bb..60cfa11f65bf 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3282,6 +3282,7 @@ static struct dentry *f2fs_fh_to_parent(struct super_block *sb, struct fid *fid,
>  }
>  
>  static const struct export_operations f2fs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = f2fs_fh_to_dentry,
>  	.fh_to_parent = f2fs_fh_to_parent,
>  	.get_parent = f2fs_get_parent,
> diff --git a/fs/fat/nfs.c b/fs/fat/nfs.c
> index 3626eb585a98..c52e63e10d35 100644
> --- a/fs/fat/nfs.c
> +++ b/fs/fat/nfs.c
> @@ -279,6 +279,7 @@ static struct dentry *fat_get_parent(struct dentry *child_dir)
>  }
>  
>  const struct export_operations fat_export_ops = {
> +	.encode_fh	= generic_encode_ino32_fh,
>  	.fh_to_dentry   = fat_fh_to_dentry,
>  	.fh_to_parent   = fat_fh_to_parent,
>  	.get_parent     = fat_get_parent,
> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> index 7ea37f49f1e1..f99591a634b4 100644
> --- a/fs/jffs2/super.c
> +++ b/fs/jffs2/super.c
> @@ -150,6 +150,7 @@ static struct dentry *jffs2_get_parent(struct dentry *child)
>  }
>  
>  static const struct export_operations jffs2_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.get_parent = jffs2_get_parent,
>  	.fh_to_dentry = jffs2_fh_to_dentry,
>  	.fh_to_parent = jffs2_fh_to_parent,
> diff --git a/fs/jfs/super.c b/fs/jfs/super.c
> index 2e2f7f6d36a0..2cc2632f3c47 100644
> --- a/fs/jfs/super.c
> +++ b/fs/jfs/super.c
> @@ -896,6 +896,7 @@ static const struct super_operations jfs_super_operations = {
>  };
>  
>  static const struct export_operations jfs_export_operations = {
> +	.encode_fh	= generic_encode_ino32_fh,
>  	.fh_to_dentry	= jfs_fh_to_dentry,
>  	.fh_to_parent	= jfs_fh_to_parent,
>  	.get_parent	= jfs_get_parent,
> diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
> index ab44f2db533b..d7498ddc4a72 100644
> --- a/fs/ntfs/namei.c
> +++ b/fs/ntfs/namei.c
> @@ -384,6 +384,7 @@ static struct dentry *ntfs_fh_to_parent(struct super_block *sb, struct fid *fid,
>   * and due to using iget() whereas NTFS needs ntfs_iget().
>   */
>  const struct export_operations ntfs_export_ops = {
> +	.encode_fh	= generic_encode_ino32_fh,
>  	.get_parent	= ntfs_get_parent,	/* Find the parent of a given
>  						   directory. */
>  	.fh_to_dentry	= ntfs_fh_to_dentry,
> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index 5661a363005e..661ffb5aa1e0 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -789,6 +789,7 @@ static int ntfs_nfs_commit_metadata(struct inode *inode)
>  }
>  
>  static const struct export_operations ntfs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = ntfs_fh_to_dentry,
>  	.fh_to_parent = ntfs_fh_to_parent,
>  	.get_parent = ntfs3_get_parent,
> diff --git a/fs/smb/client/export.c b/fs/smb/client/export.c
> index 37c28415df1e..834e9c9197b4 100644
> --- a/fs/smb/client/export.c
> +++ b/fs/smb/client/export.c
> @@ -41,13 +41,10 @@ static struct dentry *cifs_get_parent(struct dentry *dentry)
>  }
>  
>  const struct export_operations cifs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.get_parent = cifs_get_parent,
> -/*	Following five export operations are unneeded so far and can default:
> -	.get_dentry =
> -	.get_name =
> -	.find_exported_dentry =
> -	.decode_fh =
> -	.encode_fs =  */
> +/*	Following export operations are mandatory for NFS export support:
> +	.fh_to_dentry = */
>  };
>  
>  #endif /* CONFIG_CIFS_NFSD_EXPORT */
> diff --git a/fs/squashfs/export.c b/fs/squashfs/export.c
> index 723763746238..62972f0ff868 100644
> --- a/fs/squashfs/export.c
> +++ b/fs/squashfs/export.c
> @@ -173,6 +173,7 @@ __le64 *squashfs_read_inode_lookup_table(struct super_block *sb,
>  
>  
>  const struct export_operations squashfs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = squashfs_fh_to_dentry,
>  	.fh_to_parent = squashfs_fh_to_parent,
>  	.get_parent = squashfs_get_parent
> diff --git a/fs/ufs/super.c b/fs/ufs/super.c
> index 23377c1baed9..a480810cd4e3 100644
> --- a/fs/ufs/super.c
> +++ b/fs/ufs/super.c
> @@ -137,6 +137,7 @@ static struct dentry *ufs_get_parent(struct dentry *child)
>  }
>  
>  static const struct export_operations ufs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry	= ufs_fh_to_dentry,
>  	.fh_to_parent	= ufs_fh_to_parent,
>  	.get_parent	= ufs_get_parent,
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 5b3c9f30b422..6b6e01321405 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -235,7 +235,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>  
>  static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
>  {
> -	return nop;
> +	return nop && nop->encode_fh;
>  }
>  
>  static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> @@ -279,6 +279,8 @@ extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
>  /*
>   * Generic helpers for filesystems.
>   */
> +int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> +			    struct inode *parent);
>  extern struct dentry *generic_fh_to_dentry(struct super_block *sb,
>  	struct fid *fid, int fh_len, int fh_type,
>  	struct inode *(*get_inode) (struct super_block *sb, u64 ino, u32 gen));

Looks straightforward.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH 4/5] exportfs: define FILEID_INO64_GEN* file handle types

[Jeff Layton]

On Wed, 2023-10-18 at 12:59 +0300, Amir Goldstein wrote:
> Similar to the common FILEID_INO32* file handle types, define common
> FILEID_INO64* file handle types.
> 
> The type values of FILEID_INO64_GEN and FILEID_INO64_GEN_PARENT are the
> values returned by fuse and xfs for 64bit ino encoded file handle types.
> 
> Note that these type value are filesystem specific and they do not define
> a universal file handle format, for example:
> fuse encodes FILEID_INO64_GEN as [ino-hi32,ino-lo32,gen] and xfs encodes
> FILEID_INO64_GEN as [hostr-order-ino64,gen] (a.k.a xfs_fid64).
> 
> The FILEID_INO64_GEN fhandle type is going to be used for file ids for
> fanotify from filesystems that do not support NFS export.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/fuse/inode.c          |  7 ++++---
>  include/linux/exportfs.h | 11 +++++++++++
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 2e4eb7cf26fb..e63f966698a5 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1002,7 +1002,7 @@ static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
>  	}
>  
>  	*max_len = len;
> -	return parent ? 0x82 : 0x81;
> +	return parent ? FILEID_INO64_GEN_PARENT : FILEID_INO64_GEN;
>  }
>  
>  static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
> @@ -1010,7 +1010,8 @@ static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
>  {
>  	struct fuse_inode_handle handle;
>  
> -	if ((fh_type != 0x81 && fh_type != 0x82) || fh_len < 3)
> +	if ((fh_type != FILEID_INO64_GEN &&
> +	     fh_type != FILEID_INO64_GEN_PARENT) || fh_len < 3)
>  		return NULL;
>  
>  	handle.nodeid = (u64) fid->raw[0] << 32;
> @@ -1024,7 +1025,7 @@ static struct dentry *fuse_fh_to_parent(struct super_block *sb,
>  {
>  	struct fuse_inode_handle parent;
>  
> -	if (fh_type != 0x82 || fh_len < 6)
> +	if (fh_type != FILEID_INO64_GEN_PARENT || fh_len < 6)
>  		return NULL;
>  
>  	parent.nodeid = (u64) fid->raw[3] << 32;
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 6b6e01321405..21eeb9f6bdbd 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -98,6 +98,17 @@ enum fid_type {
>  	 */
>  	FILEID_FAT_WITH_PARENT = 0x72,
>  
> +	/*
> +	 * 64 bit inode number, 32 bit generation number.
> +	 */
> +	FILEID_INO64_GEN = 0x81,
> +
> +	/*
> +	 * 64 bit inode number, 32 bit generation number,
> +	 * 64 bit parent inode number, 32 bit parent generation.
> +	 */
> +	FILEID_INO64_GEN_PARENT = 0x82,
> +
>  	/*
>  	 * 128 bit child FID (struct lu_fid)
>  	 * 128 bit parent FID (struct lu_fid)

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH 5/5] exportfs: support encoding non-decodeable file handles by default

[Jeff Layton]

On Wed, 2023-10-18 at 13:00 +0300, Amir Goldstein wrote:
> AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> the encoding of a file id, which is not intended to be decoded.
> 
> This file id is used by fanotify to describe objects in events.
> 
> So far, overlayfs is the only filesystem that supports encoding
> non-decodeable file ids, by providing export_operations with an
> ->encode_fh() method and without a ->decode_fh() method.
> 
> Add support for encoding non-decodeable file ids to all the filesystems
> that do not provide export_operations, by encoding a file id of type
> FILEID_INO64_GEN from { i_ino, i_generation }.
> 
> A filesystem may that does not support NFS export, can opt-out of
> encoding non-decodeable file ids for fanotify by defining an empty
> export_operations struct (i.e. with a NULL ->encode_fh() method).
> 
> This allows the use of fanotify events with file ids on filesystems
> like 9p which do not support NFS export to bring fanotify in feature
> parity with inotify on those filesystems.
> 
> Note that fanotify also requires that the filesystems report a non-null
> fsid.  Currently, many simple filesystems that have support for inotify
> (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> used with fanotify in file id reporting mode.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
>  include/linux/exportfs.h | 10 +++++++---
>  2 files changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 30da4539e257..34e7d835d4ef 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
>  }
>  EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
>  
> +/**
> + * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
> + * @inode:   the object to encode
> + * @fid:     where to store the file handle fragment
> + * @max_len: maximum length to store there
> + *
> + * This generic function is used to encode a non-decodeable file id for
> + * fanotify for filesystems that do not support NFS export.
> + */
> +static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
> +				     int *max_len)
> +{
> +	if (*max_len < 3) {
> +		*max_len = 3;
> +		return FILEID_INVALID;
> +	}
> +
> +	fid->i64.ino = inode->i_ino;
> +	fid->i64.gen = inode->i_generation;

Note that i_ino is unsigned long and so is a 32-bit value on 32-bit
arches. If the backend storage uses 64-bit inodes, then we usually end
up hashing them down to 32-bits first. e.g. see nfs_fattr_to_ino_t().
ceph has some similar code.

The upshot is that if you're relying on i_ino, the value can change
between different arches, even when they are dealing with the same
backend filesystem.

Since this is expected to be used by filesystems that don't set up
export operations, then that may just be something they have to deal
with. I'm not sure what else you can use in lieu of i_ino in this case.

> +	*max_len = 3;
> +
> +	return FILEID_INO64_GEN;
> +}
> +
>  /**
>   * exportfs_encode_inode_fh - encode a file handle from inode
>   * @inode:   the object to encode
> @@ -401,10 +425,10 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  	if (!exportfs_can_encode_fh(nop, flags))
>  		return -EOPNOTSUPP;
>  
> -	if (nop && nop->encode_fh)
> -		return nop->encode_fh(inode, fid->raw, max_len, parent);
> +	if (!nop && (flags & EXPORT_FH_FID))
> +		return exportfs_encode_ino64_fid(inode, fid, max_len);
>  
> -	return -EOPNOTSUPP;
> +	return nop->encode_fh(inode, fid->raw, max_len, parent);
>  }
>  EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
>  
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 21eeb9f6bdbd..6688e457da64 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -134,7 +134,11 @@ struct fid {
>  			u32 parent_ino;
>  			u32 parent_gen;
>  		} i32;
> - 		struct {
> +		struct {
> +			u64 ino;
> +			u32 gen;
> +		} __packed i64;
> +		struct {
>   			u32 block;
>   			u16 partref;
>   			u16 parent_partref;
> @@ -246,7 +250,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>  
>  static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
>  {
> -	return nop && nop->encode_fh;
> +	return !nop || nop->encode_fh;
>  }
>  
>  static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> @@ -259,7 +263,7 @@ static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
>  {
>  	/*
>  	 * If a non-decodeable file handle was requested, we only need to make
> -	 * sure that filesystem can encode file handles.
> +	 * sure that filesystem did not opt-out of encoding fid.
>  	 */
>  	if (fh_flags & EXPORT_FH_FID)
>  		return exportfs_can_encode_fid(nop);

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

[Dave Kleikamp]

On 10/18/23 9:16AM, Jeff Layton wrote:
> On Wed, 2023-10-18 at 12:59 +0300, Amir Goldstein wrote:
>> export_operations ->encode_fh() no longer has a default implementation to
>> encode FILEID_INO32_GEN* file handles.
>>
>> Rename the default helper for encoding FILEID_INO32_GEN* file handles to
>> generic_encode_ino32_fh() and convert the filesystems that used the
>> default implementation to use the generic helper explicitly.

Isn't it possible for some of these filesystems to be compiled without 
CONFIG_EXPORTFS set? Should exportfs.h define an null 
generic_encode_ino32_fh() in that case?

Shaggy

>>
>> This is a step towards allowing filesystems to encode non-decodeable file
>> handles for fanotify without having to implement any export_operations.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>   Documentation/filesystems/nfs/exporting.rst |  7 ++-----
>>   Documentation/filesystems/porting.rst       |  9 +++++++++
>>   fs/affs/namei.c                             |  1 +
>>   fs/befs/linuxvfs.c                          |  1 +
>>   fs/efs/super.c                              |  1 +
>>   fs/erofs/super.c                            |  1 +
>>   fs/exportfs/expfs.c                         | 14 ++++++++------
>>   fs/ext2/super.c                             |  1 +
>>   fs/ext4/super.c                             |  1 +
>>   fs/f2fs/super.c                             |  1 +
>>   fs/fat/nfs.c                                |  1 +
>>   fs/jffs2/super.c                            |  1 +
>>   fs/jfs/super.c                              |  1 +
>>   fs/ntfs/namei.c                             |  1 +
>>   fs/ntfs3/super.c                            |  1 +
>>   fs/smb/client/export.c                      |  9 +++------
>>   fs/squashfs/export.c                        |  1 +
>>   fs/ufs/super.c                              |  1 +
>>   include/linux/exportfs.h                    |  4 +++-
>>   19 files changed, 39 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/filesystems/nfs/exporting.rst b/Documentation/filesystems/nfs/exporting.rst
>> index 4b30daee399a..de64d2d002a2 100644
>> --- a/Documentation/filesystems/nfs/exporting.rst
>> +++ b/Documentation/filesystems/nfs/exporting.rst
>> @@ -122,12 +122,9 @@ are exportable by setting the s_export_op field in the struct
>>   super_block.  This field must point to a "struct export_operations"
>>   struct which has the following members:
>>   
>> -  encode_fh (optional)
>> +  encode_fh (mandatory)
>>       Takes a dentry and creates a filehandle fragment which may later be used
>> -    to find or create a dentry for the same object.  The default
>> -    implementation creates a filehandle fragment that encodes a 32bit inode
>> -    and generation number for the inode encoded, and if necessary the
>> -    same information for the parent.
>> +    to find or create a dentry for the same object.
>>   
>>     fh_to_dentry (mandatory)
>>       Given a filehandle fragment, this should find the implied object and
>> diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
>> index 4d05b9862451..197ef78a5014 100644
>> --- a/Documentation/filesystems/porting.rst
>> +++ b/Documentation/filesystems/porting.rst
>> @@ -1045,3 +1045,12 @@ filesystem type is now moved to a later point when the devices are closed:
>>   As this is a VFS level change it has no practical consequences for filesystems
>>   other than that all of them must use one of the provided kill_litter_super(),
>>   kill_anon_super(), or kill_block_super() helpers.
>> +
>> +---
>> +
>> +**mandatory**
>> +
>> +export_operations ->encode_fh() no longer has a default implementation to
>> +encode FILEID_INO32_GEN* file handles.
>> +Fillesystems that used the default implementation may use the generic helper
>> +generic_encode_ino32_fh() explicitly.
>> diff --git a/fs/affs/namei.c b/fs/affs/namei.c
>> index 2fe4a5832fcf..d6b9758ee23d 100644
>> --- a/fs/affs/namei.c
>> +++ b/fs/affs/namei.c
>> @@ -568,6 +568,7 @@ static struct dentry *affs_fh_to_parent(struct super_block *sb, struct fid *fid,
>>   }
>>   
>>   const struct export_operations affs_export_ops = {
>> +	.encode_fh = generic_encode_ino32_fh,
>>   	.fh_to_dentry = affs_fh_to_dentry,
>>   	.fh_to_parent = affs_fh_to_parent,
>>   	.get_parent = affs_get_parent,
>> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
>> index 9a16a51fbb88..410dcaffd5ab 100644
>> --- a/fs/befs/linuxvfs.c
>> +++ b/fs/befs/linuxvfs.c
>> @@ -96,6 +96,7 @@ static const struct address_space_operations befs_symlink_aops = {
>>   };
>>   
>>   static const struct export_operations befs_export_operations = {
>> +	.encode_fh	= generic_encode_ino32_fh,
>>   	.fh_to_dentry	= befs_fh_to_dentry,
>>   	.fh_to_parent	= befs_fh_to_parent,
>>   	.get_parent	= befs_get_parent,
>> diff --git a/fs/efs/super.c b/fs/efs/super.c
>> index b287f47c165b..f17fdac76b2e 100644
>> --- a/fs/efs/super.c
>> +++ b/fs/efs/super.c
>> @@ -123,6 +123,7 @@ static const struct super_operations efs_superblock_operations = {
>>   };
>>   
>>   static const struct export_operations efs_export_ops = {
>> +	.encode_fh	= generic_encode_ino32_fh,
>>   	.fh_to_dentry	= efs_fh_to_dentry,
>>   	.fh_to_parent	= efs_fh_to_parent,
>>   	.get_parent	= efs_get_parent,
>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>> index 3700af9ee173..edbe07a24156 100644
>> --- a/fs/erofs/super.c
>> +++ b/fs/erofs/super.c
>> @@ -626,6 +626,7 @@ static struct dentry *erofs_get_parent(struct dentry *child)
>>   }
>>   
>>   static const struct export_operations erofs_export_ops = {
>> +	.encode_fh = generic_encode_ino32_fh,
>>   	.fh_to_dentry = erofs_fh_to_dentry,
>>   	.fh_to_parent = erofs_fh_to_parent,
>>   	.get_parent = erofs_get_parent,
>> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
>> index 9ee205df8fa7..30da4539e257 100644
>> --- a/fs/exportfs/expfs.c
>> +++ b/fs/exportfs/expfs.c
>> @@ -343,20 +343,21 @@ static int get_name(const struct path *path, char *name, struct dentry *child)
>>   }
>>   
>>   /**
>> - * export_encode_fh - default export_operations->encode_fh function
>> + * generic_encode_ino32_fh - generic export_operations->encode_fh function
>>    * @inode:   the object to encode
>> - * @fid:     where to store the file handle fragment
>> + * @fh:      where to store the file handle fragment
>>    * @max_len: maximum length to store there
>>    * @parent:  parent directory inode, if wanted
>>    *
>> - * This default encode_fh function assumes that the 32 inode number
>> + * This generic encode_fh function assumes that the 32 inode number
>>    * is suitable for locating an inode, and that the generation number
>>    * can be used to check that it is still valid.  It places them in the
>>    * filehandle fragment where export_decode_fh expects to find them.
>>    */
>> -static int export_encode_fh(struct inode *inode, struct fid *fid,
>> -		int *max_len, struct inode *parent)
>> +int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
>> +			    struct inode *parent)
>>   {
>> +	struct fid *fid = (void *)fh;
>>   	int len = *max_len;
>>   	int type = FILEID_INO32_GEN;
>>   
>> @@ -380,6 +381,7 @@ static int export_encode_fh(struct inode *inode, struct fid *fid,
>>   	*max_len = len;
>>   	return type;
>>   }
>> +EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
>>   
>>   /**
>>    * exportfs_encode_inode_fh - encode a file handle from inode
>> @@ -402,7 +404,7 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>>   	if (nop && nop->encode_fh)
>>   		return nop->encode_fh(inode, fid->raw, max_len, parent);
>>   
>> -	return export_encode_fh(inode, fid, max_len, parent);
>> +	return -EOPNOTSUPP;
>>   }
>>   EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
>>   
>> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
>> index aaf3e3e88cb2..b9f158a34997 100644
>> --- a/fs/ext2/super.c
>> +++ b/fs/ext2/super.c
>> @@ -397,6 +397,7 @@ static struct dentry *ext2_fh_to_parent(struct super_block *sb, struct fid *fid,
>>   }
>>   
>>   static const struct export_operations ext2_export_ops = {
>> +	.encode_fh = generic_encode_ino32_fh,
>>   	.fh_to_dentry = ext2_fh_to_dentry,
>>   	.fh_to_parent = ext2_fh_to_parent,
>>   	.get_parent = ext2_get_parent,
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index dbebd8b3127e..c44db1915437 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -1646,6 +1646,7 @@ static const struct super_operations ext4_sops = {
>>   };
>>   
>>   static const struct export_operations ext4_export_ops = {
>> +	.encode_fh = generic_encode_ino32_fh,
>>   	.fh_to_dentry = ext4_fh_to_dentry,
>>   	.fh_to_parent = ext4_fh_to_parent,
>>   	.get_parent = ext4_get_parent,
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index a8c8232852bb..60cfa11f65bf 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -3282,6 +3282,7 @@ static struct dentry *f2fs_fh_to_parent(struct super_block *sb, struct fid *fid,
>>   }
>>   
>>   static const struct export_operations f2fs_export_ops = {
>> +	.encode_fh = generic_encode_ino32_fh,
>>   	.fh_to_dentry = f2fs_fh_to_dentry,
>>   	.fh_to_parent = f2fs_fh_to_parent,
>>   	.get_parent = f2fs_get_parent,
>> diff --git a/fs/fat/nfs.c b/fs/fat/nfs.c
>> index 3626eb585a98..c52e63e10d35 100644
>> --- a/fs/fat/nfs.c
>> +++ b/fs/fat/nfs.c
>> @@ -279,6 +279,7 @@ static struct dentry *fat_get_parent(struct dentry *child_dir)
>>   }
>>   
>>   const struct export_operations fat_export_ops = {
>> +	.encode_fh	= generic_encode_ino32_fh,
>>   	.fh_to_dentry   = fat_fh_to_dentry,
>>   	.fh_to_parent   = fat_fh_to_parent,
>>   	.get_parent     = fat_get_parent,
>> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
>> index 7ea37f49f1e1..f99591a634b4 100644
>> --- a/fs/jffs2/super.c
>> +++ b/fs/jffs2/super.c
>> @@ -150,6 +150,7 @@ static struct dentry *jffs2_get_parent(struct dentry *child)
>>   }
>>   
>>   static const struct export_operations jffs2_export_ops = {
>> +	.encode_fh = generic_encode_ino32_fh,
>>   	.get_parent = jffs2_get_parent,
>>   	.fh_to_dentry = jffs2_fh_to_dentry,
>>   	.fh_to_parent = jffs2_fh_to_parent,
>> diff --git a/fs/jfs/super.c b/fs/jfs/super.c
>> index 2e2f7f6d36a0..2cc2632f3c47 100644
>> --- a/fs/jfs/super.c
>> +++ b/fs/jfs/super.c
>> @@ -896,6 +896,7 @@ static const struct super_operations jfs_super_operations = {
>>   };
>>   
>>   static const struct export_operations jfs_export_operations = {
>> +	.encode_fh	= generic_encode_ino32_fh,
>>   	.fh_to_dentry	= jfs_fh_to_dentry,
>>   	.fh_to_parent	= jfs_fh_to_parent,
>>   	.get_parent	= jfs_get_parent,
>> diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
>> index ab44f2db533b..d7498ddc4a72 100644
>> --- a/fs/ntfs/namei.c
>> +++ b/fs/ntfs/namei.c
>> @@ -384,6 +384,7 @@ static struct dentry *ntfs_fh_to_parent(struct super_block *sb, struct fid *fid,
>>    * and due to using iget() whereas NTFS needs ntfs_iget().
>>    */
>>   const struct export_operations ntfs_export_ops = {
>> +	.encode_fh	= generic_encode_ino32_fh,
>>   	.get_parent	= ntfs_get_parent,	/* Find the parent of a given
>>   						   directory. */
>>   	.fh_to_dentry	= ntfs_fh_to_dentry,
>> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
>> index 5661a363005e..661ffb5aa1e0 100644
>> --- a/fs/ntfs3/super.c
>> +++ b/fs/ntfs3/super.c
>> @@ -789,6 +789,7 @@ static int ntfs_nfs_commit_metadata(struct inode *inode)
>>   }
>>   
>>   static const struct export_operations ntfs_export_ops = {
>> +	.encode_fh = generic_encode_ino32_fh,
>>   	.fh_to_dentry = ntfs_fh_to_dentry,
>>   	.fh_to_parent = ntfs_fh_to_parent,
>>   	.get_parent = ntfs3_get_parent,
>> diff --git a/fs/smb/client/export.c b/fs/smb/client/export.c
>> index 37c28415df1e..834e9c9197b4 100644
>> --- a/fs/smb/client/export.c
>> +++ b/fs/smb/client/export.c
>> @@ -41,13 +41,10 @@ static struct dentry *cifs_get_parent(struct dentry *dentry)
>>   }
>>   
>>   const struct export_operations cifs_export_ops = {
>> +	.encode_fh = generic_encode_ino32_fh,
>>   	.get_parent = cifs_get_parent,
>> -/*	Following five export operations are unneeded so far and can default:
>> -	.get_dentry =
>> -	.get_name =
>> -	.find_exported_dentry =
>> -	.decode_fh =
>> -	.encode_fs =  */
>> +/*	Following export operations are mandatory for NFS export support:
>> +	.fh_to_dentry = */
>>   };
>>   
>>   #endif /* CONFIG_CIFS_NFSD_EXPORT */
>> diff --git a/fs/squashfs/export.c b/fs/squashfs/export.c
>> index 723763746238..62972f0ff868 100644
>> --- a/fs/squashfs/export.c
>> +++ b/fs/squashfs/export.c
>> @@ -173,6 +173,7 @@ __le64 *squashfs_read_inode_lookup_table(struct super_block *sb,
>>   
>>   
>>   const struct export_operations squashfs_export_ops = {
>> +	.encode_fh = generic_encode_ino32_fh,
>>   	.fh_to_dentry = squashfs_fh_to_dentry,
>>   	.fh_to_parent = squashfs_fh_to_parent,
>>   	.get_parent = squashfs_get_parent
>> diff --git a/fs/ufs/super.c b/fs/ufs/super.c
>> index 23377c1baed9..a480810cd4e3 100644
>> --- a/fs/ufs/super.c
>> +++ b/fs/ufs/super.c
>> @@ -137,6 +137,7 @@ static struct dentry *ufs_get_parent(struct dentry *child)
>>   }
>>   
>>   static const struct export_operations ufs_export_ops = {
>> +	.encode_fh = generic_encode_ino32_fh,
>>   	.fh_to_dentry	= ufs_fh_to_dentry,
>>   	.fh_to_parent	= ufs_fh_to_parent,
>>   	.get_parent	= ufs_get_parent,
>> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
>> index 5b3c9f30b422..6b6e01321405 100644
>> --- a/include/linux/exportfs.h
>> +++ b/include/linux/exportfs.h
>> @@ -235,7 +235,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>>   
>>   static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
>>   {
>> -	return nop;
>> +	return nop && nop->encode_fh;
>>   }
>>   
>>   static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
>> @@ -279,6 +279,8 @@ extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
>>   /*
>>    * Generic helpers for filesystems.
>>    */
>> +int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
>> +			    struct inode *parent);
>>   extern struct dentry *generic_fh_to_dentry(struct super_block *sb,
>>   	struct fid *fid, int fh_len, int fh_type,
>>   	struct inode *(*get_inode) (struct super_block *sb, u64 ino, u32 gen));
> 
> Looks straightforward.
> 
> Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH 5/5] exportfs: support encoding non-decodeable file handles by default

[Amir Goldstein]

On Wed, Oct 18, 2023 at 5:28 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Wed, 2023-10-18 at 13:00 +0300, Amir Goldstein wrote:
> > AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> > the encoding of a file id, which is not intended to be decoded.
> >
> > This file id is used by fanotify to describe objects in events.
> >
> > So far, overlayfs is the only filesystem that supports encoding
> > non-decodeable file ids, by providing export_operations with an
> > ->encode_fh() method and without a ->decode_fh() method.
> >
> > Add support for encoding non-decodeable file ids to all the filesystems
> > that do not provide export_operations, by encoding a file id of type
> > FILEID_INO64_GEN from { i_ino, i_generation }.
> >
> > A filesystem may that does not support NFS export, can opt-out of
> > encoding non-decodeable file ids for fanotify by defining an empty
> > export_operations struct (i.e. with a NULL ->encode_fh() method).
> >
> > This allows the use of fanotify events with file ids on filesystems
> > like 9p which do not support NFS export to bring fanotify in feature
> > parity with inotify on those filesystems.
> >
> > Note that fanotify also requires that the filesystems report a non-null
> > fsid.  Currently, many simple filesystems that have support for inotify
> > (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> > used with fanotify in file id reporting mode.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
> >  include/linux/exportfs.h | 10 +++++++---
> >  2 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> > index 30da4539e257..34e7d835d4ef 100644
> > --- a/fs/exportfs/expfs.c
> > +++ b/fs/exportfs/expfs.c
> > @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> >  }
> >  EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
> >
> > +/**
> > + * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
> > + * @inode:   the object to encode
> > + * @fid:     where to store the file handle fragment
> > + * @max_len: maximum length to store there
> > + *
> > + * This generic function is used to encode a non-decodeable file id for
> > + * fanotify for filesystems that do not support NFS export.
> > + */
> > +static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
> > +                                  int *max_len)
> > +{
> > +     if (*max_len < 3) {
> > +             *max_len = 3;
> > +             return FILEID_INVALID;
> > +     }
> > +
> > +     fid->i64.ino = inode->i_ino;
> > +     fid->i64.gen = inode->i_generation;
>
> Note that i_ino is unsigned long and so is a 32-bit value on 32-bit
> arches. If the backend storage uses 64-bit inodes, then we usually end
> up hashing them down to 32-bits first. e.g. see nfs_fattr_to_ino_t().
> ceph has some similar code.
>
> The upshot is that if you're relying on i_ino, the value can change
> between different arches, even when they are dealing with the same
> backend filesystem.
>
> Since this is expected to be used by filesystems that don't set up
> export operations, then that may just be something they have to deal
> with. I'm not sure what else you can use in lieu of i_ino in this case.
>

True. That is one more justification for patch [1/5].

If we only support watching inodes when the reported fid is
{i_ino, i_generation}, then the likelihood of collision drops
considerably compared to watching sb/mount.

Because watcher cannot decode fid, watcher must use
name_to_handle_at() when setting up the inode watch and
store it in some index, so it knows how to map from event->fid
to inode later.

This means that even if there is a fid collision between two inodes,
then the watcher can detect the collision when setting up the
inode watch.

Thanks,
Amir.

Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

[Chuck Lever]

On Wed, Oct 18, 2023 at 12:59:58PM +0300, Amir Goldstein wrote:
> export_operations ->encode_fh() no longer has a default implementation to
> encode FILEID_INO32_GEN* file handles.
> 
> Rename the default helper for encoding FILEID_INO32_GEN* file handles to
> generic_encode_ino32_fh() and convert the filesystems that used the
> default implementation to use the generic helper explicitly.
> 
> This is a step towards allowing filesystems to encode non-decodeable file
> handles for fanotify without having to implement any export_operations.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  Documentation/filesystems/nfs/exporting.rst |  7 ++-----
>  Documentation/filesystems/porting.rst       |  9 +++++++++
>  fs/affs/namei.c                             |  1 +
>  fs/befs/linuxvfs.c                          |  1 +
>  fs/efs/super.c                              |  1 +
>  fs/erofs/super.c                            |  1 +
>  fs/exportfs/expfs.c                         | 14 ++++++++------
>  fs/ext2/super.c                             |  1 +
>  fs/ext4/super.c                             |  1 +
>  fs/f2fs/super.c                             |  1 +
>  fs/fat/nfs.c                                |  1 +
>  fs/jffs2/super.c                            |  1 +
>  fs/jfs/super.c                              |  1 +
>  fs/ntfs/namei.c                             |  1 +
>  fs/ntfs3/super.c                            |  1 +
>  fs/smb/client/export.c                      |  9 +++------
>  fs/squashfs/export.c                        |  1 +
>  fs/ufs/super.c                              |  1 +
>  include/linux/exportfs.h                    |  4 +++-
>  19 files changed, 39 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/filesystems/nfs/exporting.rst b/Documentation/filesystems/nfs/exporting.rst
> index 4b30daee399a..de64d2d002a2 100644
> --- a/Documentation/filesystems/nfs/exporting.rst
> +++ b/Documentation/filesystems/nfs/exporting.rst
> @@ -122,12 +122,9 @@ are exportable by setting the s_export_op field in the struct
>  super_block.  This field must point to a "struct export_operations"
>  struct which has the following members:
>  
> -  encode_fh (optional)
> +  encode_fh (mandatory)
>      Takes a dentry and creates a filehandle fragment which may later be used
> -    to find or create a dentry for the same object.  The default
> -    implementation creates a filehandle fragment that encodes a 32bit inode
> -    and generation number for the inode encoded, and if necessary the
> -    same information for the parent.
> +    to find or create a dentry for the same object.
>  
>    fh_to_dentry (mandatory)
>      Given a filehandle fragment, this should find the implied object and
> diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
> index 4d05b9862451..197ef78a5014 100644
> --- a/Documentation/filesystems/porting.rst
> +++ b/Documentation/filesystems/porting.rst
> @@ -1045,3 +1045,12 @@ filesystem type is now moved to a later point when the devices are closed:
>  As this is a VFS level change it has no practical consequences for filesystems
>  other than that all of them must use one of the provided kill_litter_super(),
>  kill_anon_super(), or kill_block_super() helpers.
> +
> +---
> +
> +**mandatory**
> +
> +export_operations ->encode_fh() no longer has a default implementation to
> +encode FILEID_INO32_GEN* file handles.
> +Fillesystems that used the default implementation may use the generic helper
> +generic_encode_ino32_fh() explicitly.
> diff --git a/fs/affs/namei.c b/fs/affs/namei.c
> index 2fe4a5832fcf..d6b9758ee23d 100644
> --- a/fs/affs/namei.c
> +++ b/fs/affs/namei.c
> @@ -568,6 +568,7 @@ static struct dentry *affs_fh_to_parent(struct super_block *sb, struct fid *fid,
>  }
>  
>  const struct export_operations affs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = affs_fh_to_dentry,
>  	.fh_to_parent = affs_fh_to_parent,
>  	.get_parent = affs_get_parent,
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index 9a16a51fbb88..410dcaffd5ab 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -96,6 +96,7 @@ static const struct address_space_operations befs_symlink_aops = {
>  };
>  
>  static const struct export_operations befs_export_operations = {
> +	.encode_fh	= generic_encode_ino32_fh,
>  	.fh_to_dentry	= befs_fh_to_dentry,
>  	.fh_to_parent	= befs_fh_to_parent,
>  	.get_parent	= befs_get_parent,
> diff --git a/fs/efs/super.c b/fs/efs/super.c
> index b287f47c165b..f17fdac76b2e 100644
> --- a/fs/efs/super.c
> +++ b/fs/efs/super.c
> @@ -123,6 +123,7 @@ static const struct super_operations efs_superblock_operations = {
>  };
>  
>  static const struct export_operations efs_export_ops = {
> +	.encode_fh	= generic_encode_ino32_fh,
>  	.fh_to_dentry	= efs_fh_to_dentry,
>  	.fh_to_parent	= efs_fh_to_parent,
>  	.get_parent	= efs_get_parent,
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 3700af9ee173..edbe07a24156 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -626,6 +626,7 @@ static struct dentry *erofs_get_parent(struct dentry *child)
>  }
>  
>  static const struct export_operations erofs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = erofs_fh_to_dentry,
>  	.fh_to_parent = erofs_fh_to_parent,
>  	.get_parent = erofs_get_parent,
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 9ee205df8fa7..30da4539e257 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -343,20 +343,21 @@ static int get_name(const struct path *path, char *name, struct dentry *child)
>  }
>  
>  /**
> - * export_encode_fh - default export_operations->encode_fh function
> + * generic_encode_ino32_fh - generic export_operations->encode_fh function
>   * @inode:   the object to encode
> - * @fid:     where to store the file handle fragment
> + * @fh:      where to store the file handle fragment
>   * @max_len: maximum length to store there
>   * @parent:  parent directory inode, if wanted
>   *
> - * This default encode_fh function assumes that the 32 inode number
> + * This generic encode_fh function assumes that the 32 inode number
>   * is suitable for locating an inode, and that the generation number
>   * can be used to check that it is still valid.  It places them in the
>   * filehandle fragment where export_decode_fh expects to find them.
>   */
> -static int export_encode_fh(struct inode *inode, struct fid *fid,
> -		int *max_len, struct inode *parent)
> +int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> +			    struct inode *parent)
>  {
> +	struct fid *fid = (void *)fh;
>  	int len = *max_len;
>  	int type = FILEID_INO32_GEN;
>  
> @@ -380,6 +381,7 @@ static int export_encode_fh(struct inode *inode, struct fid *fid,
>  	*max_len = len;
>  	return type;
>  }
> +EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
>  
>  /**
>   * exportfs_encode_inode_fh - encode a file handle from inode
> @@ -402,7 +404,7 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  	if (nop && nop->encode_fh)
>  		return nop->encode_fh(inode, fid->raw, max_len, parent);
>  
> -	return export_encode_fh(inode, fid, max_len, parent);
> +	return -EOPNOTSUPP;
>  }
>  EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
>  
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index aaf3e3e88cb2..b9f158a34997 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -397,6 +397,7 @@ static struct dentry *ext2_fh_to_parent(struct super_block *sb, struct fid *fid,
>  }
>  
>  static const struct export_operations ext2_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = ext2_fh_to_dentry,
>  	.fh_to_parent = ext2_fh_to_parent,
>  	.get_parent = ext2_get_parent,
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index dbebd8b3127e..c44db1915437 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1646,6 +1646,7 @@ static const struct super_operations ext4_sops = {
>  };
>  
>  static const struct export_operations ext4_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = ext4_fh_to_dentry,
>  	.fh_to_parent = ext4_fh_to_parent,
>  	.get_parent = ext4_get_parent,
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index a8c8232852bb..60cfa11f65bf 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3282,6 +3282,7 @@ static struct dentry *f2fs_fh_to_parent(struct super_block *sb, struct fid *fid,
>  }
>  
>  static const struct export_operations f2fs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = f2fs_fh_to_dentry,
>  	.fh_to_parent = f2fs_fh_to_parent,
>  	.get_parent = f2fs_get_parent,
> diff --git a/fs/fat/nfs.c b/fs/fat/nfs.c
> index 3626eb585a98..c52e63e10d35 100644
> --- a/fs/fat/nfs.c
> +++ b/fs/fat/nfs.c
> @@ -279,6 +279,7 @@ static struct dentry *fat_get_parent(struct dentry *child_dir)
>  }
>  
>  const struct export_operations fat_export_ops = {
> +	.encode_fh	= generic_encode_ino32_fh,
>  	.fh_to_dentry   = fat_fh_to_dentry,
>  	.fh_to_parent   = fat_fh_to_parent,
>  	.get_parent     = fat_get_parent,
> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> index 7ea37f49f1e1..f99591a634b4 100644
> --- a/fs/jffs2/super.c
> +++ b/fs/jffs2/super.c
> @@ -150,6 +150,7 @@ static struct dentry *jffs2_get_parent(struct dentry *child)
>  }
>  
>  static const struct export_operations jffs2_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.get_parent = jffs2_get_parent,
>  	.fh_to_dentry = jffs2_fh_to_dentry,
>  	.fh_to_parent = jffs2_fh_to_parent,
> diff --git a/fs/jfs/super.c b/fs/jfs/super.c
> index 2e2f7f6d36a0..2cc2632f3c47 100644
> --- a/fs/jfs/super.c
> +++ b/fs/jfs/super.c
> @@ -896,6 +896,7 @@ static const struct super_operations jfs_super_operations = {
>  };
>  
>  static const struct export_operations jfs_export_operations = {
> +	.encode_fh	= generic_encode_ino32_fh,
>  	.fh_to_dentry	= jfs_fh_to_dentry,
>  	.fh_to_parent	= jfs_fh_to_parent,
>  	.get_parent	= jfs_get_parent,
> diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
> index ab44f2db533b..d7498ddc4a72 100644
> --- a/fs/ntfs/namei.c
> +++ b/fs/ntfs/namei.c
> @@ -384,6 +384,7 @@ static struct dentry *ntfs_fh_to_parent(struct super_block *sb, struct fid *fid,
>   * and due to using iget() whereas NTFS needs ntfs_iget().
>   */
>  const struct export_operations ntfs_export_ops = {
> +	.encode_fh	= generic_encode_ino32_fh,
>  	.get_parent	= ntfs_get_parent,	/* Find the parent of a given
>  						   directory. */
>  	.fh_to_dentry	= ntfs_fh_to_dentry,
> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index 5661a363005e..661ffb5aa1e0 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -789,6 +789,7 @@ static int ntfs_nfs_commit_metadata(struct inode *inode)
>  }
>  
>  static const struct export_operations ntfs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = ntfs_fh_to_dentry,
>  	.fh_to_parent = ntfs_fh_to_parent,
>  	.get_parent = ntfs3_get_parent,
> diff --git a/fs/smb/client/export.c b/fs/smb/client/export.c
> index 37c28415df1e..834e9c9197b4 100644
> --- a/fs/smb/client/export.c
> +++ b/fs/smb/client/export.c
> @@ -41,13 +41,10 @@ static struct dentry *cifs_get_parent(struct dentry *dentry)
>  }
>  
>  const struct export_operations cifs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.get_parent = cifs_get_parent,
> -/*	Following five export operations are unneeded so far and can default:
> -	.get_dentry =
> -	.get_name =
> -	.find_exported_dentry =
> -	.decode_fh =
> -	.encode_fs =  */
> +/*	Following export operations are mandatory for NFS export support:
> +	.fh_to_dentry = */
>  };
>  
>  #endif /* CONFIG_CIFS_NFSD_EXPORT */
> diff --git a/fs/squashfs/export.c b/fs/squashfs/export.c
> index 723763746238..62972f0ff868 100644
> --- a/fs/squashfs/export.c
> +++ b/fs/squashfs/export.c
> @@ -173,6 +173,7 @@ __le64 *squashfs_read_inode_lookup_table(struct super_block *sb,
>  
>  
>  const struct export_operations squashfs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry = squashfs_fh_to_dentry,
>  	.fh_to_parent = squashfs_fh_to_parent,
>  	.get_parent = squashfs_get_parent
> diff --git a/fs/ufs/super.c b/fs/ufs/super.c
> index 23377c1baed9..a480810cd4e3 100644
> --- a/fs/ufs/super.c
> +++ b/fs/ufs/super.c
> @@ -137,6 +137,7 @@ static struct dentry *ufs_get_parent(struct dentry *child)
>  }
>  
>  static const struct export_operations ufs_export_ops = {
> +	.encode_fh = generic_encode_ino32_fh,
>  	.fh_to_dentry	= ufs_fh_to_dentry,
>  	.fh_to_parent	= ufs_fh_to_parent,
>  	.get_parent	= ufs_get_parent,
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 5b3c9f30b422..6b6e01321405 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -235,7 +235,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>  
>  static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
>  {
> -	return nop;
> +	return nop && nop->encode_fh;

The ->encode_fh() method returns an integer type, not a boolean. It
would be more clear if this were written

	return nop && (nop->encode_fh != FILEID_ROOT);

(I'm just guessing at what you might have intended).


>  }
>  
>  static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> @@ -279,6 +279,8 @@ extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
>  /*
>   * Generic helpers for filesystems.
>   */
> +int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> +			    struct inode *parent);
>  extern struct dentry *generic_fh_to_dentry(struct super_block *sb,
>  	struct fid *fid, int fh_len, int fh_type,
>  	struct inode *(*get_inode) (struct super_block *sb, u64 ino, u32 gen));
> -- 
> 2.34.1
> 

-- 
Chuck Lever

Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

[Amir Goldstein]

On Wed, Oct 18, 2023 at 5:53 PM Dave Kleikamp <dave.kleikamp@oracle.com> wrote:
>
> On 10/18/23 9:16AM, Jeff Layton wrote:
> > On Wed, 2023-10-18 at 12:59 +0300, Amir Goldstein wrote:
> >> export_operations ->encode_fh() no longer has a default implementation to
> >> encode FILEID_INO32_GEN* file handles.
> >>
> >> Rename the default helper for encoding FILEID_INO32_GEN* file handles to
> >> generic_encode_ino32_fh() and convert the filesystems that used the
> >> default implementation to use the generic helper explicitly.
>
> Isn't it possible for some of these filesystems to be compiled without
> CONFIG_EXPORTFS set? Should exportfs.h define an null
> generic_encode_ino32_fh() in that case?
>

Yes, good idea!

Thanks,
Amir.

Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

[Amir Goldstein]

On Wed, Oct 18, 2023 at 6:18 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On Wed, Oct 18, 2023 at 12:59:58PM +0300, Amir Goldstein wrote:
> > export_operations ->encode_fh() no longer has a default implementation to
> > encode FILEID_INO32_GEN* file handles.
> >
> > Rename the default helper for encoding FILEID_INO32_GEN* file handles to
> > generic_encode_ino32_fh() and convert the filesystems that used the
> > default implementation to use the generic helper explicitly.
> >
> > This is a step towards allowing filesystems to encode non-decodeable file
> > handles for fanotify without having to implement any export_operations.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
[...]

> > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> > index 5b3c9f30b422..6b6e01321405 100644
> > --- a/include/linux/exportfs.h
> > +++ b/include/linux/exportfs.h
> > @@ -235,7 +235,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
> >
> >  static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
> >  {
> > -     return nop;
> > +     return nop && nop->encode_fh;
>
> The ->encode_fh() method returns an integer type, not a boolean. It
> would be more clear if this were written
>
>         return nop && (nop->encode_fh != FILEID_ROOT);
>
> (I'm just guessing at what you might have intended).
>

You must be pre-coffee ;)

This checks if the method exists, it doesn't invoke the method.

Thanks,
Amir.

Re: [PATCH 5/5] exportfs: support encoding non-decodeable file handles by default

[Chuck Lever]

On Wed, Oct 18, 2023 at 01:00:00PM +0300, Amir Goldstein wrote:
> AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> the encoding of a file id, which is not intended to be decoded.
> 
> This file id is used by fanotify to describe objects in events.
> 
> So far, overlayfs is the only filesystem that supports encoding
> non-decodeable file ids, by providing export_operations with an
> ->encode_fh() method and without a ->decode_fh() method.
> 
> Add support for encoding non-decodeable file ids to all the filesystems
> that do not provide export_operations, by encoding a file id of type
> FILEID_INO64_GEN from { i_ino, i_generation }.
> 
> A filesystem may that does not support NFS export, can opt-out of
> encoding non-decodeable file ids for fanotify by defining an empty
> export_operations struct (i.e. with a NULL ->encode_fh() method).
> 
> This allows the use of fanotify events with file ids on filesystems
> like 9p which do not support NFS export to bring fanotify in feature
> parity with inotify on those filesystems.
> 
> Note that fanotify also requires that the filesystems report a non-null
> fsid.  Currently, many simple filesystems that have support for inotify
> (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> used with fanotify in file id reporting mode.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
>  include/linux/exportfs.h | 10 +++++++---
>  2 files changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 30da4539e257..34e7d835d4ef 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
>  }
>  EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
>  
> +/**
> + * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
> + * @inode:   the object to encode
> + * @fid:     where to store the file handle fragment
> + * @max_len: maximum length to store there

Length in what units? Is the 3 below in units of bytes or
sizeof(__be32) ? I'm guessing it's the latter; if so, it should
be mentioned here. (We have XDR_UNIT for this purpose, btw).

export_encode_fh() isn't exactly clear about that either, sadly.


> + *
> + * This generic function is used to encode a non-decodeable file id for
> + * fanotify for filesystems that do not support NFS export.
> + */
> +static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
> +				     int *max_len)
> +{
> +	if (*max_len < 3) {
> +		*max_len = 3;

Let's make this a symbolic constant rather than a naked integer.


> +		return FILEID_INVALID;
> +	}
> +
> +	fid->i64.ino = inode->i_ino;
> +	fid->i64.gen = inode->i_generation;
> +	*max_len = 3;
> +
> +	return FILEID_INO64_GEN;
> +}
> +
>  /**
>   * exportfs_encode_inode_fh - encode a file handle from inode
>   * @inode:   the object to encode
> @@ -401,10 +425,10 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  	if (!exportfs_can_encode_fh(nop, flags))
>  		return -EOPNOTSUPP;
>  
> -	if (nop && nop->encode_fh)
> -		return nop->encode_fh(inode, fid->raw, max_len, parent);
> +	if (!nop && (flags & EXPORT_FH_FID))
> +		return exportfs_encode_ino64_fid(inode, fid, max_len);
>  
> -	return -EOPNOTSUPP;
> +	return nop->encode_fh(inode, fid->raw, max_len, parent);
>  }
>  EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
>  
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 21eeb9f6bdbd..6688e457da64 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -134,7 +134,11 @@ struct fid {
>  			u32 parent_ino;
>  			u32 parent_gen;
>  		} i32;
> - 		struct {
> +		struct {
> +			u64 ino;
> +			u32 gen;
> +		} __packed i64;
> +		struct {
>   			u32 block;
>   			u16 partref;
>   			u16 parent_partref;
> @@ -246,7 +250,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>  
>  static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
>  {
> -	return nop && nop->encode_fh;
> +	return !nop || nop->encode_fh;
>  }
>  
>  static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> @@ -259,7 +263,7 @@ static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
>  {
>  	/*
>  	 * If a non-decodeable file handle was requested, we only need to make
> -	 * sure that filesystem can encode file handles.
> +	 * sure that filesystem did not opt-out of encoding fid.
>  	 */
>  	if (fh_flags & EXPORT_FH_FID)
>  		return exportfs_can_encode_fid(nop);
> -- 
> 2.34.1
> 
> 

-- 
Chuck Lever

Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

[Chuck Lever]

On Wed, Oct 18, 2023 at 06:26:07PM +0300, Amir Goldstein wrote:
> On Wed, Oct 18, 2023 at 6:18 PM Chuck Lever <chuck.lever@oracle.com> wrote:
> >
> > On Wed, Oct 18, 2023 at 12:59:58PM +0300, Amir Goldstein wrote:
> > > export_operations ->encode_fh() no longer has a default implementation to
> > > encode FILEID_INO32_GEN* file handles.
> > >
> > > Rename the default helper for encoding FILEID_INO32_GEN* file handles to
> > > generic_encode_ino32_fh() and convert the filesystems that used the
> > > default implementation to use the generic helper explicitly.
> > >
> > > This is a step towards allowing filesystems to encode non-decodeable file
> > > handles for fanotify without having to implement any export_operations.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> [...]
> 
> > > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> > > index 5b3c9f30b422..6b6e01321405 100644
> > > --- a/include/linux/exportfs.h
> > > +++ b/include/linux/exportfs.h
> > > @@ -235,7 +235,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
> > >
> > >  static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
> > >  {
> > > -     return nop;
> > > +     return nop && nop->encode_fh;
> >
> > The ->encode_fh() method returns an integer type, not a boolean. It
> > would be more clear if this were written
> >
> >         return nop && (nop->encode_fh != FILEID_ROOT);
> >
> > (I'm just guessing at what you might have intended).
> >
> 
> You must be pre-coffee ;)

More like pre-lunch.


> This checks if the method exists, it doesn't invoke the method.

OK, I was confused because I thought you were filling in all the
spots where the method doesn't already exist. My brain lept to
the conclusion that this is therefore calling the method, but I
failed to notice there are no call arguments.

I might understand it now. :-)

Since you want this series to go through the vfs tree:

Acked-by: Chuck Lever <chuck.lever@oracle.com>


-- 
Chuck Lever

Re: [PATCH 5/5] exportfs: support encoding non-decodeable file handles by default

[Amir Goldstein]

On Wed, Oct 18, 2023 at 6:28 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On Wed, Oct 18, 2023 at 01:00:00PM +0300, Amir Goldstein wrote:
> > AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> > the encoding of a file id, which is not intended to be decoded.
> >
> > This file id is used by fanotify to describe objects in events.
> >
> > So far, overlayfs is the only filesystem that supports encoding
> > non-decodeable file ids, by providing export_operations with an
> > ->encode_fh() method and without a ->decode_fh() method.
> >
> > Add support for encoding non-decodeable file ids to all the filesystems
> > that do not provide export_operations, by encoding a file id of type
> > FILEID_INO64_GEN from { i_ino, i_generation }.
> >
> > A filesystem may that does not support NFS export, can opt-out of
> > encoding non-decodeable file ids for fanotify by defining an empty
> > export_operations struct (i.e. with a NULL ->encode_fh() method).
> >
> > This allows the use of fanotify events with file ids on filesystems
> > like 9p which do not support NFS export to bring fanotify in feature
> > parity with inotify on those filesystems.
> >
> > Note that fanotify also requires that the filesystems report a non-null
> > fsid.  Currently, many simple filesystems that have support for inotify
> > (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> > used with fanotify in file id reporting mode.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
> >  include/linux/exportfs.h | 10 +++++++---
> >  2 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> > index 30da4539e257..34e7d835d4ef 100644
> > --- a/fs/exportfs/expfs.c
> > +++ b/fs/exportfs/expfs.c
> > @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
> >  }
> >  EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
> >
> > +/**
> > + * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
> > + * @inode:   the object to encode
> > + * @fid:     where to store the file handle fragment
> > + * @max_len: maximum length to store there
>
> Length in what units? Is the 3 below in units of bytes or
> sizeof(__be32) ? I'm guessing it's the latter; if so, it should
> be mentioned here. (We have XDR_UNIT for this purpose, btw).
>
> export_encode_fh() isn't exactly clear about that either, sadly.
>
>

Yeh, it's the same all over the place including in filesystem
implementations.

> > + *
> > + * This generic function is used to encode a non-decodeable file id for
> > + * fanotify for filesystems that do not support NFS export.
> > + */
> > +static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
> > +                                  int *max_len)
> > +{
> > +     if (*max_len < 3) {
> > +             *max_len = 3;
>
> Let's make this a symbolic constant rather than a naked integer.
>

Sure, no problem.

Thanks for the review.
Amir.

Re: [PATCH 1/5] fanotify: limit reporting of event with non-decodeable file handles

[Jan Kara]

On Wed 18-10-23 12:59:56, Amir Goldstein wrote:
> Commit a95aef69a740 ("fanotify: support reporting non-decodeable file
> handles") merged in v6.5-rc1, added the ability to use an fanotify group
> with FAN_REPORT_FID mode to watch filesystems that do not support nfs
> export, but do know how to encode non-decodeable file handles, with the
> newly introduced AT_HANDLE_FID flag.
> 
> At the time that this commit was merged, there were no filesystems
> in-tree with those traits.
> 
> Commit 16aac5ad1fa9 ("ovl: support encoding non-decodable file handles"),
> merged in v6.6-rc1, added this trait to overlayfs, thus allowing fanotify
> watching of overlayfs with FAN_REPORT_FID mode.
> 
> In retrospect, allowing an fanotify filesystem/mount mark on such
> filesystem in FAN_REPORT_FID mode will result in getting events with
> file handles, without the ability to resolve the filesystem objects from
> those file handles (i.e. no open_by_handle_at() support).
> 
> For v6.6, the safer option would be to allow this mode for inode marks
> only, where the caller has the opportunity to use name_to_handle_at() at
> the time of setting the mark. In the future we can revise this decision.
> 
> Fixes: a95aef69a740 ("fanotify: support reporting non-decodeable file handles")
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

OK, I agree sb/mount marks reporting FIDs are hardly usable without
name_to_handle_at() so better forbid them before someone comes up with some
creative abuse. I've queued the patch into my tree.

								Honza

> ---
>  fs/notify/fanotify/fanotify_user.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index f69c451018e3..537c70beaad0 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1585,16 +1585,25 @@ static int fanotify_test_fsid(struct dentry *dentry, __kernel_fsid_t *fsid)
>  }
>  
>  /* Check if filesystem can encode a unique fid */
> -static int fanotify_test_fid(struct dentry *dentry)
> +static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
>  {
> +	unsigned int mark_type = flags & FANOTIFY_MARK_TYPE_BITS;
> +	const struct export_operations *nop = dentry->d_sb->s_export_op;
> +
> +	/*
> +	 * We need to make sure that the filesystem supports encoding of
> +	 * file handles so user can use name_to_handle_at() to compare fids
> +	 * reported with events to the file handle of watched objects.
> +	 */
> +	if (!nop)
> +		return -EOPNOTSUPP;
> +
>  	/*
> -	 * We need to make sure that the file system supports at least
> -	 * encoding a file handle so user can use name_to_handle_at() to
> -	 * compare fid returned with event to the file handle of watched
> -	 * objects. However, even the relaxed AT_HANDLE_FID flag requires
> -	 * at least empty export_operations for ecoding unique file ids.
> +	 * For sb/mount mark, we also need to make sure that the filesystem
> +	 * supports decoding file handles, so user has a way to map back the
> +	 * reported fids to filesystem objects.
>  	 */
> -	if (!dentry->d_sb->s_export_op)
> +	if (mark_type != FAN_MARK_INODE && !nop->fh_to_dentry)
>  		return -EOPNOTSUPP;
>  
>  	return 0;
> @@ -1812,7 +1821,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>  		if (ret)
>  			goto path_put_and_out;
>  
> -		ret = fanotify_test_fid(path.dentry);
> +		ret = fanotify_test_fid(path.dentry, flags);
>  		if (ret)
>  			goto path_put_and_out;
>  
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/5] exportfs: add helpers to check if filesystem can encode/decode file handles

[Jan Kara]

On Wed 18-10-23 12:59:57, Amir Goldstein wrote:
> The logic of whether filesystem can encode/decode file handles is open
> coded in many places.
> 
> In preparation to changing the logic, move the open coded logic into
> inline helpers.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Yeah, good cleanup. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/exportfs/expfs.c                |  8 ++------
>  fs/fhandle.c                       |  6 +-----
>  fs/nfsd/export.c                   |  3 +--
>  fs/notify/fanotify/fanotify_user.c |  4 ++--
>  fs/overlayfs/util.c                |  2 +-
>  include/linux/exportfs.h           | 27 +++++++++++++++++++++++++++
>  6 files changed, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index c20704aa21b3..9ee205df8fa7 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -396,11 +396,7 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  {
>  	const struct export_operations *nop = inode->i_sb->s_export_op;
>  
> -	/*
> -	 * If a decodeable file handle was requested, we need to make sure that
> -	 * filesystem can decode file handles.
> -	 */
> -	if (nop && !(flags & EXPORT_FH_FID) && !nop->fh_to_dentry)
> +	if (!exportfs_can_encode_fh(nop, flags))
>  		return -EOPNOTSUPP;
>  
>  	if (nop && nop->encode_fh)
> @@ -456,7 +452,7 @@ exportfs_decode_fh_raw(struct vfsmount *mnt, struct fid *fid, int fh_len,
>  	/*
>  	 * Try to get any dentry for the given file handle from the filesystem.
>  	 */
> -	if (!nop || !nop->fh_to_dentry)
> +	if (!exportfs_can_decode_fh(nop))
>  		return ERR_PTR(-ESTALE);
>  	result = nop->fh_to_dentry(mnt->mnt_sb, fid, fh_len, fileid_type);
>  	if (IS_ERR_OR_NULL(result))
> diff --git a/fs/fhandle.c b/fs/fhandle.c
> index 6ea8d35a9382..18b3ba8dc8ea 100644
> --- a/fs/fhandle.c
> +++ b/fs/fhandle.c
> @@ -26,12 +26,8 @@ static long do_sys_name_to_handle(const struct path *path,
>  	/*
>  	 * We need to make sure whether the file system support decoding of
>  	 * the file handle if decodeable file handle was requested.
> -	 * Otherwise, even empty export_operations are sufficient to opt-in
> -	 * to encoding FIDs.
>  	 */
> -	if (!path->dentry->d_sb->s_export_op ||
> -	    (!(fh_flags & EXPORT_FH_FID) &&
> -	     !path->dentry->d_sb->s_export_op->fh_to_dentry))
> +	if (!exportfs_can_encode_fh(path->dentry->d_sb->s_export_op, fh_flags))
>  		return -EOPNOTSUPP;
>  
>  	if (copy_from_user(&f_handle, ufh, sizeof(struct file_handle)))
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 11a0eaa2f914..dc99dfc1d411 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -421,8 +421,7 @@ static int check_export(struct path *path, int *flags, unsigned char *uuid)
>  		return -EINVAL;
>  	}
>  
> -	if (!inode->i_sb->s_export_op ||
> -	    !inode->i_sb->s_export_op->fh_to_dentry) {
> +	if (!exportfs_can_decode_fh(inode->i_sb->s_export_op)) {
>  		dprintk("exp_export: export of invalid fs type.\n");
>  		return -EINVAL;
>  	}
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 537c70beaad0..ce926eb9feea 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1595,7 +1595,7 @@ static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
>  	 * file handles so user can use name_to_handle_at() to compare fids
>  	 * reported with events to the file handle of watched objects.
>  	 */
> -	if (!nop)
> +	if (!exportfs_can_encode_fid(nop))
>  		return -EOPNOTSUPP;
>  
>  	/*
> @@ -1603,7 +1603,7 @@ static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
>  	 * supports decoding file handles, so user has a way to map back the
>  	 * reported fids to filesystem objects.
>  	 */
> -	if (mark_type != FAN_MARK_INODE && !nop->fh_to_dentry)
> +	if (mark_type != FAN_MARK_INODE && !exportfs_can_decode_fh(nop))
>  		return -EOPNOTSUPP;
>  
>  	return 0;
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 89e0d60d35b6..f0a712214ec2 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -55,7 +55,7 @@ int ovl_can_decode_fh(struct super_block *sb)
>  	if (!capable(CAP_DAC_READ_SEARCH))
>  		return 0;
>  
> -	if (!sb->s_export_op || !sb->s_export_op->fh_to_dentry)
> +	if (!exportfs_can_decode_fh(sb->s_export_op))
>  		return 0;
>  
>  	return sb->s_export_op->encode_fh ? -1 : FILEID_INO32_GEN;
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 11fbd0ee1370..5b3c9f30b422 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -233,6 +233,33 @@ extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>  			      int *max_len, int flags);
>  
> +static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
> +{
> +	return nop;
> +}
> +
> +static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> +{
> +	return nop && nop->fh_to_dentry;
> +}
> +
> +static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
> +					  int fh_flags)
> +{
> +	/*
> +	 * If a non-decodeable file handle was requested, we only need to make
> +	 * sure that filesystem can encode file handles.
> +	 */
> +	if (fh_flags & EXPORT_FH_FID)
> +		return exportfs_can_encode_fid(nop);
> +
> +	/*
> +	 * If a decodeable file handle was requested, we need to make sure that
> +	 * filesystem can also decode file handles.
> +	 */
> +	return exportfs_can_decode_fh(nop);
> +}
> +
>  static inline int exportfs_encode_fid(struct inode *inode, struct fid *fid,
>  				      int *max_len)
>  {
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

[Jan Kara]

On Wed 18-10-23 12:59:58, Amir Goldstein wrote:
> export_operations ->encode_fh() no longer has a default implementation to
> encode FILEID_INO32_GEN* file handles.
> 
> Rename the default helper for encoding FILEID_INO32_GEN* file handles to
> generic_encode_ino32_fh() and convert the filesystems that used the
> default implementation to use the generic helper explicitly.
> 
> This is a step towards allowing filesystems to encode non-decodeable file
> handles for fanotify without having to implement any export_operations.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Just one typo cleanup. Also I agree we need a "nop" variant of
generic_encode_ino32_fh() or move this to fs/libfs.c like e.g.
generic_fh_to_dentry().

> diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
> index 4d05b9862451..197ef78a5014 100644
> --- a/Documentation/filesystems/porting.rst
> +++ b/Documentation/filesystems/porting.rst
> @@ -1045,3 +1045,12 @@ filesystem type is now moved to a later point when the devices are closed:
>  As this is a VFS level change it has no practical consequences for filesystems
>  other than that all of them must use one of the provided kill_litter_super(),
>  kill_anon_super(), or kill_block_super() helpers.
> +
> +---
> +
> +**mandatory**
> +
> +export_operations ->encode_fh() no longer has a default implementation to
> +encode FILEID_INO32_GEN* file handles.
> +Fillesystems that used the default implementation may use the generic helper
   ^^^ Filesystems

> +generic_encode_ino32_fh() explicitly.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 4/5] exportfs: define FILEID_INO64_GEN* file handle types

[Jan Kara]

On Wed 18-10-23 12:59:59, Amir Goldstein wrote:
> Similar to the common FILEID_INO32* file handle types, define common
> FILEID_INO64* file handle types.
> 
> The type values of FILEID_INO64_GEN and FILEID_INO64_GEN_PARENT are the
> values returned by fuse and xfs for 64bit ino encoded file handle types.
> 
> Note that these type value are filesystem specific and they do not define
> a universal file handle format, for example:
> fuse encodes FILEID_INO64_GEN as [ino-hi32,ino-lo32,gen] and xfs encodes
> FILEID_INO64_GEN as [hostr-order-ino64,gen] (a.k.a xfs_fid64).
> 
> The FILEID_INO64_GEN fhandle type is going to be used for file ids for
> fanotify from filesystems that do not support NFS export.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Yeah, better than the plain numbers. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/fuse/inode.c          |  7 ++++---
>  include/linux/exportfs.h | 11 +++++++++++
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 2e4eb7cf26fb..e63f966698a5 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1002,7 +1002,7 @@ static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
>  	}
>  
>  	*max_len = len;
> -	return parent ? 0x82 : 0x81;
> +	return parent ? FILEID_INO64_GEN_PARENT : FILEID_INO64_GEN;
>  }
>  
>  static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
> @@ -1010,7 +1010,8 @@ static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
>  {
>  	struct fuse_inode_handle handle;
>  
> -	if ((fh_type != 0x81 && fh_type != 0x82) || fh_len < 3)
> +	if ((fh_type != FILEID_INO64_GEN &&
> +	     fh_type != FILEID_INO64_GEN_PARENT) || fh_len < 3)
>  		return NULL;
>  
>  	handle.nodeid = (u64) fid->raw[0] << 32;
> @@ -1024,7 +1025,7 @@ static struct dentry *fuse_fh_to_parent(struct super_block *sb,
>  {
>  	struct fuse_inode_handle parent;
>  
> -	if (fh_type != 0x82 || fh_len < 6)
> +	if (fh_type != FILEID_INO64_GEN_PARENT || fh_len < 6)
>  		return NULL;
>  
>  	parent.nodeid = (u64) fid->raw[3] << 32;
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 6b6e01321405..21eeb9f6bdbd 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -98,6 +98,17 @@ enum fid_type {
>  	 */
>  	FILEID_FAT_WITH_PARENT = 0x72,
>  
> +	/*
> +	 * 64 bit inode number, 32 bit generation number.
> +	 */
> +	FILEID_INO64_GEN = 0x81,
> +
> +	/*
> +	 * 64 bit inode number, 32 bit generation number,
> +	 * 64 bit parent inode number, 32 bit parent generation.
> +	 */
> +	FILEID_INO64_GEN_PARENT = 0x82,
> +
>  	/*
>  	 * 128 bit child FID (struct lu_fid)
>  	 * 128 bit parent FID (struct lu_fid)
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

[Amir Goldstein]

On Thu, Oct 19, 2023 at 5:40 PM Jan Kara <jack@suse.cz> wrote:
>
> On Wed 18-10-23 12:59:58, Amir Goldstein wrote:
> > export_operations ->encode_fh() no longer has a default implementation to
> > encode FILEID_INO32_GEN* file handles.
> >
> > Rename the default helper for encoding FILEID_INO32_GEN* file handles to
> > generic_encode_ino32_fh() and convert the filesystems that used the
> > default implementation to use the generic helper explicitly.
> >
> > This is a step towards allowing filesystems to encode non-decodeable file
> > handles for fanotify without having to implement any export_operations.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> Just one typo cleanup. Also I agree we need a "nop" variant of
> generic_encode_ino32_fh() or move this to fs/libfs.c like e.g.
> generic_fh_to_dentry().
>

I did this:

 /*
  * Generic helpers for filesystems.
  */
+#ifdef CONFIG_EXPORTFS
+int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
+                           struct inode *parent);
+#else
+#define generic_encode_ino32_fh NULL
+#endif

I like it better than moving to fs/libfs.c, because if CONFIG_EXPORTFS
is not defined, no code should be calling generic_encode_ino32_fh().

It might be a good idea to define exportfs_can_*() helpers to false
when CONFIG_EXPORTFS is not defined, but at least for fanotify,
this is not relevant because fanotify selects EXPORTFS.

> > diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
> > index 4d05b9862451..197ef78a5014 100644
> > --- a/Documentation/filesystems/porting.rst
> > +++ b/Documentation/filesystems/porting.rst
> > @@ -1045,3 +1045,12 @@ filesystem type is now moved to a later point when the devices are closed:
> >  As this is a VFS level change it has no practical consequences for filesystems
> >  other than that all of them must use one of the provided kill_litter_super(),
> >  kill_anon_super(), or kill_block_super() helpers.
> > +
> > +---
> > +
> > +**mandatory**
> > +
> > +export_operations ->encode_fh() no longer has a default implementation to
> > +encode FILEID_INO32_GEN* file handles.
> > +Fillesystems that used the default implementation may use the generic helper
>    ^^^ Filesystems
>

Thanks!
Amir.

Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

[kernel test robot]

Hi Amir,

kernel test robot noticed the following build errors:

[auto build test ERROR on jack-fs/fsnotify]
[also build test ERROR on trondmy-nfs/linux-next xiang-erofs/dev-test xiang-erofs/dev tytso-ext4/dev kleikamp-shaggy/jfs-next cifs/for-next v6.6-rc6]
[cannot apply to xiang-erofs/fixes jaegeuk-f2fs/dev-test jaegeuk-f2fs/dev mszeredi-fuse/for-next linus/master next-20231020]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Amir-Goldstein/fanotify-limit-reporting-of-event-with-non-decodeable-file-handles/20231018-180124
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
patch link:    https://lore.kernel.org/r/20231018100000.2453965-4-amir73il%40gmail.com
patch subject: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export
config: parisc-randconfig-002-20231021 (https://download.01.org/0day-ci/archive/20231022/202310220009.dfnFPdDB-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231022/202310220009.dfnFPdDB-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310220009.dfnFPdDB-lkp@intel.com/

All errors (new ones prefixed by >>):

   hppa-linux-ld: fs/ext4/super.o: in function `.L4276':
>> (.rodata+0xc9c): undefined reference to `generic_encode_ino32_fh'
   hppa-linux-ld: fs/erofs/super.o: in function `.L333':
   (.rodata+0x224): undefined reference to `generic_encode_ino32_fh'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export

[kernel test robot]

Hi Amir,

kernel test robot noticed the following build errors:

[auto build test ERROR on jack-fs/fsnotify]
[also build test ERROR on trondmy-nfs/linux-next xiang-erofs/dev-test xiang-erofs/dev tytso-ext4/dev kleikamp-shaggy/jfs-next cifs/for-next v6.6-rc6]
[cannot apply to xiang-erofs/fixes jaegeuk-f2fs/dev-test jaegeuk-f2fs/dev mszeredi-fuse/for-next linus/master next-20231020]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Amir-Goldstein/fanotify-limit-reporting-of-event-with-non-decodeable-file-handles/20231018-180124
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
patch link:    https://lore.kernel.org/r/20231018100000.2453965-4-amir73il%40gmail.com
patch subject: [PATCH 3/5] exportfs: make ->encode_fh() a mandatory method for NFS export
config: powerpc-randconfig-002-20231022 (https://download.01.org/0day-ci/archive/20231022/202310221030.mHFGcTOT-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231022/202310221030.mHFGcTOT-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310221030.mHFGcTOT-lkp@intel.com/

All errors (new ones prefixed by >>):

   powerpc-linux-ld: warning: orphan section `.init.plt' from `drivers/mfd/rsmu_core.o' being placed in section `.init.plt'
>> powerpc-linux-ld: fs/ext4/super.o:(.rodata+0x1314): undefined reference to `generic_encode_ino32_fh'
>> powerpc-linux-ld: fs/squashfs/export.o:(.rodata+0x54): undefined reference to `generic_encode_ino32_fh'
>> powerpc-linux-ld: fs/fat/nfs.o:(.rodata+0x80): undefined reference to `generic_encode_ino32_fh'
>> powerpc-linux-ld: fs/ufs/super.o:(.rodata+0x224): undefined reference to `generic_encode_ino32_fh'
>> powerpc-linux-ld: fs/affs/namei.o:(.rodata+0x120): undefined reference to `generic_encode_ino32_fh'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 5/5] exportfs: support encoding non-decodeable file handles by default

[Amir Goldstein]

On Wed, Oct 18, 2023 at 1:00 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> the encoding of a file id, which is not intended to be decoded.
>
> This file id is used by fanotify to describe objects in events.
>
> So far, overlayfs is the only filesystem that supports encoding
> non-decodeable file ids, by providing export_operations with an
> ->encode_fh() method and without a ->decode_fh() method.
>
> Add support for encoding non-decodeable file ids to all the filesystems
> that do not provide export_operations, by encoding a file id of type
> FILEID_INO64_GEN from { i_ino, i_generation }.
>
> A filesystem may that does not support NFS export, can opt-out of
> encoding non-decodeable file ids for fanotify by defining an empty
> export_operations struct (i.e. with a NULL ->encode_fh() method).
>
> This allows the use of fanotify events with file ids on filesystems
> like 9p which do not support NFS export to bring fanotify in feature
> parity with inotify on those filesystems.
>
> Note that fanotify also requires that the filesystems report a non-null
> fsid.  Currently, many simple filesystems that have support for inotify
> (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> used with fanotify in file id reporting mode.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---

Hi Jan,

Did you get a chance to look at this patch?
I saw your review comments on the rest of the series, so was waiting
for feedback on this last one before posting v2.

BTW, I am going to post a complementary patch to add fsid support for
the simple filesystems.

Thanks,
Amir.

>  fs/exportfs/expfs.c      | 30 +++++++++++++++++++++++++++---
>  include/linux/exportfs.h | 10 +++++++---
>  2 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 30da4539e257..34e7d835d4ef 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -383,6 +383,30 @@ int generic_encode_ino32_fh(struct inode *inode, __u32 *fh, int *max_len,
>  }
>  EXPORT_SYMBOL_GPL(generic_encode_ino32_fh);
>
> +/**
> + * exportfs_encode_ino64_fid - encode non-decodeable 64bit ino file id
> + * @inode:   the object to encode
> + * @fid:     where to store the file handle fragment
> + * @max_len: maximum length to store there
> + *
> + * This generic function is used to encode a non-decodeable file id for
> + * fanotify for filesystems that do not support NFS export.
> + */
> +static int exportfs_encode_ino64_fid(struct inode *inode, struct fid *fid,
> +                                    int *max_len)
> +{
> +       if (*max_len < 3) {
> +               *max_len = 3;
> +               return FILEID_INVALID;
> +       }
> +
> +       fid->i64.ino = inode->i_ino;
> +       fid->i64.gen = inode->i_generation;
> +       *max_len = 3;
> +
> +       return FILEID_INO64_GEN;
> +}
> +
>  /**
>   * exportfs_encode_inode_fh - encode a file handle from inode
>   * @inode:   the object to encode
> @@ -401,10 +425,10 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>         if (!exportfs_can_encode_fh(nop, flags))
>                 return -EOPNOTSUPP;
>
> -       if (nop && nop->encode_fh)
> -               return nop->encode_fh(inode, fid->raw, max_len, parent);
> +       if (!nop && (flags & EXPORT_FH_FID))
> +               return exportfs_encode_ino64_fid(inode, fid, max_len);
>
> -       return -EOPNOTSUPP;
> +       return nop->encode_fh(inode, fid->raw, max_len, parent);
>  }
>  EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
>
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 21eeb9f6bdbd..6688e457da64 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -134,7 +134,11 @@ struct fid {
>                         u32 parent_ino;
>                         u32 parent_gen;
>                 } i32;
> -               struct {
> +               struct {
> +                       u64 ino;
> +                       u32 gen;
> +               } __packed i64;
> +               struct {
>                         u32 block;
>                         u16 partref;
>                         u16 parent_partref;
> @@ -246,7 +250,7 @@ extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
>
>  static inline bool exportfs_can_encode_fid(const struct export_operations *nop)
>  {
> -       return nop && nop->encode_fh;
> +       return !nop || nop->encode_fh;
>  }
>
>  static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
> @@ -259,7 +263,7 @@ static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
>  {
>         /*
>          * If a non-decodeable file handle was requested, we only need to make
> -        * sure that filesystem can encode file handles.
> +        * sure that filesystem did not opt-out of encoding fid.
>          */
>         if (fh_flags & EXPORT_FH_FID)
>                 return exportfs_can_encode_fid(nop);
> --
> 2.34.1
>

Re: [PATCH 5/5] exportfs: support encoding non-decodeable file handles by default

[Jan Kara]

On Mon 23-10-23 16:55:40, Amir Goldstein wrote:
> On Wed, Oct 18, 2023 at 1:00 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> > the encoding of a file id, which is not intended to be decoded.
> >
> > This file id is used by fanotify to describe objects in events.
> >
> > So far, overlayfs is the only filesystem that supports encoding
> > non-decodeable file ids, by providing export_operations with an
> > ->encode_fh() method and without a ->decode_fh() method.
> >
> > Add support for encoding non-decodeable file ids to all the filesystems
> > that do not provide export_operations, by encoding a file id of type
> > FILEID_INO64_GEN from { i_ino, i_generation }.
> >
> > A filesystem may that does not support NFS export, can opt-out of
> > encoding non-decodeable file ids for fanotify by defining an empty
> > export_operations struct (i.e. with a NULL ->encode_fh() method).
> >
> > This allows the use of fanotify events with file ids on filesystems
> > like 9p which do not support NFS export to bring fanotify in feature
> > parity with inotify on those filesystems.
> >
> > Note that fanotify also requires that the filesystems report a non-null
> > fsid.  Currently, many simple filesystems that have support for inotify
> > (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> > used with fanotify in file id reporting mode.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> 
> Hi Jan,
> 
> Did you get a chance to look at this patch?
> I saw your review comments on the rest of the series, so was waiting
> for feedback on this last one before posting v2.

Ah, sorry. I don't have any further comments regarding this patch besides
what Chuck already wrote.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 5/5] exportfs: support encoding non-decodeable file handles by default

[Amir Goldstein]

On Mon, Oct 23, 2023 at 7:33 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 23-10-23 16:55:40, Amir Goldstein wrote:
> > On Wed, Oct 18, 2023 at 1:00 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > AT_HANDLE_FID was added as an API for name_to_handle_at() that request
> > > the encoding of a file id, which is not intended to be decoded.
> > >
> > > This file id is used by fanotify to describe objects in events.
> > >
> > > So far, overlayfs is the only filesystem that supports encoding
> > > non-decodeable file ids, by providing export_operations with an
> > > ->encode_fh() method and without a ->decode_fh() method.
> > >
> > > Add support for encoding non-decodeable file ids to all the filesystems
> > > that do not provide export_operations, by encoding a file id of type
> > > FILEID_INO64_GEN from { i_ino, i_generation }.
> > >
> > > A filesystem may that does not support NFS export, can opt-out of
> > > encoding non-decodeable file ids for fanotify by defining an empty
> > > export_operations struct (i.e. with a NULL ->encode_fh() method).
> > >
> > > This allows the use of fanotify events with file ids on filesystems
> > > like 9p which do not support NFS export to bring fanotify in feature
> > > parity with inotify on those filesystems.
> > >
> > > Note that fanotify also requires that the filesystems report a non-null
> > > fsid.  Currently, many simple filesystems that have support for inotify
> > > (e.g. debugfs, tracefs, sysfs) report a null fsid, so can still not be
> > > used with fanotify in file id reporting mode.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> >
> > Hi Jan,
> >
> > Did you get a chance to look at this patch?
> > I saw your review comments on the rest of the series, so was waiting
> > for feedback on this last one before posting v2.
>
> Ah, sorry. I don't have any further comments regarding this patch besides
> what Chuck already wrote.

No worries.
I will post v2 with minor fixes and add your RVB to all patches.

Thanks,
Amir.