On 23.01.2019 12:45, Miklos Szeredi wrote:
> On Fri, Jan 18, 2019 at 1:28 PM Kirill Tkhai <ktkhai@xxxxxxxxxxxxx> wrote:
>>
>> On 18.01.2019 15:07, Miklos Szeredi wrote:
>>> On Tue, Jan 15, 2019 at 11:19 AM Kirill Tkhai <ktkhai@xxxxxxxxxxxxx> wrote:
>>>>
>>>> fuse_dev_alloc() may be called after fc->connected
>>>> is dropped (from ioctl), so here we add sanity check
>>>> for that case.
>>>
>>> AFAICS this is not fixing a bug; i.e. even if the fuse_dev is added to
>>> the fuse_conn's list after disconnection there would be no leak.
>>>
>>> In other words, it's irrelevant whether the connection reset comes
>>> just before the ioctl completes or just after. Or am I missing
>>> something?
>>
>> Yeah, there won't be a leak. The only problem I see, userspace daemon
>> may become waiting in fuse_dev_do_read() after abort is finished.
>
> By that time fiq->connected will be reset, so fuse_dev_do_read() will
> return ENODEV/ECONNABORTED.
>
> Am I missing something?
I mean:
[cpu0] [cpu1]
fuse_device_clone()
fuse_dev_alloc() fuse_abort_conn()
spin_lock(&fc->lock)
list_for_each_entry(fud, &fc->devices, entry)
fpq->connected = 0
spin_unlock(&fc->lock)
spin_lock(&fc->lock);
list_add_tail(&fud->entry, &fc->devices);
spin_unlock(&fc->lock);
...
fuse_dev_do_read()
wait_event_interruptible_exclusive_locked() <-- wait forever
If you have manager-thread on cpu1, and it expects all worker-threads (like on cpu0)
will finish after abort, this won't be true.
Kirill
