2024-01-26 08:57:19

by Jingbo Xu

[permalink] [raw]
Subject: [PATCH v2] fuse: add support for explicit export disabling

open_by_handle_at(2) can fail with -ESTALE with a valid handle returned
by a previous name_to_handle_at(2) for evicted fuse inodes, which is
especially common when entry_valid_timeout is 0, e.g. when the fuse
daemon is in "cache=none" mode.

The time sequence is like:

name_to_handle_at(2) # succeed
evict fuse inode
open_by_handle_at(2) # fail

The root cause is that, with 0 entry_valid_timeout, the dput() called in
name_to_handle_at(2) will trigger iput -> evict(), which will send
FUSE_FORGET to the daemon. The following open_by_handle_at(2) will send
a new FUSE_LOOKUP request upon inode cache miss since the previous inode
eviction. Then the fuse daemon may fail the FUSE_LOOKUP request with
-ENOENT as the cached metadata of the requested inode has already been
cleaned up during the previous FUSE_FORGET. The returned -ENOENT is
treated as -ESTALE when open_by_handle_at(2) returns.

This confuses the application somehow, as open_by_handle_at(2) fails
when the previous name_to_handle_at(2) succeeds. The returned errno is
also confusing as the requested file is not deleted and already there.
It is reasonable to fail name_to_handle_at(2) early in this case, after
which the application can fallback to open(2) to access files.

Since this issue typically appears when entry_valid_timeout is 0 which
is configured by the fuse daemon, the fuse daemon is the right person to
explicitly disable the export when required.

Also considering FUSE_EXPORT_SUPPORT actually indicates the support for
lookups of "." and "..", and there are existing fuse daemons supporting
export without FUSE_EXPORT_SUPPORT set, for compatibility, we add a new
INIT flag for such purpose.

Signed-off-by: Jingbo Xu <[email protected]>
---
v2:
- rename to "fuse_export_fid_operations"
- bump FUSE_KERNEL_MINOR_VERSION

v1: https://lore.kernel.org/linux-fsdevel/[email protected]/
RFC: https://lore.kernel.org/all/[email protected]/
---
fs/fuse/inode.c | 11 ++++++++++-
include/uapi/linux/fuse.h | 7 ++++++-
2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 2a6d44f91729..eee200308482 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1110,6 +1110,11 @@ static struct dentry *fuse_get_parent(struct dentry *child)
return parent;
}

