linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miklos Szeredi <miklos@szeredi.hu>
To: Alexey Gladkov <gladkov.alexey@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	linux-fsdevel@vger.kernel.org, Alexey Gladkov <legion@kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: Re: [RESEND PATCH v3] fuse: Abort waiting for a response if the daemon receives a fatal signal
Date: Mon, 9 Nov 2020 16:27:05 +0100	[thread overview]
Message-ID: <CAJfpegua_ahmNa4p0me6R10wtcPpQVKNiKQOVKjuNW67RHFOOA@mail.gmail.com> (raw)
In-Reply-To: <1e796f9e008fb78fb96358ff74f39bd4865a7c88.1604926010.git.gladkov.alexey@gmail.com>

On Mon, Nov 9, 2020 at 1:48 PM Alexey Gladkov <gladkov.alexey@gmail.com> wrote:
>
> This patch removes one kind of the deadlocks inside the fuse daemon. The
> problem appear when the fuse daemon itself makes a file operation on its
> filesystem and receives a fatal signal.
>
> This deadlock can be interrupted via fusectl filesystem. But if you have
> many fuse mountpoints, it will be difficult to figure out which
> connection to break.
>
> This patch aborts the connection if the fuse server receives a fatal
> signal.

The patch itself might be acceptable, but I have some questions.

To logic of this patch says:

"If a task having the fuse device open in it's fd table receives
SIGKILL (and filesystem was initially mounted in a non-init user
namespace), then abort the filesystem operation"

You just say "server" instead of "task having the fuse device open in
it's fd table" which is sloppy to say the least.  It might also lead
to regressions, although I agree that it's unlikely.

Also how is this solving any security issue?   Just create the request
loop using two fuse filesystems and the deadlock avoidance has just
been circumvented.   So AFAICS "selling" this as a CVE fix is not
appropriate.

What's the reason for making this user-ns only?   If we drop the
security aspect, then I don't see any reason not to do this
unconditionally.

Also note, there's a proper solution for making fuse requests always
killable, and that is to introduce a shadow locking that ensures
correct fs operation in the face of requests that have returned and
released their respective VFS locks.   Now this would be a much more
complex solution, but also a much more correct one, not having issues
with correctly defining what a server is (which is not a solvable
problem).

Thanks,
Miklos

  reply	other threads:[~2020-11-09 15:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-09 12:46 [RESEND PATCH v3] fuse: Abort waiting for a response if the daemon receives a fatal signal Alexey Gladkov
2020-11-09 15:27 ` Miklos Szeredi [this message]
2020-11-09 18:54   ` Eric W. Biederman
2020-11-09 20:24     ` Miklos Szeredi
2020-11-11  7:42       ` Eric W. Biederman
2020-11-11  8:05         ` Miklos Szeredi

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=CAJfpegua_ahmNa4p0me6R10wtcPpQVKNiKQOVKjuNW67RHFOOA@mail.gmail.com \
    --to=miklos@szeredi.hu \
    --cc=ebiederm@xmission.com \
    --cc=gladkov.alexey@gmail.com \
    --cc=legion@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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).