On Tue, Aug 26, 2025 at 12:26 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
>
> On Fri, Aug 22, 2025 at 03:52:38PM -0700, Joanne Koong wrote:
> > On Fri, Aug 22, 2025 at 3:46 PM Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
> > >
> > > On Fri, Aug 22, 2025 at 4:44 AM Miklos Szeredi <mszeredi@xxxxxxxxxx> wrote:
> > > >
> > > > FUSE_INIT has always been asynchronous with mount. That means that the
> > > > server processed this request after the mount syscall returned.
> > > >
> > > > This means that FUSE_INIT can't supply the root inode's ID, hence it
> > > > currently has a hardcoded value. There are other limitations such as not
> > > > being able to perform getxattr during mount, which is needed by selinux.
> > > >
> > > > To remove these limitations allow server to process FUSE_INIT while
> > > > initializing the in-core super block for the fuse filesystem. This can
> > > > only be done if the server is prepared to handle this, so add
> > > > FUSE_DEV_IOC_SYNC_INIT ioctl, which
> > > >
> > > > a) lets the server know whether this feature is supported, returning
> > > > ENOTTY othewrwise.
> > > >
> > > > b) lets the kernel know to perform a synchronous initialization
> > > >
> > > > The implementation is slightly tricky, since fuse_dev/fuse_conn are set up
> > > > only during super block creation. This is solved by setting the private
> > > > data of the fuse device file to a special value ((struct fuse_dev *) 1) and
> > > > waiting for this to be turned into a proper fuse_dev before commecing with
> > > > operations on the device file.
> > > >
> > > > Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
> > > > ---
> > > > I tested this with my raw-interface tester, so no libfuse update yet. Will
> > > > work on that next.
> > > >
> > > > fs/fuse/cuse.c | 3 +-
> > > > fs/fuse/dev.c | 74 +++++++++++++++++++++++++++++----------
> > > > fs/fuse/dev_uring.c | 4 +--
> > > > fs/fuse/fuse_dev_i.h | 13 +++++--
> > > > fs/fuse/fuse_i.h | 3 ++
> > > > fs/fuse/inode.c | 46 +++++++++++++++++++-----
> > > > include/uapi/linux/fuse.h | 1 +
> > > > 7 files changed, 112 insertions(+), 32 deletions(-)
> > > >
> > > > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > > > index 9d26a5bc394d..d5f9f2abc569 100644
> > > > --- a/fs/fuse/inode.c
> > > > +++ b/fs/fuse/inode.c
> > > > @@ -1918,8 +1934,22 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> > > > return err;
> > > > /* file->private_data shall be visible on all CPUs after this */
> > > > smp_mb();
> > > > - fuse_send_init(get_fuse_mount_super(sb));
> > > > - return 0;
> > > > +
> > > > + fm = get_fuse_mount_super(sb);
> > > > +
> > > > + if (fm->fc->sync_init) {
> > > > + struct fuse_init_args *ia = fuse_new_init(fm);
> > > > +
> > > > + err = fuse_simple_request(fm, &ia->args);
> > > > + if (err > 0)
> > > > + err = 0;
> > > > + process_init_reply(fm, &ia->args, err);
> > >
> > > Do we need a fuse_dev_free() here if err < 0? If err < 0 then the
>
> Er... are you asking if we should drop the newly created fud via
> fuse_dev_release if err != 0? (AFAICT there is no fuse_dev_free?)
>
That's weird, I see fuse_dev_free() in fs/fuse/inode.c (eg
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/fuse/inode.c#n1624)
> > > mount fails, but fuse_fill_super_common() -> fuse_dev_alloc_install()
> > > will have already been called which if i'm understanding it correctly
> > > means otherwise the fc will get leaked in this case. Or I guess
> > > another option is to retain original behavior with having the mount
> > > succeed even if the init server reply returns back an error code?
>
> <shrug> I was figuring that it was fine to leave the fud attached to the
> device fd until the caller close()s it, but OTOH maybe the fuse server
> would like to try to mount again? Do fuse servers do that?
Won't this still leak the reference? From what I see, the mount will
create the fc (refcount 1) then when the mount does the dev
installation (fuse_dev_install()) that'll acquire another reference on
fc. With the caller close()ing it, that releases 1 refcount but
there's still 1 refcount that needs to be released by
fuse_mount_destroy(). As I understand it, if the mount fails, the
.kill_sb -> fuse_mount_destroy() never gets triggered (since it was
never mounted) which will leave 1 refcount remaining.
>
> --D
>
