linux-kernel - Re: [PATCH] fuse: add support for explicit export disabling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOQ4uxjN6f=M8jjM0-_eysLy8Jx1+r0Dy3MhWHc8f2r7RnEmdQ@mail.gmail.com>
Date: Wed, 24 Jan 2024 16:10:50 +0200
From: Amir Goldstein <amir73il@...il.com>
To: Miklos Szeredi <miklos@...redi.hu>
Cc: Jingbo Xu <jefflexu@...ux.alibaba.com>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fuse: add support for explicit export disabling

On Wed, Jan 24, 2024 at 2:17 PM Miklos Szeredi <miklos@...redi.hu> wrote:
>
> On Wed, 24 Jan 2024 at 12:30, Jingbo Xu <jefflexu@...ux.alibaba.com> 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.
>
> This looks good overall.
>
> >
> > Signed-off-by: Jingbo Xu <jefflexu@...ux.alibaba.com>
> > ---
> > RFC: https://lore.kernel.org/all/20240123093701.94166-1-jefflexu@linux.alibaba.com/
> > ---
> >  fs/fuse/inode.c           | 11 ++++++++++-
> >  include/uapi/linux/fuse.h |  2 ++
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index 2a6d44f91729..851940c0e930 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_fid_operations = {
>
> Nit: I'd call this fuse_no_export_operations (or something else that
> emphasizes the fact that this is only for encoding and not for full
> export support).

Not that I really care what the name is, but overlayfs already has
ovl_export_fid_operations and the name aspires from AT_HANDLE_FID,
which is already documented in man pages.

How about fuse_export_fid_operations?

Thanks,
Amir.