[PATCH] fuse: clear FR_PENDING flag when moving requests out of pending queue

* [PATCH] fuse: clear FR_PENDING flag when moving requests out of pending queue
@ 2017-01-12 20:04 Tahsin Erdogan
  2017-01-13 11:13 ` Miklos Szeredi
  0 siblings, 1 reply; 6+ messages in thread
From: Tahsin Erdogan @ 2017-01-12 20:04 UTC (permalink / raw)
  To: Miklos Szeredi, linux-fsdevel; +Cc: linux-kernel, Tahsin Erdogan

fuse_abort_conn() moves requests from pending list to a temporary list
before canceling them. This operation races with request_wait_answer()
which also tries to remove the request after it gets a fatal signal. It
checks FR_PENDING flag to determine whether the request is still in the
pending list.

Make fuse_abort_conn() clear FR_PENDING flag so that request_wait_answer()
does not remove the request from temporary list.

This bug manifests itself as a panic that looks like this:

 general protection fault: 0000 [#1] SMP
 CPU: 2 PID: 1888 Comm: fusexmp Not tainted 4.9.0-rc8+ #47
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs
01/01/2011
 task: ffff88023616a100 task.stack: ffffc90001c20000
 RIP: 0010:[<ffffffff804d5a8f>]  [<ffffffff804d5a8f>]
end_requests+0x5f/0x90
 RSP: 0018:ffffc90001c23b78  EFLAGS: 00010246
 RAX: dead000000000200 RBX: 0000000000000000 RCX: 00000010e447a7df
 RDX: ffff8802331dc000 RSI: ffff8802331dc190 RDI: ffff8802336d7800
 RBP: ffffc90001c23b98 R08: 0000000000000000 R09: 0000000000000000
 R10: ffff880235c2b778 R11: ffff880235a22910 R12: ffff8802331dc190
 R13: ffffc90001c23bc8 R14: ffff8802336d7800 R15: ffff8802331166a0
 FS:  0000000000000000(0000) GS:ffff88023fd00000(0000)
knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000b3c1c8 CR3: 0000000001008000 CR4: 00000000000006e0
 Stack:
  ffff8802336d7800 ffff8802336d7878 ffff8802331166a0 ffff8802336d7800
  ffffc90001c23c00 ffffffff804d77d8 ffff8802336d7d60 ffff8802336d7800
  ffffc90001c23bb8 ffffc90001c23bb8 ffffc90001c23bc8 ffffc90001c23bc8
 Call Trace:
  [<ffffffff804d77d8>] fuse_abort_conn+0x2a8/0x310
  [<ffffffff804d7899>] fuse_dev_release+0x59/0x90
  [<ffffffff80379c7d>] __fput+0x9d/0x1d0
  [<ffffffff80379de9>] ____fput+0x9/0x10
  [<ffffffff8028b51e>] task_work_run+0x7e/0xa0
  [<ffffffff8027510b>] do_exit+0x27b/0xa60
  [<ffffffff8027596a>] do_group_exit+0x3a/0xa0
  [<ffffffff8027f34a>] get_signal+0x1aa/0x5b0
  [<ffffffff802a8400>] ? __wake_up_common+0x80/0x80
  [<ffffffff8021a053>] do_signal+0x23/0x660
  [<ffffffff804d6f8f>] ? fuse_dev_read+0x4f/0x60
  [<ffffffff8026c738>] exit_to_usermode_loop+0x34/0x6b
  [<ffffffff80201565>] syscall_return_slowpath+0x55/0x60
  [<ffffffff80a2d79f>] entry_SYSCALL_64_fastpath+0x92/0x94
 Code: 84 24 9c 00 00 00 99 ff ff ff f0 41 80 64 24 30 7f f0 41 80 64 24
31 fe 49 8b 44 24 08 49 8b 14 24 4c 89 e6 4c 89 f7 48 89 42 08 <48> 89
10 4d 89 24 24 4d 89 64 24 08 e8 d0 fd ff ff 49 8b 45 00
 RIP  [<ffffffff804d5a8f>] end_requests+0x5f/0x90
  RSP <ffffc90001c23b78>
 ---[ end trace 7da3774b682d0b94 ]---

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
 fs/fuse/dev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 70ea57c7b6bb..4e06a27ed7f8 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2025,7 +2025,6 @@ static void end_requests(struct fuse_conn *fc, struct list_head *head)
 		struct fuse_req *req;
 		req = list_entry(head->next, struct fuse_req, list);
 		req->out.h.error = -ECONNABORTED;
-		clear_bit(FR_PENDING, &req->flags);
 		clear_bit(FR_SENT, &req->flags);
 		list_del_init(&req->list);
 		request_end(fc, req);
@@ -2103,6 +2102,8 @@ void fuse_abort_conn(struct fuse_conn *fc)
 		spin_lock(&fiq->waitq.lock);
 		fiq->connected = 0;
 		list_splice_init(&fiq->pending, &to_end2);
+		list_for_each_entry(req, &to_end2, list)
+			clear_bit(FR_PENDING, &req->flags);
 		while (forget_pending(fiq))
 			kfree(dequeue_forget(fiq, 1, NULL));
 		wake_up_all_locked(&fiq->waitq);
-- 
2.11.0.390.gc69c2f50cf-goog

^ permalink raw reply related	[flat|nested] 6+ messages in thread