Message-ID: <CAJfpegv76ZbHec=N3z1LfYazwbsgzwr0UZf6LWd+G=J3EPmi1Q@mail.gmail.com>
Date:	Tue, 30 Sep 2014 05:15:36 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	Maxim Patlasov <MPatlasov@...allels.com>
Cc:	fuse-devel <fuse-devel@...ts.sourceforge.net>,
	Anand Avati <avati@...ster.org>,
	Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linux-Fsdevel <linux-fsdevel@...r.kernel.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 0/5] fuse: handle release synchronously (v4)

On Fri, Sep 26, 2014 at 5:28 PM, Miklos Szeredi <miklos@...redi.hu> wrote:
> [Adding CC's]
>
> On Thu, Sep 25, 2014 at 2:05 PM, Maxim Patlasov <MPatlasov@...allels.com> wrote:
>
>> In short, the problem is that fuse_release (that's usually called on last
>> user close(2)) sends FUSE_RELEASE to userspace and returns without waiting
>> for ACK from userspace. Consequently, there is a gap when user regards the
>> file released while userspace fuse is still working on it. An attempt to
>> access the file from another node leads to complicated synchronization
>> problems because the first node still "holds" the file.
>>
>> The patch-set resolves the problem by making fuse_release synchronous:
>> wait for ACK from userspace for FUSE_RELEASE if the feature is ON.
>>
>> It must be emphasized that even if the feature is enabled (i.e. fuse_release
>> is synchronous), nothing guarantees that fuse_release() will be called
>> in the context of close(2). In fact, it may happen later, on last fput().
>> However, there are still a lot of use cases when close(2) is synchronous,
>> so the feature must be regarded as an optimization maximizing chances of
>> synchronous behaviour of close(2).
>
> Okay, we have the common case of close -> last-fput ->release.  This
> being synchronous is fine.  Related cases are munmap(), and weird
> corner cases including I/O on file descriptor being done in parallel
> with close() in another thread.  Synchronous behavior is also fine in
> these cases, since the task calling the last fput() is in fact
> responsible for releasing the file.
>
> And then we have the uncommon cases when fput() is called from
> something unrelated.  Take the following DoS example:  malicious app
> creates a socket loop (sockA is sent over sockB and sockB is sent over
> sockA) and in addition it tacks a fuse fd onto one of the sockets.
> The fuse fd is implemented to block forever on release.  In this case
> the loop will persist after the sockets are closed due to refcounting.
> Later, a garbage collection is triggered from a completely unrelated
> socket operation.   This result in the unrelated task being blocked
> forever.
>
> The simplest way to avoid such an attack is to make the sync-release
> feature privileged.  But even if it's privileged, the fact that
> ->release can take a lot of time and a completely unrelated task could
> be waiting for it to finish is not a good thing.
>
> So I'm wondering: could we have some way of distinguishing "good
> release" from "bad release"?  Maybe adding an fput_sync() variant that
> passes an "sync" flag to ->release()?

If we just concentrate on close(2), then there's a much simpler
solution: in fuse_flush() check if file_count(file) is 1.  If it is,
then this is a final close and we can mark the FLUSH request as such
with a flag (FUSE_FLUSH_FINAL).  Userspace filesystem can act
accordingly and may even return an error value to close(2).  In this
case we could even omit the RELEASE request as an optimization, since
we know the file cannot be resurrected if this was the last reference.

Do we need to be synchronous in any other case?  E.g. munmap() comes to mind.

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists