Skip to content

Commit

Permalink
Merge tag 'afs-fixes-20200616' of git://git.kernel.org/pub/scm/linux/…
Browse files Browse the repository at this point in the history
…kernel/git/dhowells/linux-fs

Pull AFS fixes from David Howells:
 "I've managed to get xfstests kind of working with afs. Here are a set
  of patches that fix most of the bugs found.

  There are a number of primary issues:

   - Incorrect handling of mtime and non-handling of ctime. It might be
     argued, that the latter isn't a bug since the AFS protocol doesn't
     support ctime, but I should probably still update it locally.

   - Shared-write mmap, truncate and writeback bugs. This includes not
     changing i_size under the callback lock, overwriting local i_size
     with the reply from the server after a partial writeback, not
     limiting the writeback from an mmapped page to EOF.

   - Checks for an abort code indicating that the primary vnode in an
     operation was deleted by a third-party are done in the wrong place.

   - Silly rename bugs. This includes an incomplete conversion to the
     new operation handling, duplicate nlink handling, nlink changing
     not being done inside the callback lock and insufficient handling
     of third-party conflicting directory changes.

  And some secondary ones:

   - The UAEOVERFLOW abort code should map to EOVERFLOW not EREMOTEIO.

   - Remove a couple of unused or incompletely used bits.

   - Remove a couple of redundant success checks.

  These seem to fix all the data-corruption bugs found by

	./check -afs -g quick

  along with the obvious silly rename bugs and time bugs.

  There are still some test failures, but they seem to fall into two
  classes: firstly, the authentication/security model is different to
  the standard UNIX model and permission is arbitrated by the server and
  cached locally; and secondly, there are a number of features that AFS
  does not support (such as mknod). But in these cases, the tests
  themselves need to be adapted or skipped.

  Using the in-kernel afs client with xfstests also found a bug in the
  AuriStor AFS server that has been fixed for a future release"

* tag 'afs-fixes-20200616' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs:
  afs: Fix silly rename
  afs: afs_vnode_commit_status() doesn't need to check the RPC error
  afs: Fix use of afs_check_for_remote_deletion()
  afs: Remove afs_operation::abort_code
  afs: Fix yfs_fs_fetch_status() to honour vnode selector
  afs: Remove yfs_fs_fetch_file_status() as it's not used
  afs: Fix the mapping of the UAEOVERFLOW abort code
  afs: Fix truncation issues and mmap writeback size
  afs: Concoct ctimes
  afs: Fix EOF corruption
  afs: afs_write_end() should change i_size under the right lock
  afs: Fix non-setting of mtime when writing into mmap
  • Loading branch information
Linus Torvalds committed Jun 17, 2020
2 parents f17957f + b6489a4 commit 26c20ff
Show file tree
Hide file tree
Showing 10 changed files with 225 additions and 124 deletions.
62 changes: 55 additions & 7 deletions fs/afs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ static void afs_do_lookup_success(struct afs_operation *op)
vp = &op->file[0];
abort_code = vp->scb.status.abort_code;
if (abort_code != 0) {
op->abort_code = abort_code;
op->ac.abort_code = abort_code;
op->error = afs_abort_to_error(abort_code);
}
break;
Expand Down Expand Up @@ -696,10 +696,11 @@ static const struct afs_operation_ops afs_inline_bulk_status_operation = {
.success = afs_do_lookup_success,
};

