Skip to content

Commit

Permalink
afs: Add missing vnode validation checks
Browse files Browse the repository at this point in the history
afs_d_revalidate() should only be validating the directory entry it is
given and the directory to which that belongs; it shouldn't be validating
the inode/vnode to which that dentry points.  Besides, validation need to
be done even if we don't call afs_d_revalidate() - which might be the case
if we're starting from a file descriptor.

In order for afs_d_revalidate() to be fixed, validation points must be
added in some other places.  Certain directory operations, such as
afs_unlink(), already check this, but not all and not all file operations
either.

Note that the validation of a vnode not only checks to see if the
attributes we have are correct, but also gets a promise from the server to
notify us if that file gets changed by a third party.

Add the following checks:

 - Check the vnode we're going to make a hard link to.
 - Check the vnode we're going to move/rename.
 - Check the vnode we're going to read from.
 - Check the vnode we're going to write to.
 - Check the vnode we're going to sync.
 - Check the vnode we're going to make a mapped page writable for.

Some of these aren't strictly necessary as we're going to perform a server
operation that might get the attributes anyway from which we can determine
if something changed - though it might not get us a callback promise.

Signed-off-by: David Howells <[email protected]>
Tested-by: Markus Suvanto <[email protected]>
cc: [email protected]
Link: https://lore.kernel.org/r/163111667354.283156.12720698333342917516.stgit@warthog.procyon.org.uk/
  • Loading branch information
dhowells committed Sep 13, 2021
1 parent 581b202 commit 3978d81
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 3 deletions.
11 changes: 11 additions & 0 deletions fs/afs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -1792,6 +1792,10 @@ static int afs_link(struct dentry *from, struct inode *dir,
goto error;
}

ret = afs_validate(vnode, op->key);
if (ret < 0)
goto error_op;

afs_op_set_vnode(op, 0, dvnode);
afs_op_set_vnode(op, 1, vnode);
op->file[0].dv_delta = 1;
Expand All @@ -1805,6 +1809,8 @@ static int afs_link(struct dentry *from, struct inode *dir,
op->create.reason = afs_edit_dir_for_link;
return afs_do_sync_operation(op);

error_op:
afs_put_operation(op);
error:
d_drop(dentry);
_leave(" = %d", ret);
Expand Down Expand Up @@ -1989,6 +1995,11 @@ static int afs_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
if (IS_ERR(op))
return PTR_ERR(op);

ret = afs_validate(vnode, op->key);
op->error = ret;
if (ret < 0)
goto error;

afs_op_set_vnode(op, 0, orig_dvnode);
afs_op_set_vnode(op, 1, new_dvnode); /* May be same as orig_dvnode */
op->file[0].dv_delta = 1;
Expand Down
16 changes: 15 additions & 1 deletion fs/afs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ static void afs_invalidatepage(struct page *page, unsigned int offset,
static int afs_releasepage(struct page *page, gfp_t gfp_flags);

static void afs_readahead(struct readahead_control *ractl);
static ssize_t afs_file_read_iter(struct kiocb *iocb, struct iov_iter *iter);

const struct file_operations afs_file_operations = {
.open = afs_open,
.release = afs_release,
.llseek = generic_file_llseek,
.read_iter = generic_file_read_iter,
.read_iter = afs_file_read_iter,
.write_iter = afs_file_write,
.mmap = afs_file_mmap,
.splice_read = generic_file_splice_read,
Expand Down Expand Up @@ -503,3 +504,16 @@ static int afs_file_mmap(struct file *file, struct vm_area_struct *vma)
vma->vm_ops = &afs_vm_ops;
return ret;
}

static ssize_t afs_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
{
struct afs_vnode *vnode = AFS_FS_I(file_inode(iocb->ki_filp));
struct afs_file *af = iocb->ki_filp->private_data;
int ret;

ret = afs_validate(vnode, af->key);
if (ret < 0)
return ret;

return generic_file_read_iter(iocb, iter);
}
17 changes: 15 additions & 2 deletions fs/afs/write.c
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,7 @@ int afs_writepages(struct address_space *mapping,
ssize_t afs_file_write(struct kiocb *iocb, struct iov_iter *from)
{
struct afs_vnode *vnode = AFS_FS_I(file_inode(iocb->ki_filp));
struct afs_file *af = iocb->ki_filp->private_data;
ssize_t result;
size_t count = iov_iter_count(from);

Expand All @@ -822,6 +823,10 @@ ssize_t afs_file_write(struct kiocb *iocb, struct iov_iter *from)
if (!count)
return 0;

result = afs_validate(vnode, af->key);
if (result < 0)
return result;

result = generic_file_write_iter(iocb, from);

_leave(" = %zd", result);
Expand All @@ -835,13 +840,18 @@ ssize_t afs_file_write(struct kiocb *iocb, struct iov_iter *from)
*/
int afs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
{
struct inode *inode = file_inode(file);
struct afs_vnode *vnode = AFS_FS_I(inode);
struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
struct afs_file *af = file->private_data;
int ret;

_enter("{%llx:%llu},{n=%pD},%d",
vnode->fid.vid, vnode->fid.vnode, file,
datasync);

ret = afs_validate(vnode, af->key);
if (ret < 0)
return ret;

return file_write_and_wait_range(file, start, end);
}

Expand All @@ -855,11 +865,14 @@ vm_fault_t afs_page_mkwrite(struct vm_fault *vmf)
struct file *file = vmf->vma->vm_file;
struct inode *inode = file_inode(file);
struct afs_vnode *vnode = AFS_FS_I(inode);
struct afs_file *af = file->private_data;
unsigned long priv;
vm_fault_t ret = VM_FAULT_RETRY;

_enter("{{%llx:%llu}},{%lx}", vnode->fid.vid, vnode->fid.vnode, page->index);

afs_validate(vnode, af->key);

sb_start_pagefault(inode->i_sb);

/* Wait for the page to be written to the cache before we allow it to
Expand Down

0 comments on commit 3978d81

Please sign in to comment.