+/* only for fid encoding; no support for file handle */
+static const struct export_operations fuse_export_fid_operations = {
+ .encode_fh = fuse_encode_fh,
+};
+
static const struct export_operations fuse_export_operations = {
.fh_to_dentry = fuse_fh_to_dentry,
.fh_to_parent = fuse_fh_to_parent,
@@ -1284,6 +1289,8 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
fc->create_supp_group = 1;
if (flags & FUSE_DIRECT_IO_ALLOW_MMAP)
fc->direct_io_allow_mmap = 1;
+ if (flags & FUSE_NO_EXPORT_SUPPORT)
+ fm->sb->s_export_op = &fuse_export_fid_operations;
} else {
ra_pages = fc->max_read / PAGE_SIZE;
fc->no_lock = 1;
@@ -1330,7 +1337,8 @@ void fuse_send_init(struct fuse_mount *fm)
FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_INIT_EXT |
FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP |
- FUSE_HAS_EXPIRE_ONLY | FUSE_DIRECT_IO_ALLOW_MMAP;
+ FUSE_HAS_EXPIRE_ONLY | FUSE_DIRECT_IO_ALLOW_MMAP |
+ FUSE_NO_EXPORT_SUPPORT;
#ifdef CONFIG_FUSE_DAX
if (fm->fc->dax)
flags |= FUSE_MAP_ALIGNMENT;
@@ -1527,6 +1535,7 @@ static int fuse_fill_super_submount(struct super_block *sb,
sb->s_bdi = bdi_get(parent_sb->s_bdi);

sb->s_xattr = parent_sb->s_xattr;
+ sb->s_export_op = parent_sb->s_export_op;
sb->s_time_gran = parent_sb->s_time_gran;
sb->s_blocksize = parent_sb->s_blocksize;
sb->s_blocksize_bits = parent_sb->s_blocksize_bits;
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index e7418d15fe39..38d9f285a599 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -211,6 +211,9 @@
* 7.39
* - add FUSE_DIRECT_IO_ALLOW_MMAP
* - add FUSE_STATX and related structures
+ *
+ * 7.40
+ * - add FUSE_NO_EXPORT_SUPPORT
*/

#ifndef _LINUX_FUSE_H
@@ -246,7 +249,7 @@
#define FUSE_KERNEL_VERSION 7

/** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 39
+#define FUSE_KERNEL_MINOR_VERSION 40

/** The node ID of the root inode */
#define FUSE_ROOT_ID 1
@@ -410,6 +413,7 @@ struct fuse_file_lock {
* symlink and mknod (single group that matches parent)
* FUSE_HAS_EXPIRE_ONLY: kernel supports expiry-only entry invalidation
* FUSE_DIRECT_IO_ALLOW_MMAP: allow shared mmap in FOPEN_DIRECT_IO mode.
+ * FUSE_NO_EXPORT_SUPPORT: explicitly disable export support
*/
#define FUSE_ASYNC_READ (1 << 0)
#define FUSE_POSIX_LOCKS (1 << 1)
@@ -449,6 +453,7 @@ struct fuse_file_lock {
#define FUSE_CREATE_SUPP_GROUP (1ULL << 34)
#define FUSE_HAS_EXPIRE_ONLY (1ULL << 35)
#define FUSE_DIRECT_IO_ALLOW_MMAP (1ULL << 36)
+#define FUSE_NO_EXPORT_SUPPORT (1ULL << 37)

/* Obsolete alias for FUSE_DIRECT_IO_ALLOW_MMAP */
#define FUSE_DIRECT_IO_RELAX FUSE_DIRECT_IO_ALLOW_MMAP
--
2.19.1.6.gb485710b



2024-01-26 09:01:04

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2] fuse: add support for explicit export disabling

On Fri, Jan 26, 2024 at 9:21 AM Jingbo Xu <[email protected]> wrote:
>
> open_by_handle_at(2) can fail with -ESTALE with a valid handle returned
> by a previous name_to_handle_at(2) for evicted fuse inodes, which is
> especially common when entry_valid_timeout is 0, e.g. when the fuse
> daemon is in "cache=none" mode.
>
> The time sequence is like:
>
> name_to_handle_at(2) # succeed
> evict fuse inode
> open_by_handle_at(2) # fail
>
> The root cause is that, with 0 entry_valid_timeout, the dput() called in
> name_to_handle_at(2) will trigger iput -> evict(), which will send
> FUSE_FORGET to the daemon. The following open_by_handle_at(2) will send
> a new FUSE_LOOKUP request upon inode cache miss since the previous inode
> eviction. Then the fuse daemon may fail the FUSE_LOOKUP request with
> -ENOENT as the cached metadata of the requested inode has already been
> cleaned up during the previous FUSE_FORGET. The returned -ENOENT is
> treated as -ESTALE when open_by_handle_at(2) returns.
>
> This confuses the application somehow, as open_by_handle_at(2) fails
> when the previous name_to_handle_at(2) succeeds. The returned errno is
> also confusing as the requested file is not deleted and already there.
> It is reasonable to fail name_to_handle_at(2) early in this case, after
> which the application can fallback to open(2) to access files.
>
> Since this issue typically appears when entry_valid_timeout is 0 which
> is configured by the fuse daemon, the fuse daemon is the right person to
> explicitly disable the export when required.
>
> Also considering FUSE_EXPORT_SUPPORT actually indicates the support for
> lookups of "." and "..", and there are existing fuse daemons supporting
> export without FUSE_EXPORT_SUPPORT set, for compatibility, we add a new
> INIT flag for such purpose.
>
> Signed-off-by: Jingbo Xu <[email protected]>

Reviewed-by: Amir Goldstein <[email protected]>

> ---
> v2:
> - rename to "fuse_export_fid_operations"
> - bump FUSE_KERNEL_MINOR_VERSION
>
> v1: https://lore.kernel.org/linux-fsdevel/[email protected]/
> RFC: https://lore.kernel.org/all/[email protected]/
> ---
> fs/fuse/inode.c | 11 ++++++++++-
> include/uapi/linux/fuse.h | 7 ++++++-
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 2a6d44f91729..eee200308482 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1110,6 +1110,11 @@ static struct dentry *fuse_get_parent(struct dentry *child)
> return parent;
> }
>
> +/* only for fid encoding; no support for file handle */
> +static const struct export_operations fuse_export_fid_operations = {
> + .encode_fh = fuse_encode_fh,
> +};
> +
> static const struct export_operations fuse_export_operations = {
> .fh_to_dentry = fuse_fh_to_dentry,
> .fh_to_parent = fuse_fh_to_parent,
> @@ -1284,6 +1289,8 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
> fc->create_supp_group = 1;
> if (flags & FUSE_DIRECT_IO_ALLOW_MMAP)
> fc->direct_io_allow_mmap = 1;
> + if (flags & FUSE_NO_EXPORT_SUPPORT)
> + fm->sb->s_export_op = &fuse_export_fid_operations;
> } else {
> ra_pages = fc->max_read / PAGE_SIZE;
> fc->no_lock = 1;
> @@ -1330,7 +1337,8 @@ void fuse_send_init(struct fuse_mount *fm)
> FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
> FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_INIT_EXT |
> FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP |
> - FUSE_HAS_EXPIRE_ONLY | FUSE_DIRECT_IO_ALLOW_MMAP;
> + FUSE_HAS_EXPIRE_ONLY | FUSE_DIRECT_IO_ALLOW_MMAP |
> + FUSE_NO_EXPORT_SUPPORT;
> #ifdef CONFIG_FUSE_DAX
> if (fm->fc->dax)
> flags |= FUSE_MAP_ALIGNMENT;
> @@ -1527,6 +1535,7 @@ static int fuse_fill_super_submount(struct super_block *sb,
> sb->s_bdi = bdi_get(parent_sb->s_bdi);
>
> sb->s_xattr = parent_sb->s_xattr;
> + sb->s_export_op = parent_sb->s_export_op;
> sb->s_time_gran = parent_sb->s_time_gran;
> sb->s_blocksize = parent_sb->s_blocksize;
> sb->s_blocksize_bits = parent_sb->s_blocksize_bits;
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index e7418d15fe39..38d9f285a599 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -211,6 +211,9 @@
> * 7.39
> * - add FUSE_DIRECT_IO_ALLOW_MMAP
> * - add FUSE_STATX and related structures
> + *
> + * 7.40
> + * - add FUSE_NO_EXPORT_SUPPORT
> */
>
> #ifndef _LINUX_FUSE_H
> @@ -246,7 +249,7 @@
> #define FUSE_KERNEL_VERSION 7
>
> /** Minor version number of this interface */
> -#define FUSE_KERNEL_MINOR_VERSION 39
> +#define FUSE_KERNEL_MINOR_VERSION 40
>
> /** The node ID of the root inode */
> #define FUSE_ROOT_ID 1
> @@ -410,6 +413,7 @@ struct fuse_file_lock {
> * symlink and mknod (single group that matches parent)
> * FUSE_HAS_EXPIRE_ONLY: kernel supports expiry-only entry invalidation
> * FUSE_DIRECT_IO_ALLOW_MMAP: allow shared mmap in FOPEN_DIRECT_IO mode.
> + * FUSE_NO_EXPORT_SUPPORT: explicitly disable export support
> */
> #define FUSE_ASYNC_READ (1 << 0)
> #define FUSE_POSIX_LOCKS (1 << 1)
> @@ -449,6 +453,7 @@ struct fuse_file_lock {
> #define FUSE_CREATE_SUPP_GROUP (1ULL << 34)
> #define FUSE_HAS_EXPIRE_ONLY (1ULL << 35)
> #define FUSE_DIRECT_IO_ALLOW_MMAP (1ULL << 36)
> +#define FUSE_NO_EXPORT_SUPPORT (1ULL << 37)
>
> /* Obsolete alias for FUSE_DIRECT_IO_ALLOW_MMAP */
> #define FUSE_DIRECT_IO_RELAX FUSE_DIRECT_IO_ALLOW_MMAP
> --
> 2.19.1.6.gb485710b
>