lkml.org 
[lkml]   [2013]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 2/4] fuse: writepages: crop secondary requests
From
On Thu, Oct 3, 2013 at 5:50 PM, Maxim Patlasov <mpatlasov@parallels.com> wrote:
> On 10/03/2013 07:14 PM, Miklos Szeredi wrote:
>>
>> On Thu, Oct 03, 2013 at 05:28:30PM +0400, Maxim Patlasov wrote:
>>
>>> 1. There is an in-flight primary request with a chain of secondary ones.
>>> 2. User calls ftruncate(2) to extend file; fuse_set_nowrite() makes
>>> fi->writectr negative and starts waiting for completion of that
>>> in-flight request
>>> 3. Userspace fuse daemon ACKs the request and fuse_writepage_end()
>>> is called; it calls __fuse_flush_writepages(), but the latter does
>>> nothing because fi->writectr < 0
>>> 4. fuse_do_setattr() proceeds extending i_size and calling
>>> __fuse_release_nowrite(). But now new (increased) i_size will be
>>> used as 'crop' arg of __fuse_flush_writepages()
>>>
>>> stale data can leak to the server.
>>
>> So, lets do this then: skip fuse_flush_writepages() and call
>> fuse_send_writepage() directly. It will ignore the NOWRITE logic, but
>> that's
>> okay, this happens rarely and cannot happen more than once in a row.
>>
>> Does this look good?
>
>
> Yes, but let's at least add a comment explaining why it's safe. There are
> three different cases and what you write above explains only one of them:
>
> 1st case (trivial): there are no concurrent activities using
> fuse_set/release_nowrite. Then we're on safe side because
> fuse_flush_writepages() would call fuse_send_writepage() anyway.
> 2nd case: someone called fuse_set_nowrite and it is waiting now for
> completion of all in-flight requests. Here what you wrote about "happening
> rarely and no more than once" is applicable.
> 3rd case: someone (e.g. fuse_do_setattr()) is in the middle of
> fuse_set_nowrite..fuse_release_nowrite section. The fact that
> fuse_set_nowrite returned implies that all in-flight requests were completed
> along with all its secondary requests (because we increment writectr for a
> secondry before decrementing it for the primary -- that's how
> fuse_writepage_end is implemeted). Further requests are blocked by negative
> writectr. Hence there cannot be any in-flight requests and no invocations of
> fuse_writepage_end while we're in fuse_set_nowrite..fuse_release_nowrite
> section.
>
> It looks obvious now, but I'm not sure we'll able to recollect it later.

Added your analysis as a comment and all patches pushed to writepages.v2.

>> Can you actually trigger this path with your testing?
>
>
> No.

Hmm, did you do any testing with the wait-for-page-writeback disabled
in fuse_mkwrite()?

Thanks,
Miklos


\
 
 \ /
  Last update: 2013-10-04 08:21    [W:0.079 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site