On Thu, 7 Nov 2024 at 18:59, Hanna Czenczek <hreitz@xxxxxxxxxx> wrote:
> > Regardless, something smells here: fuse_mount_remove() is only called
> > if sb->s_root is set (both plain fuse and virtiofs). The top level
> > fuse_mount is added to fc->mounts in fuse_conn_init(), way before
> > sb->s_root is set...
> >
> > Will look into this.
This is the deal:
1) When the top sb is created the fm->fc_entry is chained on
fc->mounts in fuse_conn_init() which is called near the top of
fuse_get_tree() and virtio_fs_get_tree(). If things fail during
->get_tree() or an existing fc is used instead of the newly allocated
one, then fuse_mount_destroy() is called, which frees the fm and
fm->fc, ignoring the mounts list. This is okay, though a bit
confusing.
2) When a submount is created, then fm is only chained onto fc->mounts
towards the end of fuse_get_tree_submount() in the success case.
There should be no way for fuse_mount_destroy() to see fm chained onto
fc yet fc having other fuse mounts. The below patch reflects this
with a WARN_ON(). Not tested yet, but there would be memory
corruption if it wasn't the case.
As for your patch, the condition (sb->s_root || (fm &&
fm->fc->initialized)) should work AFAICS.
Thanks,
Miklos
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index fd3321e29a3e..0c4eb5b89e71 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1653,6 +1653,7 @@ static int fuse_get_tree_submount(struct fs_context *fsc)
if (!fm)
return -ENOMEM;
+ INIT_LIST_HEAD(&fm->fc_entry);
fm->fc = fuse_conn_get(fc);
fsc->s_fs_info = fm;
sb = sget_fc(fsc, NULL, set_anon_super_fc);
@@ -1976,6 +1977,13 @@ static void fuse_sb_destroy(struct super_block *sb)
void fuse_mount_destroy(struct fuse_mount *fm)
{
+ /*
+ * We can get here in case of an error before the top sb is fully set
+ * up. The sole reference to the fc must come from fm in that case
+ * otherwise may end up with corruption on fc->mounts list.
+ */
+ WARN_ON(!list_empty(&fm->fc_entry) &&
refcount_read(&fm->fc->count) != 1);
+
fuse_conn_put(fm->fc);
kfree_rcu(fm, rcu);
}
