linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miklos Szeredi <miklos@szeredi.hu>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	syzbot <syzbot+bb6d800770577a083f8c@syzkaller.appspotmail.com>
Subject: Re: INFO: task hung in fuse_reverse_inval_entry
Date: Thu, 26 Jul 2018 10:44:26 +0200	[thread overview]
Message-ID: <CAJfpegvp8xEJc2-MkQvS-HEhtTAt-sZb1VvDPPUx1c+FdqcjEg@mail.gmail.com> (raw)
In-Reply-To: <CACT4Y+ZZ_v6izsiCGym=r+MiRmxaFE=eH54dz7t0BOQ7VxTNiw@mail.gmail.com>

On Wed, Jul 25, 2018 at 11:12 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Tue, Jul 24, 2018 at 5:17 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>>>> Biggest conceptual problem: your definition of fuse-server is weak.
>>>>>> Take the following example: process A is holding the fuse device fd
>>>>>> and is forwarding requests and replies to/from process B via a pipe.
>>>>>> So basically A is just a proxy that does nothing interesting, the
>>>>>> "real" server is B.  But according to your definition B is not a
>>>>>> server, only A is.
>>>>>
>>>>> I proposed to abort fuse conn when all fuse device fd's are "killed"
>>>>> (all processes having the fd opened are killed). So if _only_ process
>>>>> B is killed, then, yes, it will still hang. However if A is killed or
>>>>> both A and B (say, process group, everything inside of pid namespace,
>>>>> etc) then the deadlock will be autoresolved without human
>>>>> intervention.
>>>>
>>>> Okay, so you're saying:
>>>>
>>>> 1) when process gets SIGKILL and is uninterruptible sleep mark process as doomed
>>>> 2) for a particular fuse instance find set of fuse device fd
>>>> references that are in non-doomed tasks; if there are none then abort
>>>> fuse instance
>>>>
>>>> Right?
>>>
>>>
>>> Yes, something like this.
>>> Perhaps checking for "uninterruptible sleep" is excessive. If it has
>>> SIGKILL pending it's pretty much doomed already. This info should be
>>> already available for tasks.
>>> Not saying that it's better, but what I described was the other way
>>> around: when a task killed it drops a reference to all opened fuse
>>> fds, when the last fd is dropped, the connection can be aborted.
>>
>> struct task_struct {
>> [...]
>>     struct files_struct        *files;
>> [...]
>> };
>>
>> struct files_struct {
>> [...]
>>     struct fdtable __rcu *fdt;
>> [...]
>> };
>>
>> struct fdtable {
>> [...]
>>     struct file __rcu **fd;      /* current fd array */
>> [...]
>> };
>>
>> So there we have an array of pointers to struct files.  Suppose we'd
>> magically be able to find files that point to fuse devices upon
>> receiving SIGKILL, what would we do with them?  We can't close them:
>> other tasks might still be pointing to the same files_struct.
>>
>> We could do a global search for non-doomed tasks referencing the same
>> fuse device, but I have no clue how we'd go about doing that without
>> racing with forks, fd sending, etc...
>
>
> Good questions for which I don't have answers.
>
> Maybe more waits in fuse need to be interruptible? E.g. request_wait_answer?

That's an interesting aspect.  Making request_wait_answer always be
killable would help with the issue you raise (killing set of processes
taking part in deadlock should resolve deadlock), but it breaks
another aspect of the interface.

Namely that userspace filesystems expect some serialization from
kernel when performing operations.  If we allow killing of a process
in the middle of an fs operation, then that serialization is no longer
there, which can break the server.

One solution to that is to duplicate all locking in the server
(libfuse normally), but it would not solve the issue for legacy
libfuse or legacy non-libfuse servers.  It would also be difficult to
test.  Also it doesn't solve the problem of killing the server, as
that alone doesn't resolve the deadlock.

Thanks,
Miklos

  reply	other threads:[~2018-07-26 10:00 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-23  7:59 INFO: task hung in fuse_reverse_inval_entry syzbot
2018-07-23  8:11 ` Dmitry Vyukov
2018-07-23 12:12   ` Miklos Szeredi
2018-07-23 12:22     ` Dmitry Vyukov
2018-07-23 12:33       ` Miklos Szeredi
2018-07-23 12:46         ` Dmitry Vyukov
2018-07-23 13:05           ` Miklos Szeredi
2018-07-23 13:37             ` Dmitry Vyukov
2018-07-23 15:09               ` Miklos Szeredi
2018-07-23 15:19                 ` Dmitry Vyukov
2018-07-24 15:17                   ` Miklos Szeredi
2018-07-25  9:12                     ` Dmitry Vyukov
2018-07-26  8:44                       ` Miklos Szeredi [this message]
2018-07-26  9:12                         ` Miklos Szeredi
2018-11-02 19:31                           ` Dmitry Vyukov
2019-11-07 13:42 ` syzbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJfpegvp8xEJc2-MkQvS-HEhtTAt-sZb1VvDPPUx1c+FdqcjEg@mail.gmail.com \
    --to=miklos@szeredi.hu \
    --cc=dvyukov@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=syzbot+bb6d800770577a083f8c@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).