afs: Fix EOF corruption
authorDavid Howells <dhowells@redhat.com>
Fri, 12 Jun 2020 23:03:48 +0000 (00:03 +0100)
committerDavid Howells <dhowells@redhat.com>
Mon, 15 Jun 2020 14:41:02 +0000 (15:41 +0100)
When doing a partial writeback, afs_write_back_from_locked_page() may
generate an FS.StoreData RPC request that writes out part of a file when a
file has been constructed from pieces by doing seek, write, seek, write,
... as is done by ld.

The FS.StoreData RPC is given the current i_size as the file length, but
the server basically ignores it unless the data length is 0 (in which case
it's just a truncate operation).  The revised file length returned in the
result of the RPC may then not reflect what we suggested - and this leads
to i_size getting moved backwards - which causes issues later.

Fix the client to take account of this by ignoring the returned file size
unless the data version number jumped unexpectedly - in which case we're
going to have to clear the pagecache and reload anyway.

This can be observed when doing a kernel build on an AFS mount.  The
following pair of commands produce the issue:

  ld -m elf_x86_64 -z max-page-size=0x200000 --emit-relocs \
      -T arch/x86/realmode/rm/realmode.lds \
      arch/x86/realmode/rm/header.o \
      arch/x86/realmode/rm/trampoline_64.o \
      arch/x86/realmode/rm/stack.o \
      arch/x86/realmode/rm/reboot.o \
      -o arch/x86/realmode/rm/realmode.elf
  arch/x86/tools/relocs --realmode \
      arch/x86/realmode/rm/realmode.elf \
      >arch/x86/realmode/rm/realmode.relocs

This results in the latter giving:

Cannot read ELF section headers 0/18: Success

as the realmode.elf file got corrupted.

The sequence of events can also be driven with:

xfs_io -t -f \
-c "pwrite -S 0x58 0 0x58" \
-c "pwrite -S 0x59 10000 1000" \
-c "close" \
/afs/example.com/scratch/a

Fixes: 31143d5d515e ("AFS: implement basic file write support")
Signed-off-by: David Howells <dhowells@redhat.com>
fs/afs/inode.c

index cd0a006..8d10bfb 100644 (file)
@@ -168,6 +168,7 @@ static void afs_apply_status(struct afs_operation *op,
        struct timespec64 t;
        umode_t mode;
        bool data_changed = false;
+       bool change_size = false;
 
        _enter("{%llx:%llu.%u} %s",
               vp->fid.vid, vp->fid.vnode, vp->fid.unique,
@@ -226,6 +227,7 @@ static void afs_apply_status(struct afs_operation *op,
                } else {
                        set_bit(AFS_VNODE_ZAP_DATA, &vnode->flags);
                }
+               change_size = true;
        } else if (vnode->status.type == AFS_FTYPE_DIR) {
                /* Expected directory change is handled elsewhere so
                 * that we can locally edit the directory and save on a
@@ -233,11 +235,19 @@ static void afs_apply_status(struct afs_operation *op,
                 */
                if (test_bit(AFS_VNODE_DIR_VALID, &vnode->flags))
                        data_changed = false;
+               change_size = true;
        }
 
        if (data_changed) {
                inode_set_iversion_raw(&vnode->vfs_inode, status->data_version);
-               afs_set_i_size(vnode, status->size);
+
+               /* Only update the size if the data version jumped.  If the
+                * file is being modified locally, then we might have our own
+                * idea of what the size should be that's not the same as
+                * what's on the server.
+                */
+               if (change_size)
+                       afs_set_i_size(vnode, status->size);
        }
 }