Re: [resend PATCH v2 2/2] fuse: ensure that submounts lookup their parent

[Krister Johansen]

Hi Miklos,

On Tue, Oct 10, 2023 at 10:15:38AM +0200, Miklos Szeredi wrote:
> On Tue, 10 Oct 2023 at 04:35, Krister Johansen <kjlx@xxxxxxxxxxxxxxxxxx> wrote:
> 
> > If I manually traverse the path to the submount via something like cd
> > and ls from the initial mount namespace, it'll stay referenced until I
> > run a umount for the automounted path.  I'm reasonably sure it's the
> > container setup that's causing the detaching.
> 
> Okay.  Can you please try the attached test patch.  It's not a proper
> solution, but I think it's the right direction.

Thanks, I tested this patch and was unable to reproduce the scenario
where an OOM resulted in the submount from the guest in the guest
getting an EBADF from virtiofsd.  (I did generate OOMs, though).

I am curious what you have in mind in order to move this towards a
proper fix?  I shied away from the approach of stealing a nlookup from
mp_fi beacuse it wasn't clear that I could always count on the nlookup
in the parent staying positive.  E.g. I was afraid I was either going to
not have enough nlookups to move to submounts, or trigger a forget from
an exiting container that leads to an EBADF from the initial mount
namespace.

-K

> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 2e4eb7cf26fb..d5f47203dfbc 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1524,6 +1524,18 @@ static int fuse_get_tree_submount(struct fs_context *fsc)
>  		return err;
>  	}
>  
> +	spin_lock(&mp_fi->lock);
> +	if (mp_fi->nlookup) {
> +		struct fuse_inode *fi = get_fuse_inode(d_inode(sb->s_root));
> +		mp_fi->nlookup--;
> +		spin_unlock(&mp_fi->lock);
> +		spin_lock(&fi->lock);
> +		fi->nlookup++;
> +		spin_unlock(&fi->lock);
> +	} else {
> +		spin_unlock(&mp_fi->lock);
> +	}
> +
>  	down_write(&fc->killsb);
>  	list_add_tail(&fm->fc_entry, &fc->mounts);
>  	up_write(&fc->killsb);