[2/2] fs/fuse: fix unsafe killing fiq->pending requests

Submitted by Pavel Butsykin on Jan. 24, 2019, 1:12 p.m.

Details

Message ID 20190124131245.29372-2-pbutsykin@virtuozzo.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Pavel Butsykin Jan. 24, 2019, 1:12 p.m.
There are two problems related to the lack of fiq locking in 
fuse_invalidate_files(). The first is an unsafe iteration of
fiq->pending list in fuse_kill_requests(). The second problem
is the race between __fuse_request_send() and fuse_invalidate_files():

__fuse_request_send():                                  fuse_invalidate_files():
spin_lock(&fiq->waitq.lock);
test_bit(FUSE_S_FAIL_IMMEDIATELY, &ff->ff_state)
                                                        spin_lock(&fc->lock);
                                                        list_for_each_entry(ff, &fi->rw_files, rw_entry)
                                                          set_bit(FUSE_S_FAIL_IMMEDIATELY, &ff->ff_state);
                                                        spin_unlock(&fc->lock);

                                                        spin_lock(&fc->lock);

                                                        spin_lock(&fpq->lock);
                                                        fuse_kill_requests(fc, inode, &fiq->pending);
                                                        spin_unlock(&fpq->lock);
                                                        spin_unlock(&fc->lock);

queue_request(fiq, req);    <-- add a new request after fuse_invalidate_files(),
spin_unlock(&fiq->waitq.lock);  that's wrong.

The patch fixes both problems.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
---
 fs/fuse/inode.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index cb275ff21991..80bfe45d5d46 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -437,9 +437,12 @@  int fuse_invalidate_files(struct fuse_conn *fc, u64 nodeid)
 			spin_lock(&fpq->lock);
 			for (i = 0; i < FUSE_PQ_HASH_SIZE; i++)
 				fuse_kill_requests(fc, inode, &fpq->processing[i]);
-			fuse_kill_requests(fc, inode, &fiq->pending);
 			fuse_kill_requests(fc, inode, &fpq->io);
 			spin_unlock(&fpq->lock);
+
+			spin_lock(&fiq->waitq.lock);
+			fuse_kill_requests(fc, inode, &fiq->pending);
+			spin_unlock(&fiq->waitq.lock);
 		}
 		fuse_kill_requests(fc, inode, &fc->main_iq.pending);
 		fuse_kill_requests(fc, inode, &fc->bg_queue);

Comments

Kirill Tkhai Feb. 1, 2019, 2:16 p.m.
On 24.01.2019 16:12, Pavel Butsykin wrote:
> There are two problems related to the lack of fiq locking in 
> fuse_invalidate_files(). The first is an unsafe iteration of
> fiq->pending list in fuse_kill_requests(). The second problem
> is the race between __fuse_request_send() and fuse_invalidate_files():
> 
> __fuse_request_send():                                  fuse_invalidate_files():
> spin_lock(&fiq->waitq.lock);
> test_bit(FUSE_S_FAIL_IMMEDIATELY, &ff->ff_state)
>                                                         spin_lock(&fc->lock);
>                                                         list_for_each_entry(ff, &fi->rw_files, rw_entry)
>                                                           set_bit(FUSE_S_FAIL_IMMEDIATELY, &ff->ff_state);
>                                                         spin_unlock(&fc->lock);
> 
>                                                         spin_lock(&fc->lock);
> 
>                                                         spin_lock(&fpq->lock);
>                                                         fuse_kill_requests(fc, inode, &fiq->pending);
>                                                         spin_unlock(&fpq->lock);
>                                                         spin_unlock(&fc->lock);
> 
> queue_request(fiq, req);    <-- add a new request after fuse_invalidate_files(),
> spin_unlock(&fiq->waitq.lock);  that's wrong.
> 
> The patch fixes both problems.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>

Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>

> ---
>  fs/fuse/inode.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index cb275ff21991..80bfe45d5d46 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -437,9 +437,12 @@ int fuse_invalidate_files(struct fuse_conn *fc, u64 nodeid)
>  			spin_lock(&fpq->lock);
>  			for (i = 0; i < FUSE_PQ_HASH_SIZE; i++)
>  				fuse_kill_requests(fc, inode, &fpq->processing[i]);
> -			fuse_kill_requests(fc, inode, &fiq->pending);
>  			fuse_kill_requests(fc, inode, &fpq->io);
>  			spin_unlock(&fpq->lock);
> +
> +			spin_lock(&fiq->waitq.lock);
> +			fuse_kill_requests(fc, inode, &fiq->pending);
> +			spin_unlock(&fiq->waitq.lock);
>  		}
>  		fuse_kill_requests(fc, inode, &fc->main_iq.pending);
>  		fuse_kill_requests(fc, inode, &fc->bg_queue);
>