]>
Commit | Line | Data |
---|---|---|
cd033818 SL |
1 | From cab2e5096300bcb12ce3b04f8e5e7c0538fde023 Mon Sep 17 00:00:00 2001 |
2 | From: Kirill Smelkov <kirr@nexedi.com> | |
3 | Date: Wed, 27 Mar 2019 10:15:15 +0000 | |
4 | Subject: fuse: require /dev/fuse reads to have enough buffer capacity | |
5 | ||
6 | [ Upstream commit d4b13963f217dd947da5c0cabd1569e914d21699 ] | |
7 | ||
8 | A FUSE filesystem server queues /dev/fuse sys_read calls to get | |
9 | filesystem requests to handle. It does not know in advance what would be | |
10 | that request as it can be anything that client issues - LOOKUP, READ, | |
11 | WRITE, ... Many requests are short and retrieve data from the | |
12 | filesystem. However WRITE and NOTIFY_REPLY write data into filesystem. | |
13 | ||
14 | Before getting into operation phase, FUSE filesystem server and kernel | |
15 | client negotiate what should be the maximum write size the client will | |
16 | ever issue. After negotiation the contract in between server/client is | |
17 | that the filesystem server then should queue /dev/fuse sys_read calls with | |
18 | enough buffer capacity to receive any client request - WRITE in | |
19 | particular, while FUSE client should not, in particular, send WRITE | |
20 | requests with > negotiated max_write payload. FUSE client in kernel and | |
21 | libfuse historically reserve 4K for request header. This way the | |
22 | contract is that filesystem server should queue sys_reads with | |
23 | 4K+max_write buffer. | |
24 | ||
25 | If the filesystem server does not follow this contract, what can happen | |
26 | is that fuse_dev_do_read will see that request size is > buffer size, | |
27 | and then it will return EIO to client who issued the request but won't | |
28 | indicate in any way that there is a problem to filesystem server. | |
29 | This can be hard to diagnose because for some requests, e.g. for | |
30 | NOTIFY_REPLY which mimics WRITE, there is no client thread that is | |
31 | waiting for request completion and that EIO goes nowhere, while on | |
32 | filesystem server side things look like the kernel is not replying back | |
33 | after successful NOTIFY_RETRIEVE request made by the server. | |
34 | ||
35 | We can make the problem easy to diagnose if we indicate via error return to | |
36 | filesystem server when it is violating the contract. This should not | |
37 | practically cause problems because if a filesystem server is using shorter | |
38 | buffer, writes to it were already very likely to cause EIO, and if the | |
39 | filesystem is read-only it should be too following FUSE_MIN_READ_BUFFER | |
40 | minimum buffer size. | |
41 | ||
42 | Please see [1] for context where the problem of stuck filesystem was hit | |
43 | for real (because kernel client was incorrectly sending more than | |
44 | max_write data with NOTIFY_REPLY; see also previous patch), how the | |
45 | situation was traced and for more involving patch that did not make it | |
46 | into the tree. | |
47 | ||
48 | [1] https://marc.info/?l=linux-fsdevel&m=155057023600853&w=2 | |
49 | ||
50 | Signed-off-by: Kirill Smelkov <kirr@nexedi.com> | |
51 | Cc: Han-Wen Nienhuys <hanwen@google.com> | |
52 | Cc: Jakob Unterwurzacher <jakobunt@gmail.com> | |
53 | Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> | |
54 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
55 | --- | |
56 | fs/fuse/dev.c | 10 ++++++++++ | |
57 | 1 file changed, 10 insertions(+) | |
58 | ||
59 | diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c | |
60 | index 341196338e48..fbb978e75c6b 100644 | |
61 | --- a/fs/fuse/dev.c | |
62 | +++ b/fs/fuse/dev.c | |
63 | @@ -1265,6 +1265,16 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file, | |
64 | struct fuse_in *in; | |
65 | unsigned reqsize; | |
66 | ||
67 | + /* | |
68 | + * Require sane minimum read buffer - that has capacity for fixed part | |
69 | + * of any request header + negotated max_write room for data. If the | |
70 | + * requirement is not satisfied return EINVAL to the filesystem server | |
71 | + * to indicate that it is not following FUSE server/client contract. | |
72 | + * Don't dequeue / abort any request. | |
73 | + */ | |
74 | + if (nbytes < max_t(size_t, FUSE_MIN_READ_BUFFER, 4096 + fc->max_write)) | |
75 | + return -EINVAL; | |
76 | + | |
77 | restart: | |
78 | spin_lock(&fiq->waitq.lock); | |
79 | err = -EAGAIN; | |
80 | -- | |
81 | 2.20.1 | |
82 |