static const struct afs_operation_ops afs_fetch_status_operation = {
static const struct afs_operation_ops afs_lookup_fetch_status_operation = {
.issue_afs_rpc = afs_fs_fetch_status,
.issue_yfs_rpc = yfs_fs_fetch_status,
.success = afs_do_lookup_success,
.aborted = afs_check_for_remote_deletion,
};

/*
Expand Down Expand Up @@ -1236,6 +1237,17 @@ void afs_d_release(struct dentry *dentry)
_enter("%pd", dentry);
}

void afs_check_for_remote_deletion(struct afs_operation *op)
{
struct afs_vnode *vnode = op->file[0].vnode;

switch (op->ac.abort_code) {
case VNOVNODE:
set_bit(AFS_VNODE_DELETED, &vnode->flags);
afs_break_callback(vnode, afs_cb_break_for_deleted);
}
}

/*
* Create a new inode for create/mkdir/symlink
*/
Expand Down Expand Up @@ -1268,7 +1280,7 @@ static void afs_vnode_new_inode(struct afs_operation *op)
static void afs_create_success(struct afs_operation *op)
{
_enter("op=%08x", op->debug_id);
afs_check_for_remote_deletion(op, op->file[0].vnode);
op->ctime = op->file[0].scb.status.mtime_client;
afs_vnode_commit_status(op, &op->file[0]);
afs_update_dentry_version(op, &op->file[0], op->dentry);
afs_vnode_new_inode(op);
Expand Down Expand Up @@ -1302,6 +1314,7 @@ static const struct afs_operation_ops afs_mkdir_operation = {
.issue_afs_rpc = afs_fs_make_dir,
.issue_yfs_rpc = yfs_fs_make_dir,
.success = afs_create_success,
.aborted = afs_check_for_remote_deletion,
.edit_dir = afs_create_edit_dir,
.put = afs_create_put,
};
Expand All @@ -1325,6 +1338,7 @@ static int afs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)

afs_op_set_vnode(op, 0, dvnode);
op->file[0].dv_delta = 1;
op->file[0].update_ctime = true;
op->dentry = dentry;
op->create.mode = S_IFDIR | mode;
op->create.reason = afs_edit_dir_for_mkdir;
Expand All @@ -1350,7 +1364,7 @@ static void afs_dir_remove_subdir(struct dentry *dentry)
static void afs_rmdir_success(struct afs_operation *op)
{
_enter("op=%08x", op->debug_id);
afs_check_for_remote_deletion(op, op->file[0].vnode);
op->ctime = op->file[0].scb.status.mtime_client;
afs_vnode_commit_status(op, &op->file[0]);
afs_update_dentry_version(op, &op->file[0], op->dentry);
}
Expand Down Expand Up @@ -1382,6 +1396,7 @@ static const struct afs_operation_ops afs_rmdir_operation = {
.issue_afs_rpc = afs_fs_remove_dir,
.issue_yfs_rpc = yfs_fs_remove_dir,
.success = afs_rmdir_success,
.aborted = afs_check_for_remote_deletion,
.edit_dir = afs_rmdir_edit_dir,
.put = afs_rmdir_put,
};
Expand All @@ -1404,6 +1419,7 @@ static int afs_rmdir(struct inode *dir, struct dentry *dentry)

afs_op_set_vnode(op, 0, dvnode);
op->file[0].dv_delta = 1;
op->file[0].update_ctime = true;

op->dentry = dentry;
op->ops = &afs_rmdir_operation;
Expand Down Expand Up @@ -1479,7 +1495,8 @@ static void afs_dir_remove_link(struct afs_operation *op)
static void afs_unlink_success(struct afs_operation *op)
{
_enter("op=%08x", op->debug_id);
afs_check_for_remote_deletion(op, op->file[0].vnode);
op->ctime = op->file[0].scb.status.mtime_client;
afs_check_dir_conflict(op, &op->file[0]);
afs_vnode_commit_status(op, &op->file[0]);
afs_vnode_commit_status(op, &op->file[1]);
afs_update_dentry_version(op, &op->file[0], op->dentry);
Expand Down Expand Up @@ -1511,6 +1528,7 @@ static const struct afs_operation_ops afs_unlink_operation = {
.issue_afs_rpc = afs_fs_remove_file,
.issue_yfs_rpc = yfs_fs_remove_file,
.success = afs_unlink_success,
.aborted = afs_check_for_remote_deletion,
.edit_dir = afs_unlink_edit_dir,
.put = afs_unlink_put,
};
Expand All @@ -1537,6 +1555,7 @@ static int afs_unlink(struct inode *dir, struct dentry *dentry)

afs_op_set_vnode(op, 0, dvnode);
op->file[0].dv_delta = 1;
op->file[0].update_ctime = true;

/* Try to make sure we have a callback promise on the victim. */
ret = afs_validate(vnode, op->key);
Expand All @@ -1561,9 +1580,25 @@ static int afs_unlink(struct inode *dir, struct dentry *dentry)
spin_unlock(&dentry->d_lock);

op->file[1].vnode = vnode;
op->file[1].update_ctime = true;
op->file[1].op_unlinked = true;
op->dentry = dentry;
op->ops = &afs_unlink_operation;
return afs_do_sync_operation(op);
afs_begin_vnode_operation(op);
afs_wait_for_operation(op);

/* If there was a conflict with a third party, check the status of the
* unlinked vnode.
*/
if (op->error == 0 && (op->flags & AFS_OPERATION_DIR_CONFLICT)) {
op->file[1].update_ctime = false;
op->fetch_status.which = 1;
op->ops = &afs_fetch_status_operation;
afs_begin_vnode_operation(op);
afs_wait_for_operation(op);
}

return afs_put_operation(op);

error:
return afs_put_operation(op);
Expand All @@ -1573,6 +1608,7 @@ static const struct afs_operation_ops afs_create_operation = {
.issue_afs_rpc = afs_fs_create_file,
.issue_yfs_rpc = yfs_fs_create_file,
.success = afs_create_success,
.aborted = afs_check_for_remote_deletion,
.edit_dir = afs_create_edit_dir,
.put = afs_create_put,
};
Expand Down Expand Up @@ -1601,6 +1637,7 @@ static int afs_create(struct inode *dir, struct dentry *dentry, umode_t mode,

afs_op_set_vnode(op, 0, dvnode);
op->file[0].dv_delta = 1;
op->file[0].update_ctime = true;

op->dentry = dentry;
op->create.mode = S_IFREG | mode;
Expand All @@ -1620,6 +1657,7 @@ static void afs_link_success(struct afs_operation *op)
struct afs_vnode_param *vp = &op->file[1];

_enter("op=%08x", op->debug_id);
op->ctime = dvp->scb.status.mtime_client;
afs_vnode_commit_status(op, dvp);
afs_vnode_commit_status(op, vp);
afs_update_dentry_version(op, dvp, op->dentry);
Expand All @@ -1640,6 +1678,7 @@ static const struct afs_operation_ops afs_link_operation = {
.issue_afs_rpc = afs_fs_link,
.issue_yfs_rpc = yfs_fs_link,
.success = afs_link_success,
.aborted = afs_check_for_remote_deletion,
.edit_dir = afs_create_edit_dir,
.put = afs_link_put,
};
Expand Down Expand Up @@ -1672,6 +1711,8 @@ static int afs_link(struct dentry *from, struct inode *dir,
afs_op_set_vnode(op, 0, dvnode);
afs_op_set_vnode(op, 1, vnode);
op->file[0].dv_delta = 1;
op->file[0].update_ctime = true;
op->file[1].update_ctime = true;

op->dentry = dentry;
op->dentry_2 = from;
Expand All @@ -1689,6 +1730,7 @@ static const struct afs_operation_ops afs_symlink_operation = {
.issue_afs_rpc = afs_fs_symlink,
.issue_yfs_rpc = yfs_fs_symlink,
.success = afs_create_success,
.aborted = afs_check_for_remote_deletion,
.edit_dir = afs_create_edit_dir,
.put = afs_create_put,
};
Expand Down Expand Up @@ -1740,9 +1782,13 @@ static void afs_rename_success(struct afs_operation *op)
{
_enter("op=%08x", op->debug_id);

op->ctime = op->file[0].scb.status.mtime_client;
afs_check_dir_conflict(op, &op->file[1]);
afs_vnode_commit_status(op, &op->file[0]);
if (op->file[1].vnode != op->file[0].vnode)
if (op->file[1].vnode != op->file[0].vnode) {
op->ctime = op->file[1].scb.status.mtime_client;
afs_vnode_commit_status(op, &op->file[1]);
}
}

static void afs_rename_edit_dir(struct afs_operation *op)
Expand Down Expand Up @@ -1860,6 +1906,8 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
afs_op_set_vnode(op, 1, new_dvnode); /* May be same as orig_dvnode */
op->file[0].dv_delta = 1;
op->file[1].dv_delta = 1;
op->file[0].update_ctime = true;
op->file[1].update_ctime = true;

op->dentry = old_dentry;
op->dentry_2 = new_dentry;
Expand Down
38 changes: 28 additions & 10 deletions fs/afs/dir_silly.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ static void afs_silly_rename_success(struct afs_operation *op)
{
_enter("op=%08x", op->debug_id);

afs_check_dir_conflict(op, &op->file[0]);
afs_vnode_commit_status(op, &op->file[0]);
}

Expand Down Expand Up @@ -69,6 +70,11 @@ static int afs_do_silly_rename(struct afs_vnode *dvnode, struct afs_vnode *vnode
return PTR_ERR(op);

afs_op_set_vnode(op, 0, dvnode);
afs_op_set_vnode(op, 1, dvnode);
op->file[0].dv_delta = 1;
op->file[1].dv_delta = 1;
op->file[0].update_ctime = true;
op->file[1].update_ctime = true;

op->dentry = old;
op->dentry_2 = new;
Expand Down Expand Up @@ -129,6 +135,7 @@ int afs_sillyrename(struct afs_vnode *dvnode, struct afs_vnode *vnode,
switch (ret) {
case 0:
/* The rename succeeded. */
set_bit(AFS_VNODE_SILLY_DELETED, &vnode->flags);
d_move(dentry, sdentry);
break;
case -ERESTARTSYS:
Expand All @@ -148,19 +155,11 @@ int afs_sillyrename(struct afs_vnode *dvnode, struct afs_vnode *vnode,

static void afs_silly_unlink_success(struct afs_operation *op)
{
struct afs_vnode *vnode = op->file[1].vnode;

_enter("op=%08x", op->debug_id);
afs_check_for_remote_deletion(op, op->file[0].vnode);
afs_check_dir_conflict(op, &op->file[0]);
afs_vnode_commit_status(op, &op->file[0]);
afs_vnode_commit_status(op, &op->file[1]);
afs_update_dentry_version(op, &op->file[0], op->dentry);

drop_nlink(&vnode->vfs_inode);
if (vnode->vfs_inode.i_nlink == 0) {
set_bit(AFS_VNODE_DELETED, &vnode->flags);
clear_bit(AFS_VNODE_CB_PROMISED, &vnode->flags);
}
}

static void afs_silly_unlink_edit_dir(struct afs_operation *op)
Expand All @@ -181,6 +180,7 @@ static const struct afs_operation_ops afs_silly_unlink_operation = {
.issue_afs_rpc = afs_fs_remove_file,
.issue_yfs_rpc = yfs_fs_remove_file,
.success = afs_silly_unlink_success,
.aborted = afs_check_for_remote_deletion,
.edit_dir = afs_silly_unlink_edit_dir,
};

Expand All @@ -200,12 +200,30 @@ static int afs_do_silly_unlink(struct afs_vnode *dvnode, struct afs_vnode *vnode

afs_op_set_vnode(op, 0, dvnode);
afs_op_set_vnode(op, 1, vnode);
op->file[0].dv_delta = 1;
op->file[0].update_ctime = true;
op->file[1].op_unlinked = true;
op->file[1].update_ctime = true;

op->dentry = dentry;
op->ops = &afs_silly_unlink_operation;

trace_afs_silly_rename(vnode, true);
return afs_do_sync_operation(op);
afs_begin_vnode_operation(op);
afs_wait_for_operation(op);

/* If there was a conflict with a third party, check the status of the
* unlinked vnode.
*/
if (op->error == 0 && (op->flags & AFS_OPERATION_DIR_CONFLICT)) {
op->file[1].update_ctime = false;
op->fetch_status.which = 1;
op->ops = &afs_fetch_status_operation;
afs_begin_vnode_operation(op);
afs_wait_for_operation(op);
}

return afs_put_operation(op);
}

/*
Expand Down
2 changes: 1 addition & 1 deletion fs/afs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ static void afs_fetch_data_success(struct afs_operation *op)
struct afs_vnode *vnode = op->file[0].vnode;

_enter("op=%08x", op->debug_id);
afs_check_for_remote_deletion(op, vnode);
afs_vnode_commit_status(op, &op->file[0]);
afs_stat_v(vnode, n_fetches);
atomic_long_add(op->fetch.req->actual_len, &op->net->n_fetch_bytes);
Expand All @@ -240,6 +239,7 @@ static const struct afs_operation_ops afs_fetch_data_operation = {
.issue_afs_rpc = afs_fs_fetch_data,
.issue_yfs_rpc = yfs_fs_fetch_data,
.success = afs_fetch_data_success,
.aborted = afs_check_for_remote_deletion,
.put = afs_fetch_data_put,
};

Expand Down
4 changes: 1 addition & 3 deletions fs/afs/flock.c
Original file line number Diff line number Diff line change
Expand Up @@ -175,17 +175,15 @@ static void afs_kill_lockers_enoent(struct afs_vnode *vnode)

static void afs_lock_success(struct afs_operation *op)
{
struct afs_vnode *vnode = op->file[0].vnode;

_enter("op=%08x", op->debug_id);
afs_check_for_remote_deletion(op, vnode);
afs_vnode_commit_status(op, &op->file[0]);
}

static const struct afs_operation_ops afs_set_lock_operation = {
.issue_afs_rpc = afs_fs_set_lock,
.issue_yfs_rpc = yfs_fs_set_lock,
.success = afs_lock_success,
.aborted = afs_check_for_remote_deletion,
};

/*
Expand Down
10 changes: 9 additions & 1 deletion fs/afs/fs_operation.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,17 @@ void afs_wait_for_operation(struct afs_operation *op)
op->error = afs_wait_for_call_to_complete(op->call, &op->ac);
}

if (op->error == 0) {
switch (op->error) {
case 0:
_debug("success");
op->ops->success(op);
break;
case -ECONNABORTED:
if (op->ops->aborted)
op->ops->aborted(op);
break;
default:
break;
}

afs_end_vnode_operation(op);
Expand Down
Loading

0 comments on commit 26c20ff

Please sign in to comment.