[Devel,RHEL7,COMMIT] fuse: fuse_send_writepage() must check FUSE_S_FAIL_IMMEDIATELY

Submitted by Konstantin Khorenko on Dec. 8, 2016, 3:01 p.m.

Details

Message ID 201612081501.uB8F18do003757@finist_cl7.x64_64.work.ct
State New
Series "fuse: fuse_send_writepage() must check FUSE_S_FAIL_IMMEDIATELY"
Headers show

Commit Message

Konstantin Khorenko Dec. 8, 2016, 3:01 p.m.
The commit is pushed to "branch-rh7-3.10.0-327.36.1.vz7.20.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-327.36.1.vz7.20.12
------>
commit d9db74f4ea93d48aebf0f3f298ff7da1e92d4e5b
Author: Maxim Patlasov <mpatlasov@virtuozzo.com>
Date:   Thu Dec 8 19:01:07 2016 +0400

    fuse: fuse_send_writepage() must check FUSE_S_FAIL_IMMEDIATELY
    
    The patch fixes the following race (leading to deadlock):
    
    1. Thread A. Enter fuse_prepare_write() checks for FUSE_S_FAIL_IMMEDIATELY,
    but it was not set yet, so it doesn't return -EIO.
    
    2. Thread B. Enter fuse_invalidate_files(). It sets FUSE_S_FAIL_IMMEDIATELY,
    calls filemap_write_and_wait() and fuse_kill_requests(), then release
    fc->lock.
    
    3. Thread A. fuse_commit_write() marks page as "Dirty", then
    fuse_write_end() unlocks the page.
    
    4. Thread B. fuse_invalidate_files() calls invalidate_inode_pages2(). The
    page is dirty, so it ends up in fuse_launder_page() calling
    fuse_writepage_locked(). The latter successfully proceeds queuing
    fuse-write-back request, but then fuse_launder_page() calls
    fuse_wait_on_page_writeback() that blocks forever because Thread A is still
    blocked in fuse_invalidate_files().
    
    Signed-off-by: Maxim Patlasov <mpatlasov@virtuozzo.com>
---
 fs/fuse/file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 641b570..a14c929 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1875,7 +1875,8 @@  __acquires(fc->lock)
 	struct fuse_write_in *inarg = &req->misc.write.in;
 	__u64 data_size = req->num_pages * PAGE_CACHE_SIZE;
 
