AFS Fixes

Reviewed-by: Marc Dionne <marc.dionne@auristor.com>
 -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEEqG5UsNXhtOCrfGQP+7dXa6fLC2sFAl1UELcACgkQ+7dXa6fL
 C2viWw//eDLvElBjaQsabfpMOVGkf02t+zCzNfKSC0KdM1+GUZ1FyXQ0UAtgwICY
 Sp01h/RZa0V07sfYP7R5kL4/KIMdODmhrP0iiHDpoMjKCL7qR9tFbJDAcHtH8xz2
 52UV2dmdDBI/wdw/i5dn6M02SoYAQMl1XT49SkzhFSELVchkpraGsf1vf4yITeVe
 eI1TaOxI+TUaeH5f6+KWp6c8K8q70p3KfrR2VmCWkBrD7PNg9lp19pVnz8tdofYu
 xURHQbJulSqM+mY7pcNBOi2iWy3dCLjBTkVJIwIhZcZqLThACY38SSaPtmdhgif4
 wcyyZUtd8EGPzPPqbfCx7ycTIIDtL/r98XtGyiTJBKrCK+flZONdu0g/oIzvJ/Wu
 hV4+ButxCuMakbLOe+Hew3lhHFOy7m9XZtOURzxzZSm9uazHDMxnw4ocxIOs24F1
 qus1sG0+rlVDcMYjo2tKEAzOl/ZejJ/NUTd60ANIWKTHply2/2/5dH94B0yLwDnp
 tfifBrBkyqFB4XUKGvqvvJczl0d7+zsEScs4VQLVO/WhATjj6jNnrYKgwvBS5pCM
 890qUzj3TRW7ciZLi0THMEHBlEfbEWhNCaggAqieIvbKv7t4Kh2cUBaIsxo4IYqU
 PBZZhFXRul5ocTJrV9pScl4RbzxE5V0j9cwSiiWnzZL1sQucIgQ=
 =zivP
 -----END PGP SIGNATURE-----

Merge tag 'afs-fixes-20190814' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs

Pull afs fixes from David Howells:

 - Fix the CB.ProbeUuid handler to generate its reply correctly.

 - Fix a mix up in indices when parsing a Volume Location entry record.

 - Fix a potential NULL-pointer deref when cleaning up a read request.

 - Fix the expected data version of the destination directory in
   afs_rename().

 - Fix afs_d_revalidate() to only update d_fsdata if it's not the same
   as the directory data version to reduce the likelihood of overwriting
   the result of a competing operation. (d_fsdata carries the directory
   DV or the least-significant word thereof).

 - Fix the tracking of the data-version on a directory and make sure
   that dentry objects get properly initialised, updated and
   revalidated.

   Also fix rename to update d_fsdata to match the new directory's DV if
   the dentry gets moved over and unhash the dentry to stop
   afs_d_revalidate() from interfering.

* tag 'afs-fixes-20190814' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs:
  afs: Fix missing dentry data version updating
  afs: Only update d_fsdata if different in afs_d_revalidate()
  afs: Fix off-by-one in afs_rename() expected data version calculation
  fs: afs: Fix a possible null-pointer dereference in afs_put_read()
  afs: Fix loop index mixup in afs_deliver_vl_get_entry_by_name_u()
  afs: Fix the CB.ProbeUuid service handler to reply correctly
This commit is contained in:
Linus Torvalds 2019-08-14 14:21:14 -07:00
commit e22a97a2a8
4 changed files with 89 additions and 33 deletions

View File

@ -505,18 +505,14 @@ static void SRXAFSCB_ProbeUuid(struct work_struct *work)
struct afs_call *call = container_of(work, struct afs_call, work);
struct afs_uuid *r = call->request;
struct {
__be32 match;
} reply;
_enter("");
if (memcmp(r, &call->net->uuid, sizeof(call->net->uuid)) == 0)
reply.match = htonl(0);
afs_send_empty_reply(call);
else
reply.match = htonl(1);
rxrpc_kernel_abort_call(call->net->socket, call->rxcall,
1, 1, "K-1");
afs_send_simple_reply(call, &reply, sizeof(reply));
afs_put_call(call);
_leave("");
}

