Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
        Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
        Kingdom.
        Registered in England and Wales under Company Registration No. 3798903
From:   David Howells <dhowells@redhat.com>
To:     Marc Dionne <marc.dionne@auristor.com>
cc:     dhowells@redhat.com, linux-afs@lists.infradead.org,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] afs: Fix accidental truncation when storing data
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3526894.1687960024.1@warthog.procyon.org.uk>
Content-Transfer-Encoding: quoted-printable
Date:   Wed, 28 Jun 2023 14:47:04 +0100
Message-ID: <3526895.1687960024@warthog.procyon.org.uk>
Precedence: bulk

    =

When an AFS FS.StoreData RPC call is made, amongst other things it is give=
n
the resultant file size to be.  On the server, this is processed by
truncating the file to new size and then writing the data.

Now, kafs has a lock (vnode->io_lock) that serves to serialise operations
against a specific vnode (ie. inode), but the parameters for the op are se=
t
before the lock is taken.  This allows two writebacks (say sync and kswapd=
)
to race - and if writes are ongoing the writeback for a later write could
occur before the writeback for an earlier one if the latter gets
interrupted.

Note that afs_writepages() cannot take i_mutex and only takes a shared loc=
k
on vnode->validate_lock.

Also note that the server does the truncation and the write inside a lock,
so there's no problem at that end.

Fix this by moving the calculation for the proposed new i_size inside the
vnode->io_lock.  Also reset the iterator (which we might have read from)
and update the mtime setting there.

Fixes: bd80d8a80e12 ("afs: Use ITER_XARRAY for writing")
Reported-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-afs@lists.infradead.org
cc: linux-fsdevel@vger.kernel.org
---
 fs/afs/write.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/afs/write.c b/fs/afs/write.c
index 8750b99c3f56..c1f4391ccd7c 100644
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -413,17 +413,19 @@ static int afs_store_data(struct afs_vnode *vnode, s=
truct iov_iter *iter, loff_t
 	afs_op_set_vnode(op, 0, vnode);
 	op->file[0].dv_delta =3D 1;
 	op->file[0].modification =3D true;
-	op->store.write_iter =3D iter;
 	op->store.pos =3D pos;
 	op->store.size =3D size;
-	op->store.i_size =3D max(pos + size, vnode->netfs.remote_i_size);
 	op->store.laundering =3D laundering;
-	op->mtime =3D vnode->netfs.inode.i_mtime;
 	op->flags |=3D AFS_OPERATION_UNINTR;
 	op->ops =3D &afs_store_data_operation;
 =

 try_next_key:
 	afs_begin_vnode_operation(op);
+
+	op->store.write_iter =3D iter;
+	op->store.i_size =3D max(pos + size, vnode->netfs.remote_i_size);
+	op->mtime =3D vnode->netfs.inode.i_mtime;
+
 	afs_wait_for_operation(op);
 =

 	switch (op->error) {