• Jeff Layton's avatar
    nfs: don't extend writes to cover entire page if pagecache is invalid · 81d9bce5
    Jeff Layton authored
    Jian reported that the following sequence would leave "testfile" with
    corrupt data:
    
        # mount localhost:/export /mnt/nfs/ -o vers=3
        # echo abc > /mnt/nfs/testfile; echo def >> /export/testfile; echo ghi >> /mnt/nfs/testfile
        # cat -v /export/testfile
        abc
        ^@^@^@^@ghi
    
    While there's no locking involved here, the operations are serialized,
    so CTO should prevent corruption.
    
    The first write to the file is fine and writes 4 bytes. The file is then
    extended on the server. When it's reopened a GETATTR is issued and the
    size change is noticed. This causes NFS_INO_INVALID_DATA to be set on
    the file. Because the file is opened for write only,
    nfs_want_read_modify_write() returns 0 to nfs_write_begin().
    nfs_updatepage then calls nfs_write_pageuptodate() to see if it should
    extend the nfs_page to cover the whole page. NFS_INO_INVALID_DATA is
    still set on the file at that point, but that flag is ignored and
    nfs_pageuptodate erroneously extends the write to cover the whole page,
    with the write done on the server side filled in with zeroes.
    
    This patch just has that function check for NFS_INO_INVALID_DATA in
    addition to NFS_INO_REVAL_PAGECACHE. This fixes the bug, but looking
    over the code, I wonder if we might have a similar bug in
    nfs_revalidate_size(). The difference between those two flags is very
    subtle, so it seems like we ought to be checking for
    NFS_INO_INVALID_DATA in most of the places that we look for
    NFS_INO_REVAL_PAGECACHE.
    
    I believe this is regression introduced by commit 8d197a56. The code
    did check for NFS_INO_INVALID_DATA prior to that patch.
    
    Original bug report is here:
    
        https://bugzilla.redhat.com/show_bug.cgi?id=885743
    
    Cc: <stable@vger.kernel.org> # 3.5+
    Reported-by: default avatarJian Li <jiali@redhat.com>
    Signed-off-by: default avatarJeff Layton <jlayton@redhat.com>
    Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
    81d9bce5
write.c 48 KB