View File

@ -440,7 +440,7 @@ static int afs_dir_iterate_block(struct afs_vnode *dvnode,
* iterate through the data blob that lists the contents of an AFS directory
*/
static int afs_dir_iterate(struct inode *dir, struct dir_context *ctx,
struct key *key)
struct key *key, afs_dataversion_t *_dir_version)
{
struct afs_vnode *dvnode = AFS_FS_I(dir);
struct afs_xdr_dir_page *dbuf;
@ -460,6 +460,7 @@ static int afs_dir_iterate(struct inode *dir, struct dir_context *ctx,
req = afs_read_dir(dvnode, key);
if (IS_ERR(req))
return PTR_ERR(req);
*_dir_version = req->data_version;
/* round the file position up to the next entry boundary */
ctx->pos += sizeof(union afs_xdr_dirent) - 1;
@ -514,7 +515,10 @@ out:
*/
static int afs_readdir(struct file *file, struct dir_context *ctx)
{
return afs_dir_iterate(file_inode(file), ctx, afs_file_key(file));
afs_dataversion_t dir_version;
return afs_dir_iterate(file_inode(file), ctx, afs_file_key(file),
&dir_version);
}
/*
@ -555,7 +559,8 @@ static int afs_lookup_one_filldir(struct dir_context *ctx, const char *name,
* - just returns the FID the dentry name maps to if found
*/
static int afs_do_lookup_one(struct inode *dir, struct dentry *dentry,
struct afs_fid *fid, struct key *key)
struct afs_fid *fid, struct key *key,
afs_dataversion_t *_dir_version)
{
struct afs_super_info *as = dir->i_sb->s_fs_info;
struct afs_lookup_one_cookie cookie = {
@ -568,7 +573,7 @@ static int afs_do_lookup_one(struct inode *dir, struct dentry *dentry,
_enter("{%lu},%p{%pd},", dir->i_ino, dentry, dentry);
/* search the directory */
ret = afs_dir_iterate(dir, &cookie.ctx, key);
ret = afs_dir_iterate(dir, &cookie.ctx, key, _dir_version);
if (ret < 0) {
_leave(" = %d [iter]", ret);
return ret;
@ -642,6 +647,7 @@ static struct inode *afs_do_lookup(struct inode *dir, struct dentry *dentry,
struct afs_server *server;
struct afs_vnode *dvnode = AFS_FS_I(dir), *vnode;
struct inode *inode = NULL, *ti;
afs_dataversion_t data_version = READ_ONCE(dvnode->status.data_version);
int ret, i;
_enter("{%lu},%p{%pd},", dir->i_ino, dentry, dentry);
@ -669,12 +675,14 @@ static struct inode *afs_do_lookup(struct inode *dir, struct dentry *dentry,
cookie->fids[i].vid = as->volume->vid;
/* search the directory */
ret = afs_dir_iterate(dir, &cookie->ctx, key);
ret = afs_dir_iterate(dir, &cookie->ctx, key, &data_version);
if (ret < 0) {
inode = ERR_PTR(ret);
goto out;
}
dentry->d_fsdata = (void *)(unsigned long)data_version;
inode = ERR_PTR(-ENOENT);
if (!cookie->found)
goto out;
@ -968,7 +976,8 @@ static int afs_d_revalidate(struct dentry *dentry, unsigned int flags)
struct dentry *parent;
struct inode *inode;
struct key *key;
long dir_version, de_version;
afs_dataversion_t dir_version;
long de_version;
int ret;
if (flags & LOOKUP_RCU)
@ -1014,20 +1023,20 @@ static int afs_d_revalidate(struct dentry *dentry, unsigned int flags)
* on a 32-bit system, we only have 32 bits in the dentry to store the
* version.
*/
dir_version = (long)dir->status.data_version;
dir_version = dir->status.data_version;
de_version = (long)dentry->d_fsdata;
if (de_version == dir_version)
goto out_valid;
if (de_version == (long)dir_version)
goto out_valid_noupdate;
dir_version = (long)dir->invalid_before;
if (de_version - dir_version >= 0)
dir_version = dir->invalid_before;
if (de_version - (long)dir_version >= 0)
goto out_valid;
_debug("dir modified");
afs_stat_v(dir, n_reval);
/* search the directory for this vnode */
ret = afs_do_lookup_one(&dir->vfs_inode, dentry, &fid, key);
ret = afs_do_lookup_one(&dir->vfs_inode, dentry, &fid, key, &dir_version);
switch (ret) {
case 0:
/* the filename maps to something */
@ -1080,7 +1089,8 @@ static int afs_d_revalidate(struct dentry *dentry, unsigned int flags)
}
out_valid:
dentry->d_fsdata = (void *)dir_version;
dentry->d_fsdata = (void *)(unsigned long)dir_version;
out_valid_noupdate:
dput(parent);
key_put(key);
_leave(" = 1 [valid]");
@ -1185,6 +1195,20 @@ static void afs_prep_for_new_inode(struct afs_fs_cursor *fc,
iget_data->cb_s_break = fc->cbi->server->cb_s_break;
}
/*
* Note that a dentry got changed. We need to set d_fsdata to the data version
* number derived from the result of the operation. It doesn't matter if
* d_fsdata goes backwards as we'll just revalidate.
*/
static void afs_update_dentry_version(struct afs_fs_cursor *fc,
struct dentry *dentry,
struct afs_status_cb *scb)
{
if (fc->ac.error == 0)
dentry->d_fsdata =
(void *)(unsigned long)scb->status.data_version;
}
/*
* create a directory on an AFS filesystem
*/
@ -1227,6 +1251,7 @@ static int afs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
afs_check_for_remote_deletion(&fc, dvnode);
afs_vnode_commit_status(&fc, dvnode, fc.cb_break,
&data_version, &scb[0]);
afs_update_dentry_version(&fc, dentry, &scb[0]);
afs_vnode_new_inode(&fc, dentry, &iget_data, &scb[1]);
ret = afs_end_vnode_operation(&fc);
if (ret < 0)
@ -1319,6 +1344,7 @@ static int afs_rmdir(struct inode *dir, struct dentry *dentry)
afs_vnode_commit_status(&fc, dvnode, fc.cb_break,
&data_version, scb);
afs_update_dentry_version(&fc, dentry, scb);
ret = afs_end_vnode_operation(&fc);
if (ret == 0) {
afs_dir_remove_subdir(dentry);
@ -1458,6 +1484,7 @@ static int afs_unlink(struct inode *dir, struct dentry *dentry)
&data_version, &scb[0]);
afs_vnode_commit_status(&fc, vnode, fc.cb_break_2,
&data_version_2, &scb[1]);
afs_update_dentry_version(&fc, dentry, &scb[0]);
ret = afs_end_vnode_operation(&fc);
if (ret == 0 && !(scb[1].have_status || scb[1].have_error))
ret = afs_dir_remove_link(dvnode, dentry, key);
@ -1526,6 +1553,7 @@ static int afs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
afs_check_for_remote_deletion(&fc, dvnode);
afs_vnode_commit_status(&fc, dvnode, fc.cb_break,
&data_version, &scb[0]);
afs_update_dentry_version(&fc, dentry, &scb[0]);
afs_vnode_new_inode(&fc, dentry, &iget_data, &scb[1]);
ret = afs_end_vnode_operation(&fc);
if (ret < 0)
@ -1607,6 +1635,7 @@ static int afs_link(struct dentry *from, struct inode *dir,
afs_vnode_commit_status(&fc, vnode, fc.cb_break_2,
NULL, &scb[1]);
ihold(&vnode->vfs_inode);
afs_update_dentry_version(&fc, dentry, &scb[0]);
d_instantiate(dentry, &vnode->vfs_inode);
mutex_unlock(&vnode->io_lock);
@ -1686,6 +1715,7 @@ static int afs_symlink(struct inode *dir, struct dentry *dentry,
afs_check_for_remote_deletion(&fc, dvnode);
afs_vnode_commit_status(&fc, dvnode, fc.cb_break,
&data_version, &scb[0]);
afs_update_dentry_version(&fc, dentry, &scb[0]);
afs_vnode_new_inode(&fc, dentry, &iget_data, &scb[1]);
ret = afs_end_vnode_operation(&fc);
if (ret < 0)
@ -1791,6 +1821,17 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
}
}
/* This bit is potentially nasty as there's a potential race with
* afs_d_revalidate{,_rcu}(). We have to change d_fsdata on the dentry
* to reflect it's new parent's new data_version after the op, but
* d_revalidate may see old_dentry between the op having taken place
* and the version being updated.
*
* So drop the old_dentry for now to make other threads go through
* lookup instead - which we hold a lock against.
*/
d_drop(old_dentry);
ret = -ERESTARTSYS;
if (afs_begin_vnode_operation(&fc, orig_dvnode, key, true)) {
afs_dataversion_t orig_data_version;
@ -1802,9 +1843,9 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
if (orig_dvnode != new_dvnode) {
if (mutex_lock_interruptible_nested(&new_dvnode->io_lock, 1) < 0) {
afs_end_vnode_operation(&fc);
goto error_rehash;
goto error_rehash_old;
}
new_data_version = new_dvnode->status.data_version;
new_data_version = new_dvnode->status.data_version + 1;
} else {
new_data_version = orig_data_version;
new_scb = &scb[0];
@ -1827,7 +1868,7 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
}
ret = afs_end_vnode_operation(&fc);
if (ret < 0)
goto error_rehash;
goto error_rehash_old;
}
if (ret == 0) {
@ -1853,10 +1894,26 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry,
drop_nlink(new_inode);
spin_unlock(&new_inode->i_lock);
}
/* Now we can update d_fsdata on the dentries to reflect their
* new parent's data_version.
*
* 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]);
}
d_move(old_dentry, new_dentry);
goto error_tmp;
}
error_rehash_old:
d_rehash(new_dentry);
error_rehash:
if (rehash)
d_rehash(rehash);

View File

@ -191,11 +191,13 @@ void afs_put_read(struct afs_read *req)
int i;
if (refcount_dec_and_test(&req->usage)) {
for (i = 0; i < req->nr_pages; i++)
if (req->pages[i])
put_page(req->pages[i]);
if (req->pages != req->array)
kfree(req->pages);
if (req->pages) {
for (i = 0; i < req->nr_pages; i++)
if (req->pages[i])
put_page(req->pages[i]);
if (req->pages != req->array)
kfree(req->pages);
}
kfree(req);
}
}

View File

@ -56,23 +56,24 @@ static int afs_deliver_vl_get_entry_by_name_u(struct afs_call *call)
struct afs_uuid__xdr *xdr;
struct afs_uuid *uuid;
int j;
int n = entry->nr_servers;
tmp = ntohl(uvldb->serverFlags[i]);
if (tmp & AFS_VLSF_DONTUSE ||
(new_only && !(tmp & AFS_VLSF_NEWREPSITE)))
continue;
if (tmp & AFS_VLSF_RWVOL) {
entry->fs_mask[i] |= AFS_VOL_VTM_RW;
entry->fs_mask[n] |= AFS_VOL_VTM_RW;
if (vlflags & AFS_VLF_BACKEXISTS)
entry->fs_mask[i] |= AFS_VOL_VTM_BAK;
entry->fs_mask[n] |= AFS_VOL_VTM_BAK;
}
if (tmp & AFS_VLSF_ROVOL)
entry->fs_mask[i] |= AFS_VOL_VTM_RO;
if (!entry->fs_mask[i])
entry->fs_mask[n] |= AFS_VOL_VTM_RO;
if (!entry->fs_mask[n])
continue;
xdr = &uvldb->serverNumber[i];
uuid = (struct afs_uuid *)&entry->fs_server[i];
uuid = (struct afs_uuid *)&entry->fs_server[n];
uuid->time_low = xdr->time_low;
uuid->time_mid = htons(ntohl(xdr->time_mid));
uuid->time_hi_and_version = htons(ntohl(xdr->time_hi_and_version));