Commit 26c20ffc authored by Linus Torvalds's avatar Linus Torvalds
Browse files

Merge tag 'afs-fixes-20200616' of git://git.kernel.org/pub/scm/linux/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
parents f17957f7 b6489a49

fs/afs/dir.c

+55 −7
Original line number Diff line number Diff line
@@ -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;
@@ -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,

};



/*
@@ -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

 */
@@ -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);
@@ -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,

};
@@ -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;
@@ -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);

}
@@ -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,

};
@@ -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;
@@ -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);
@@ -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,

};
@@ -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);
@@ -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);
@@ -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,

};
@@ -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;
@@ -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);
@@ -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,

};
@@ -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;
@@ -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,

};
@@ -1740,10 +1782,14 @@ 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)

{
@@ -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;

fs/afs/dir_silly.c

+28 −10
Original line number Diff line number Diff line
@@ -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]);

}
@@ -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;
@@ -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:
@@ -148,19 +155,11 @@ out: 


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)
@@ -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,

};
@@ -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);

}



/*

fs/afs/file.c

+1 −1
Original line number Diff line number Diff line
@@ -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);
@@ -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,

};

fs/afs/flock.c

+1 −3
Original line number Diff line number Diff line
@@ -175,10 +175,7 @@ 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]);

}
@@ -186,6 +183,7 @@ 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,

};



/*

fs/afs/fs_operation.c

+9 −1
Original line number Diff line number Diff line
@@ -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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE