Hi, I still face, with 12.1-RELEASE-p2, the issue discussed here : https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=230258#c35 Which continued there : https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=235774 The issue is the following. Let's assume raw file is the underlying file. And fuse file the corresponding fuse file. 1. create raw file with some data 2. open fuse file 3. verify fuse size : correct 4. verify fuse data : correct 5. add some data to raw file 6. verify fuse size : correct (*) 7. verify fuse data : garbage (*) as expected we have to use fuse options attr_timeout=0 entry_timeout=0. So, if we keep the fuse file opened, while the underlying one is modified : - we read the fuse file up to the expected size, perfect ; - but we read garbage. If we close and re-open the fuse file, then we correctly read it. There are 2 possible workarounds : - use FUSE direct_io mount option ; - sysctl vfs.fusefs.data_cache_mode = 0. I don't know whether or not the default behavior is correct. But at least with Linux, we correctly read the data even keeping the fuse file opened. There may then still be a cache issue. Many thanks for your feedback !
In step 5, are you extending the underlying file from EOF onwards, or are you rewriting the whole thing? And in step 7, is the data correct up until the old EOF, or is it all wrong? Do you have a standalone test case that can demonstrate the problem? It sounds to me like the correct answer might be for the fuse server to return FOPEN_DIRECT_IO for every FUSE_OPEN request. That would prevent the kernel from using the cache at all.
Thank you for your prompt response Alan. Whatever the file is extended or rewriten in step 5, the data which has been read in step 4 is returned in step 7, up to the old EOF. Below is a standone Perl testcase, the last one failing from the fusefs-encfs test suite. It asumes you mount a 1:1 fuse FS from /tmp/raw to /tmp/fuse, mounted with -o attr_timeout=0 (entry_timeout=0 is useless here, btw not sure what it's useful for). If you don't have such a 1:1 fuse FS, you can use EncFS in insecure (i.e. 1:1) mode : pkg install fusefs-encfs mkdir -p /tmp/raw /tmp/fuse encfs --insecure -o attr_timeout=0 --nodatacache /tmp/raw /tmp/fuse # choose x mode, don't care about encryption numbers, disable file data encryption and choose null / no encryption of filenames. Below Perl test case will then fail, until you use one of the 2 workarounds given above. Or until you enable the in-loop open/close of the fuse file (and disable the global one). Of course feel free if needed, thx ! ### Standalone Perl test case : # Get the file size from stat() sub statSize { my $f = shift; my @s = stat($f) or die("stat on '$f' failed"); return $s[7]; } # Get the file size by read() sub readSize { my $fh = shift; seek($fh, 0, 0); my $block = 4*1024; my $s; my $data; my $sum = read($fh, $data, $block); while ($s += read($fh, $data, $block)) { $sum += $s; } return $sum; } # Verify that the file size matches the target (both stat() & read()) sub sizeVerify { my $ok = 1; my $fh = shift; my $s0 = shift; $ss = statSize($fh); if ($s0 != $ss) { $ok = 0; print("# stat size $ss, expected $s0\n"); } $sr = readSize($fh); if ($s0 != $sr) { $ok = 0; print("# read size $sr, expected $s0\n"); } return $ok; } open(my $rfh, ">", "/tmp/raw/grow"); $rfh->autoflush; open(my $vfh, "<", "/tmp/raw/grow"); open(my $ffh, "<", "/tmp/fuse/grow"); for($i=5; $i < 80; $i += 5) { #open(my $ffh, "<", "/tmp/fuse/grow"); printf($rfh "%04d.", $i) or die("write failed"); sizeVerify($vfh, $i) or die("unexpected raw file size"); sizeVerify($ffh, $i) or die("unexpected fuse file size"); print "size:$i\n"; seek($vfh, 0, 0); seek($ffh, 0, 0); print "raw :".<$vfh>."\n"; print "fuse:".<$ffh>."\n---\n"; #close($ffh); } ### Failing result : size:5 raw :0005. fuse:0005. --- size:10 raw :0005.0010. fuse:0005. --- size:15 raw :0005.0010.0015. fuse:0005. --- size:20 raw :0005.0010.0015.0020. fuse:0005. --- size:25 raw :0005.0010.0015.0020.0025. fuse:0005. --- size:30 raw :0005.0010.0015.0020.0025.0030. fuse:0005. --- size:35 raw :0005.0010.0015.0020.0025.0030.0035. fuse:0005. --- size:40 raw :0005.0010.0015.0020.0025.0030.0035.0040. fuse:0005. --- size:45 raw :0005.0010.0015.0020.0025.0030.0035.0040.0045. fuse:0005. --- size:50 raw :0005.0010.0015.0020.0025.0030.0035.0040.0045.0050. fuse:0005. --- size:55 raw :0005.0010.0015.0020.0025.0030.0035.0040.0045.0050.0055. fuse:0005. --- size:60 raw :0005.0010.0015.0020.0025.0030.0035.0040.0045.0050.0055.0060. fuse:0005. --- size:65 raw :0005.0010.0015.0020.0025.0030.0035.0040.0045.0050.0055.0060.0065. fuse:0005. --- size:70 raw :0005.0010.0015.0020.0025.0030.0035.0040.0045.0050.0055.0060.0065.0070. fuse:0005. --- size:75 raw :0005.0010.0015.0020.0025.0030.0035.0040.0045.0050.0055.0060.0065.0070.0075. fuse:0005. ---
So there are two things going on here: 1) When fusefs discovers that a file's cached size is invalid, it's supposed to fix the size and discard any invalid cached buffers. It looks like we're doing that correctly for truncations and for large extensions (I'm still trying to figure out how large "large" is), but as you found not for small extensions. 2) The test simulates a buggy fuse server. If it's ever possible for the file system's data to change behind the kernel's back, then it the server MUST use direct_io. The kernel is using a heuristic to detect this behind-the-back behavior, but what if the data changes yet the file's length stays the same? The heuristic can't catch that. The situation is somewhat better with newer FUSE protocol versions that have an asynchronous cache invalidation mechanism, but even then it's racy. This is probably why nobody has reported the bug yet; well-behaved fuse servers don't trigger it.
A little bit of context, there are some use-cases where it is expected that raw/underlying file could be modified. fusefs-encfs has a so-called "reverse" mode. It gives you, through fuse, an encrypted view of your plain raw/underlying files. You can then for example sync your on-the-fly encrypted files to a remote untrusted storage location. Even if it's not to expect (as the result may rather look strange), a raw/underlying file could be modified while it's read through fuse. Coming back to your first point, glad to see we may have found thanks to this a remaining cache issue. And regarding the second point, you're certainly right that fuse server should enforce direct_io in such use cases. I'll then certainly modify fusefs-encfs accordingly once this ticket will be "solved", so that we'll have the full story / it'll be consistent.
Created attachment 212269 [details] invalidate cache when a file changes unexpectedly Ben, please test the attached patch. It causes fusefs to invalidate a file's cache when it detects that a buggy server has modified the file.
Alan, thank you very much for this patch, I just tested it and it works as expected : when the underlying file size is modified, cache is correctly invalidated. The attached test case then now correctly runs. Perfect. Of course, as you already stated above, if the data changes but the file's length stays the same, then the patch does not help, and we then should use DIRECT_IO. Thank you again !
Code review in progress at https://reviews.freebsd.org/D24012
A commit references this bug: Author: asomers Date: Wed Mar 11 04:29:47 UTC 2020 New revision: 358867 URL: https://svnweb.freebsd.org/changeset/base/358867 Log: fusefs: avoid cache corruption with buggy fuse servers The FUSE protocol allows the client (kernel) to cache a file's size, if the server (userspace daemon) allows it. A well-behaved daemon obviously should not change a file's size while a client has it cached. But a buggy daemon might. If the kernel ever detects that that has happened, then it should invalidate the entire cache for that file. Previously, we would not only cache stale data, but in the case of a file extension while we had the size cached, we accidentally extended the cache with zeros. PR: 244178 Reported by: Ben RUBSON <ben.rubson@gmx.com> Reviewed by: cem MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D24012 Changes: head/sys/fs/fuse/fuse_internal.c head/sys/fs/fuse/fuse_node.c head/sys/fs/fuse/fuse_node.h head/sys/fs/fuse/fuse_vnops.c head/tests/sys/fs/fusefs/Makefile head/tests/sys/fs/fusefs/cache.cc head/tests/sys/fs/fusefs/getattr.cc head/tests/sys/fs/fusefs/io.cc head/tests/sys/fs/fusefs/utils.cc head/tests/sys/fs/fusefs/utils.hh
A commit references this bug: Author: asomers Date: Sun Mar 22 15:24:27 UTC 2020 New revision: 359214 URL: https://svnweb.freebsd.org/changeset/base/359214 Log: MFC r358867: fusefs: avoid cache corruption with buggy fuse servers The FUSE protocol allows the client (kernel) to cache a file's size, if the server (userspace daemon) allows it. A well-behaved daemon obviously should not change a file's size while a client has it cached. But a buggy daemon might. If the kernel ever detects that that has happened, then it should invalidate the entire cache for that file. Previously, we would not only cache stale data, but in the case of a file extension while we had the size cached, we accidentally extended the cache with zeros. PR: 244178 Reported by: Ben RUBSON <ben.rubson@gmx.com> Reviewed by: cem Differential Revision: https://reviews.freebsd.org/D24012 Changes: _U stable/12/ stable/12/sys/fs/fuse/fuse_internal.c stable/12/sys/fs/fuse/fuse_node.c stable/12/sys/fs/fuse/fuse_node.h stable/12/sys/fs/fuse/fuse_vnops.c stable/12/tests/sys/fs/fusefs/Makefile stable/12/tests/sys/fs/fusefs/cache.cc stable/12/tests/sys/fs/fusefs/getattr.cc stable/12/tests/sys/fs/fusefs/io.cc stable/12/tests/sys/fs/fusefs/utils.cc stable/12/tests/sys/fs/fusefs/utils.hh