On Wed, Jun 8, 2022 at 3:34 AM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
>
> On Tue, Jun 07, 2022 at 07:05:04PM +0800, Xie Yongji wrote:
> > The commit 15c8e72e88e0 ("fuse: allow skipping control
> > interface and forced unmount") tries to remove the control
> > interface for virtio-fs since it does not support aborting
> > requests which are being processed. But it doesn't work now.
>
> Aha.., so "no_control" basically has no effect? I was looking at
> the code and did not find anybody using "no_control" and I was
> wondering who is making use of "no_control" variable.
>
> I mounted virtiofs and noticed a directory named "40" showed up
> under /sys/fs/fuse/connections/. That must be belonging to
> virtiofs instance, I am assuming.
>
I think so.
> BTW, if there are multiple fuse connections, how will one figure
> out which directory belongs to which instance. Because without knowing
> that, one will be shooting in dark while trying to read/write any
> of the control files.
>
We can use "stat $mountpoint" to get the device minor ID which is the
name of the corresponding control directory.
> So I think a separate patch should be sent which just gets rid of
> "no_control" saying nobody uses. it.
>
OK.
> >
> > This commit fixes the bug, but only remove the abort interface
> > instead since other interfaces should be useful.
>
> Hmm.., so writing to "abort" file is bad as it ultimately does.
>
> fc->connected = 0;
>
Another problem is that it might trigger UAF since
virtio_fs_request_complete() doesn't know the requests are aborted.
> So getting rid of this file till we support aborting the pending
> requests properly, makes sense.
>
> I think this probably should be a separate patch which explains
> why adding "no_abort_control" is a good idea.
>
OK.
Thanks,
Yongji
