afs: Skip checking chunkBytes sanity for RW files 69/13969/2
authorAndrew Deason <adeason@dson.org>
Fri, 29 Nov 2019 17:42:47 +0000 (11:42 -0600)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 29 Nov 2019 19:27:33 +0000 (14:27 -0500)
Currently, the IsDCacheSizeOK check can trigger a false positive for a
dcache, if the data in the dcache was populated by a local write to a
file that was later extended with sparse data.

For example: say a client opens a new file, and writes 4 bytes to
offset 0, and then writes 4 bytes to offset 0x400000. After the first
write, the first chunk for the file will contain just 4 bytes, and
after the second write, the first chunk is unchanged (since we're
writing to a different area of the file), but the file is now 0x400004
bytes long. The sparse area of the file will be correctly filled with
zeroes for local reads and on the fileserver, but the 4-byte chunk
causes IsDCacheSizeOK to complain and mark the dcache as invalid.

Even though nothing is wrong, this causes the following scary messages
to potentially appear in the kernel log, and the relevant dcache to be
invalidated:

    afs: Detected corrupt dcache for file 1.536870913.2.2: chunk 0 (offset 0) has 4 bytes, but it should have 131072 bytes
    afs: (dcache 0xfffffdeadbeefb4d, file length 4194308, DV 1, dcache mtime 1575049956, index 996, dflags 0x2, mflags 0x0, states 0x4, vcache states 0x1)
    afs: Ignoring the dcache for now, but this may indicate corruption in the AFS cache, or a bug.

It's probably difficult or impossible to detect if this specific case
is happening, so to avoid this scenario, just avoid doing the size
check at all for RW data from the cache.

Change-Id: Ia40ec838c525d9abc13a03be39028e4ca04a9457
Reviewed-on: https://gerrit.openafs.org/13969
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

src/afs/afs_dcache.c

index 2f54bf7..ef32feb 100644 (file)
@@ -1773,6 +1773,29 @@ IsDCacheSizeOK(struct dcache *adc, struct vcache *avc, afs_int32 chunk_bytes,
        return 1;
     }
 
+    if (!from_net && (adc->f.states & DRW)) {
+       /*
+        * The dcache data we're looking at is from our local cache (not from a
+        * fileserver), and it's for data in an RW volume. For cached RW data,
+        * there are some edge cases that can cause the below length checks to
+        * trigger false positives.
+        *
+        * For example: if the local client writes 4 bytes to a new file at
+        * offset 0, and then 4 bytes at offset 0x400000, the file will be
+        * 0x400004 bytes long, but the first dcache chunk will only contain 4
+        * bytes. If such a file is fetched from a fileserver, the first chunk
+        * will have a full chunk of data (most of it zeroes), but on the
+        * client that did the write, the sparse data will not appear in the
+        * dcache.
+        *
+        * Such false positives should only be possible with RW data, since
+        * non-RW data is never generated locally. So to avoid the false
+        * positives, assume the dcache length is OK for RW data if the dcache
+        * came from our local cache (and not directly from a fileserver).
+        */
+       return 1;
+    }
+
     if (file_length < chunk_start) {
        expected_bytes = 0;