From: Andrew Deason <adeason@dson.org>
Date: Mon, 18 Nov 2019 02:58:15 +0000 (-0600)
Subject: afs: Ensure CDirty is set during afs_write loop
X-Git-Tag: openafs-devel-1_9_0~195
X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=9d0854547522f7b2fb1bb7aa876fe9f901674747

afs: Ensure CDirty is set during afs_write loop

Currently, in afs_write(), we set CDirty on the given vcache, and then
write the given data into various dcaches. When writing to a dcache,
we call afs_DoPartialWrite, which may cause us to flush the dirty data
to the fileserver and clear the CDirty bit.

If we were given more than 1 chunk of data to write, we will then go
through another iteration of the loop, writing more dirty data into
dcaches, but CDirty will not be set. This can cause issues with, for
example, afs_SimpleVStat() or afs_ProcessFS(), which use CDirty to
determine whether or not to merge in FetchStatus info from the
fileserver into our local cache. This can cause our local cache to
incorrectly reflect the state of the file on the fileserver, instead
of the state of the locally-modified file in our cache.

A more detailed example is as follows. Consider a small C program that
copies a file, fchmod()ing the destination before closing it:

    void
    do_copy(char *src_name, char *dest_name)
    {
        /* error checking elided */
        src_fd = open(src_name, O_RDONLY);
        dest_fd = open(dest_name, O_WRONLY|O_CREAT|O_TRUNC, 0755);
        fstat(src_fd, &st);
        src_buf = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, src_fd, 0);
        write(dest_fd, src_buf, st.st_size);
        munmap(src_buf, st.st_size);
        close(src_fd);
        fchmod(dest_fd, 0100644);
        close(dest_fd);
    }

Currently, on FBSD, using this to copy a 7862648-byte file, using a
smallish cache (10000 blocks) will cause the destination to appear to
be truncated, because avc->f.m.Length will be incorrect, even though
all of the relevant data was written to the fileserver.

On most other platforms such as SOLARIS and LINUX, this is not a
problem, since currently they only write one page of data at a time to
afs_write(), and so they never hit multiple iterations of the while()
loop inside afs_write().

To fix this, just set CDirty on every iteration of the while() loop in
afs_write(). In general, we need to set CDirty after calling
afs_DoPartialStore() anywhere if the caller continues to write more
data. But all callers already do this, except for this one instance in
afs_write().

Thanks to tcreech@tcreech.com for helping find occurrences of the
relevant issue.

FIXES 135041

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

diff --git a/src/afs/VNOPS/afs_vnop_write.c b/src/afs/VNOPS/afs_vnop_write.c
index 1c203ce..c720f6a 100644
--- a/src/afs/VNOPS/afs_vnop_write.c
+++ b/src/afs/VNOPS/afs_vnop_write.c
@@ -302,9 +302,16 @@ afs_write(struct vcache *avc, struct uio *auio, int aio,
 #else
     afs_FakeOpen(avc);
 #endif
-    avc->f.states |= CDirty;
 
     while (totalLength > 0) {
+	/*
+	 * Note that we must set CDirty for every iteration of this loop.
+	 * CDirty may get cleared below (such as during afs_DoPartialStore),
+	 * but we're still writing to the file, so make sure CDirty is set
+	 * here.
+	 */
+	avc->f.states |= CDirty;
+
 	tdc = afs_ObtainDCacheForWriting(avc, filePos, totalLength, treq,
 					 noLock);
 	if (!tdc) {