afs: Fix rename operation status delivery
authorDavid Howells <dhowells@redhat.com>
Wed, 8 Apr 2020 19:56:20 +0000 (20:56 +0100)
committerDavid Howells <dhowells@redhat.com>
Mon, 13 Apr 2020 14:09:01 +0000 (15:09 +0100)
The afs_deliver_fs_rename() and yfs_deliver_fs_rename() functions both only
decode the second file status returned unless the parent directories are
different - unfortunately, this means that the xdr pointer isn't advanced
and the volsync record will be read incorrectly in such an instance.

Fix this by always decoding the second status into the second
status/callback block which wasn't being used if the dirs were the same.

The afs_update_dentry_version() calls that update the directory data
version numbers on the dentries can then unconditionally use the second
status record as this will always reflect the state of the destination dir
(the two records will be identical if the destination dir is the same as
the source dir)

Fixes: 260a980317da ("[AFS]: Add "directory write" support.")
Fixes: 30062bd13e36 ("afs: Implement YFS support in the fs client")
Signed-off-by: David Howells <dhowells@redhat.com>
fs/afs/dir.c
fs/afs/fsclient.c
fs/afs/yfsclient.c

index 5c794f4b051afcc953f97ceff3b562402a31b4e0..31d297e0f76554da7e2556111c3e37b196fca029 100644 (file)
@@ -1892,7 +1892,6 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
        if (afs_begin_vnode_operation(&fc, orig_dvnode, key, true)) {
                afs_dataversion_t orig_data_version;
                afs_dataversion_t new_data_version;
-               struct afs_status_cb *new_scb = &scb[1];
 
                orig_data_version = orig_dvnode->status.data_version + 1;
 
@@ -1904,7 +1903,6 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
                        new_data_version = new_dvnode->status.data_version + 1;
                } else {
                        new_data_version = orig_data_version;
-                       new_scb = &scb[0];
                }
 
                while (afs_select_fileserver(&fc)) {
@@ -1912,7 +1910,7 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
                        fc.cb_break_2 = afs_calc_vnode_cb_break(new_dvnode);
                        afs_fs_rename(&fc, old_dentry->d_name.name,
                                      new_dvnode, new_dentry->d_name.name,
-                                     &scb[0], new_scb);
+                                     &scb[0], &scb[1]);
                }
 
                afs_vnode_commit_status(&fc, orig_dvnode, fc.cb_break,
@@ -1957,13 +1955,8 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
                 * Note that if we ever implement RENAME_EXCHANGE, we'll have
                 * to update both dentries with opposing dir versions.
                 */
-               if (new_dvnode != orig_dvnode) {
-                       afs_update_dentry_version(&fc, old_dentry, &scb[1]);
-                       afs_update_dentry_version(&fc, new_dentry, &scb[1]);
-               } else {
-                       afs_update_dentry_version(&fc, old_dentry, &scb[0]);
-                       afs_update_dentry_version(&fc, new_dentry, &scb[0]);
-               }
+               afs_update_dentry_version(&fc, old_dentry, &scb[1]);
+               afs_update_dentry_version(&fc, new_dentry, &scb[1]);
                d_move(old_dentry, new_dentry);
                goto error_tmp;
        }
index de4331670c84a9f027b8fc5bbb2a09f27e5e32ea..68fc46634346a13270497bbcf87cfb0d99340d02 100644 (file)
@@ -986,16 +986,16 @@ static int afs_deliver_fs_rename(struct afs_call *call)
        if (ret < 0)
                return ret;
 
-       /* unmarshall the reply once we've received all of it */
+       /* If the two dirs are the same, we have two copies of the same status
+        * report, so we just decode it twice.
+        */
        bp = call->buffer;
        ret = xdr_decode_AFSFetchStatus(&bp, call, call->out_dir_scb);
        if (ret < 0)
                return ret;
-       if (call->out_dir_scb != call->out_scb) {
-               ret = xdr_decode_AFSFetchStatus(&bp, call, call->out_scb);
-               if (ret < 0)
-                       return ret;
-       }
+       ret = xdr_decode_AFSFetchStatus(&bp, call, call->out_scb);
+       if (ret < 0)
+               return ret;
        xdr_decode_AFSVolSync(&bp, call->out_volsync);
 
        _leave(" = 0 [done]");
index a0f7c3186645eafb404c1f1f7689d1a3e36022ec..83b6d67325f6cc120830ac9fcfb0cc51a39d36dc 100644 (file)
@@ -1157,11 +1157,9 @@ static int yfs_deliver_fs_rename(struct afs_call *call)
        ret = xdr_decode_YFSFetchStatus(&bp, call, call->out_dir_scb);
        if (ret < 0)
                return ret;
-       if (call->out_dir_scb != call->out_scb) {
-               ret = xdr_decode_YFSFetchStatus(&bp, call, call->out_scb);
-               if (ret < 0)
-                       return ret;
-       }
+       ret = xdr_decode_YFSFetchStatus(&bp, call, call->out_scb);
+       if (ret < 0)
+               return ret;
 
        xdr_decode_YFSVolSync(&bp, call->out_volsync);
        _leave(" = 0 [done]");