-	if (!fc->connected)
+	if (!fc->connected ||
+	    test_bit(FUSE_S_FAIL_IMMEDIATELY, &req->ff->ff_state))
 		goto out_free;
 
 	if (inarg->offset + data_size <= size) {

Comments

Konstantin Khorenko Dec. 8, 2016, 3:01 p.m.
Please consider to RK.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 12/08/2016 06:01 PM, Konstantin Khorenko wrote:
> The commit is pushed to "branch-rh7-3.10.0-327.36.1.vz7.20.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
> after rh7-3.10.0-327.36.1.vz7.20.12
> ------>
> commit d9db74f4ea93d48aebf0f3f298ff7da1e92d4e5b
> Author: Maxim Patlasov <mpatlasov@virtuozzo.com>
> Date:   Thu Dec 8 19:01:07 2016 +0400
>
>     fuse: fuse_send_writepage() must check FUSE_S_FAIL_IMMEDIATELY
>
>     The patch fixes the following race (leading to deadlock):
>
>     1. Thread A. Enter fuse_prepare_write() checks for FUSE_S_FAIL_IMMEDIATELY,
>     but it was not set yet, so it doesn't return -EIO.
>
>     2. Thread B. Enter fuse_invalidate_files(). It sets FUSE_S_FAIL_IMMEDIATELY,
>     calls filemap_write_and_wait() and fuse_kill_requests(), then release
>     fc->lock.
>
>     3. Thread A. fuse_commit_write() marks page as "Dirty", then
>     fuse_write_end() unlocks the page.
>
>     4. Thread B. fuse_invalidate_files() calls invalidate_inode_pages2(). The
>     page is dirty, so it ends up in fuse_launder_page() calling
>     fuse_writepage_locked(). The latter successfully proceeds queuing
>     fuse-write-back request, but then fuse_launder_page() calls
>     fuse_wait_on_page_writeback() that blocks forever because Thread A is still
>     blocked in fuse_invalidate_files().
>
>     Signed-off-by: Maxim Patlasov <mpatlasov@virtuozzo.com>
> ---
>  fs/fuse/file.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 641b570..a14c929 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1875,7 +1875,8 @@ __acquires(fc->lock)
>  	struct fuse_write_in *inarg = &req->misc.write.in;
>  	__u64 data_size = req->num_pages * PAGE_CACHE_SIZE;
>
> -	if (!fc->connected)
> +	if (!fc->connected ||
> +	    test_bit(FUSE_S_FAIL_IMMEDIATELY, &req->ff->ff_state))
>  		goto out_free;
>
>  	if (inarg->offset + data_size <= size) {
> .
>
Evgenii Shatokhin Dec. 8, 2016, 3:46 p.m.
On 08.12.2016 18:01, Konstantin Khorenko wrote:
> Please consider to RK.

Queued, thanks.

>
> --
> Best regards,
>
> Konstantin Khorenko,
> Virtuozzo Linux Kernel Team
>
> On 12/08/2016 06:01 PM, Konstantin Khorenko wrote:
>> The commit is pushed to "branch-rh7-3.10.0-327.36.1.vz7.20.x-ovz" and
>> will appear at https://src.openvz.org/scm/ovz/vzkernel.git
>> after rh7-3.10.0-327.36.1.vz7.20.12
>> ------>
>> commit d9db74f4ea93d48aebf0f3f298ff7da1e92d4e5b
>> Author: Maxim Patlasov <mpatlasov@virtuozzo.com>
>> Date:   Thu Dec 8 19:01:07 2016 +0400
>>
>>     fuse: fuse_send_writepage() must check FUSE_S_FAIL_IMMEDIATELY
>>
>>     The patch fixes the following race (leading to deadlock):
>>
>>     1. Thread A. Enter fuse_prepare_write() checks for
>> FUSE_S_FAIL_IMMEDIATELY,
>>     but it was not set yet, so it doesn't return -EIO.
>>
>>     2. Thread B. Enter fuse_invalidate_files(). It sets
>> FUSE_S_FAIL_IMMEDIATELY,
>>     calls filemap_write_and_wait() and fuse_kill_requests(), then release
>>     fc->lock.
>>
>>     3. Thread A. fuse_commit_write() marks page as "Dirty", then
>>     fuse_write_end() unlocks the page.
>>
>>     4. Thread B. fuse_invalidate_files() calls
>> invalidate_inode_pages2(). The
>>     page is dirty, so it ends up in fuse_launder_page() calling
>>     fuse_writepage_locked(). The latter successfully proceeds queuing
>>     fuse-write-back request, but then fuse_launder_page() calls
>>     fuse_wait_on_page_writeback() that blocks forever because Thread A
>> is still
>>     blocked in fuse_invalidate_files().
>>
>>     Signed-off-by: Maxim Patlasov <mpatlasov@virtuozzo.com>
>> ---
>>  fs/fuse/file.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
>> index 641b570..a14c929 100644
>> --- a/fs/fuse/file.c
>> +++ b/fs/fuse/file.c
>> @@ -1875,7 +1875,8 @@ __acquires(fc->lock)
>>      struct fuse_write_in *inarg = &req->misc.write.in;
>>      __u64 data_size = req->num_pages * PAGE_CACHE_SIZE;
>>
>> -    if (!fc->connected)
>> +    if (!fc->connected ||
>> +        test_bit(FUSE_S_FAIL_IMMEDIATELY, &req->ff->ff_state))
>>          goto out_free;
>>
>>      if (inarg->offset + data_size <= size) {
>> .
>>
> .
>