Skip to contentSkip to conversation

nfsd and special kernel filesystems

PATCH Applied Amir Goldstein 3 participants 5 messages Jan 29

Subsystems (4)
lore
Share & tools
lore

Amir Goldstein

author reviewer
#0 lore raw

v4 cover nfsd and special kernel filesystems

Christian,

This v4 addresses Chuck and Jeff's review on v3.

The first doc patch is applicable to the doc update in vfs-7.0.misc. The 2nd fix patch is independent of the doc changes in vfs-7.0.misc, so it should be easier to backport.

Thanks,
Amir.

Changes since v3:
- Fix typo and doc comments from Chunk
- Add RVB

Changes since v2:
- Rebase over vfs-7.0.misc
- Split to doc/fix patches
- Remove RVBs

Amir Goldstein (2):
exportfs: clarify the documentation of open()/permission() expotrfs
ops
nfsd: do not allow exporting of special kernel filesystems

 fs/nfsd/export.c         |  8 +++++---
 include/linux/exportfs.h | 21 +++++++++++++++++++--
 2 files changed, 24 insertions(+), 5 deletions(-)
Signature · 1 line
2.52.0
author reviewer
#1 lore raw

PATCH v4 1/2 exportfs: clarify the documentation of open()/permission() expotrfs ops

pidfs and nsfs recently gained support for encode/decode of file handles via name_to_handle_at(2)/open_by_handle_at(2).

These special kernel filesystems have custom ->open() and ->permission() export methods, which nfsd does not respect and it was never meant to be used for exporting those filesystems by nfsd.

Update kernel-doc comments to express the fact the those methods are for open_by_handle(2) system only and not compatible with nfsd.

---
 include/linux/exportfs.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
include/linux/exportfs.h +10 -2
index 262e24d833134..0660953c3fb76 100644
@@ -200,6 +200,10 @@ struct handle_to_path_ctx {
  * @get_parent:     find the parent of a given directory
  * @commit_metadata: commit metadata changes to stable storage
  *
+ * Methods for open_by_handle(2) syscall with special kernel file systems:
+ * @permission:     custom permission for opening a file by handle
+ * @open:           custom open routine for opening file by handle
+ *
  * See Documentation/filesystems/nfs/exporting.rst for details on how to use
  * this interface correctly and the definition of the flags.
  *
@@ -244,10 +248,14 @@ struct handle_to_path_ctx {
  *    space cannot be allocated, a %ERR_PTR should be returned.
  *
  * @permission:
- *    Allow filesystems to specify a custom permission function.
+ *    Allow filesystems to specify a custom permission function for the
+ *    open_by_handle_at(2) syscall instead of the default permission check.
+ *    This custom permission function is not respected by nfsd.
  *
  * @open:
- *    Allow filesystems to specify a custom open function.
+ *    Allow filesystems to specify a custom open function for the
+ *    open_by_handle_at(2) syscall instead of the default file_open_root().
+ *    This custom open function is not respected by nfsd.
  *
  * @commit_metadata:
  *    @commit_metadata should commit metadata changes to stable storage.
Signature · 1 line
2.52.0
author reviewer
#2 lore raw

PATCH v4 2/2 nfsd: do not allow exporting of special kernel filesystems

In reply to Amir Goldstein

pidfs and nsfs recently gained support for encode/decode of file handles via name_to_handle_at(2)/open_by_handle_at(2).

These special kernel filesystems have custom ->open() and ->permission() export methods, which nfsd does not respect and it was never meant to be used for exporting those filesystems by nfsd.

Therefore, do not allow nfsd to export filesystems with custom ->open() or ->permission() methods.

Fixes b3caba8f7a34a ("pidfs: implement file handle support")
Fixes 5222470b2fbb3 ("nsfs: support file handles")
---
 fs/nfsd/export.c         | 8 +++++---
 include/linux/exportfs.h | 9 +++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)
fs/nfsd/export.c +5 -3
index 2a1499f2ad196..09fe268fe2c76 100644
@@ -427,7 +427,8 @@ static int check_export(const struct path *path, int *flags, unsigned char *uuid
 	 *       either a device number (so FS_REQUIRES_DEV needed)
 	 *       or an FSID number (so NFSEXP_FSID or ->uuid is needed).
 	 * 2:  We must be able to find an inode from a filehandle.
-	 *       This means that s_export_op must be set.
+	 *       This means that s_export_op must be set and comply with
+	 *       the requirements for remote filesystem export.
 	 * 3: We must not currently be on an idmapped mount.
 	 */
 	if (!(inode->i_sb->s_type->fs_flags & FS_REQUIRES_DEV) &&
@@ -437,8 +438,9 @@ static int check_export(const struct path *path, int *flags, unsigned char *uuid
 		return -EINVAL;
 	}
 
-	if (!exportfs_can_decode_fh(inode->i_sb->s_export_op)) {
-		dprintk("exp_export: export of invalid fs type.\n");
+	if (!exportfs_may_export(inode->i_sb->s_export_op)) {
+		dprintk("exp_export: export of invalid fs type (%s).\n",
+			inode->i_sb->s_type->name);
 		return -EINVAL;
 	}
 
include/linux/exportfs.h +9 -0
index 0660953c3fb76..8bcdba28b4060 100644
@@ -338,6 +338,15 @@ static inline bool exportfs_can_decode_fh(const struct export_operations *nop)
 	return nop && nop->fh_to_dentry;
 }
 
+static inline bool exportfs_may_export(const struct export_operations *nop)
+{
+	/*
+	 * Do not allow nfs export for filesystems with custom ->open() or
+	 * ->permission() ops, which nfsd does not respect (e.g. pidfs, nsfs).
+	 */
+	return exportfs_can_decode_fh(nop) && !nop->open && !nop->permission;
+}
+
 static inline bool exportfs_can_encode_fh(const struct export_operations *nop,
 					  int fh_flags)
 {
Signature · 1 line
2.52.0
Reviewed-by on the v4 series
#3 lore raw
In reply to Amir Goldstein
Quoted 20 lines · Amir Goldstein

Christian,

This v4 addresses Chuck and Jeff's review on v3.

The first doc patch is applicable to the doc update in vfs-7.0.misc. The 2nd fix patch is independent of the doc changes in vfs-7.0.misc, so it should be easier to backport.

Thanks,
Amir.

Changes since v3:
- Fix typo and doc comments from Chunk
- Add RVB

Changes since v2:
- Rebase over vfs-7.0.misc
- Split to doc/fix patches
- Remove RVBs

Quoted 9 lines

Amir Goldstein (2):
exportfs: clarify the documentation of open()/permission() expotrfs
ops
nfsd: do not allow exporting of special kernel filesystems

 fs/nfsd/export.c         |  8 +++++---
 include/linux/exportfs.h | 21 +++++++++++++++++++--
 2 files changed, 24 insertions(+), 5 deletions(-)
Signature · 1 line
Chuck Lever
maintainer Applied
#4 lore raw
In reply to Amir Goldstein
Quoted 9 lines · Amir Goldstein

Christian,

This v4 addresses Chuck and Jeff's review on v3.

The first doc patch is applicable to the doc update in vfs-7.0.misc. The 2nd fix patch is independent of the doc changes in vfs-7.0.misc, so it should be easier to backport.

[...]

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

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

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

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

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.0.misc

[1/2] exportfs: clarify the documentation of open()/permission() expotrfs ops

      https://git.kernel.org/vfs/vfs/c/a39162f77f49
[2/2] nfsd: do not allow exporting of special kernel filesystems
      https://git.kernel.org/vfs/vfs/c/b3c78bc53630