[Devel] [PATCH] fs/fuse kio: fix stack overrun in request_end()

[Kirill Tkhai]

On 31.05.2019 12:30, Kirill Tkhai wrote:
> On 31.05.2019 12:28, Pavel Butsykin wrote:
>>
>>
>> On 31.05.2019 12:16, Kirill Tkhai wrote:
>>> On 31.05.2019 11:39, Pavel Butsykin wrote:
>>>> Unexpectedly, request_end() function turned out to be recursive, which leads to
>>>> stack overrun:
>>>> [76293.902783]  [<ffffffffc0faa4ff>] request_end+0x334/0x3cf [fuse]
>>>> [76293.902789]  [<ffffffffc0fdc9d3>] pcs_fuse_submit+0x49d/0x4f5 [fuse_kio_pcs]
>>>> [76293.902795]  [<ffffffffc0fbd775>] ? fuse_flush_writepages+0x8f/0x8f [fuse]
>>>> [76293.902800]  [<ffffffffc0fdd745>] kpcs_req_send+0x210/0x3bb [fuse_kio_pcs]
>>>> [76293.902805]  [<ffffffffc0faa17b>] flush_bg_queue+0x1ba/0x20a [fuse]
>>>> [76293.902810]  [<ffffffffc0faa4ff>] request_end+0x334/0x3cf [fuse]
>>>> [76293.902816]  [<ffffffffc0fdc9d3>] pcs_fuse_submit+0x49d/0x4f5 [fuse_kio_pcs]
>>>> [76293.902818]  [<ffffffff8c57cdd0>] ? debug_check_no_locks_freed+0x2a0/0x2a0
>>>> [76293.902824]  [<ffffffffc0fbd775>] ? fuse_flush_writepages+0x8f/0x8f [fuse]
>>>> [76293.902829]  [<ffffffffc0fdd745>] kpcs_req_send+0x210/0x3bb [fuse_kio_pcs]
>>>> [76293.902834]  [<ffffffffc0faa17b>] flush_bg_queue+0x1ba/0x20a [fuse]
>>>> [76293.902839]  [<ffffffffc0faa4ff>] request_end+0x334/0x3cf [fuse]
>>>>
>>>> To fix this, let's call request_end() inside kpcs_req_send() synchronously for
>>>> background requests.
>>>
>>> Oh. I'm not sure this complexity is worth its payload.
>>
>> I just wanted to avoid modification of fuse code. But probably the price is too
>> expensive.
> 
> The problem is this gives also synchronization complexity like we need to think
> whether we should care about flushing pcs_dentry_info::work on inode destroy.

Also, we may do something like the below to not touch generic code at all:

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index b8a524f04298..e8441aaf3903 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -396,9 +396,13 @@ static void flush_bg_queue(struct fuse_conn *fc, struct fuse_iqueue *fiq)
 	struct fuse_req *req, *next;
 	LIST_HEAD(kio_reqs);
 
+	if (fc->flush_bg_queue_nesting)
+		return;
+	fc->flush_bg_queue_nesting++;
+
 	while (fc->active_background < fc->max_background &&
 	       !list_empty(&fc->bg_queue)) {
-
+again:
 		req = list_first_entry(&fc->bg_queue, struct fuse_req, list);
 		list_del_init(&req->list);
 		fc->active_background++;
@@ -424,6 +428,12 @@ static void flush_bg_queue(struct fuse_conn *fc, struct fuse_iqueue *fiq)
 		fc->kio.op->req_send(fc, req, NULL, true);
 	}
 	spin_lock(&fc->bg_lock);
+
+	if (fc->active_background < fc->max_background &&
+	       !list_empty(&fc->bg_queue))
+		goto again;
+
+	fc->flush_bg_queue_nesting--;
 }
 
 /*