Skip to content

Commit efeda80

Browse files
trondmyamschuma-ntap
authored andcommitted
NFSv4: Fix revalidation of dentries with delegations
If a dentry was not initially looked up while we were holding a delegation, then we do still need to revalidate that it still holds the same name. If there are multiple hard links to the same file, then all the hard links need validation. Reported-by: Benjamin Coddington <[email protected]> Signed-off-by: Trond Myklebust <[email protected]> Reviewed-by: Benjamin Coddington <[email protected]> Tested-by: Benjamin Coddington <[email protected]> [Anna: Put nfs_unset_verifier_delegated() under CONFIG_NFS_V4] Signed-off-by: Anna Schumaker <[email protected]>
1 parent cf5b405 commit efeda80

File tree

4 files changed

+115
-23
lines changed

4 files changed

+115
-23
lines changed

fs/nfs/delegation.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ static void nfs_mark_delegation_revoked(struct nfs_delegation *delegation)
4242
if (!test_and_set_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) {
4343
delegation->stateid.type = NFS4_INVALID_STATEID_TYPE;
4444
atomic_long_dec(&nfs_active_delegations);
45+
if (!test_bit(NFS_DELEGATION_RETURNING, &delegation->flags))
46+
nfs_clear_verifier_delegated(delegation->inode);
4547
}
4648
}
4749

@@ -276,6 +278,8 @@ nfs_start_delegation_return_locked(struct nfs_inode *nfsi)
276278
if (!test_and_set_bit(NFS_DELEGATION_RETURNING, &delegation->flags))
277279
ret = delegation;
278280
spin_unlock(&delegation->lock);
281+
if (ret)
282+
nfs_clear_verifier_delegated(&nfsi->vfs_inode);
279283
out:
280284
return ret;
281285
}
@@ -689,6 +693,8 @@ void nfs4_inode_return_delegation_on_close(struct inode *inode)
689693
ret = delegation;
690694
}
691695
spin_unlock(&delegation->lock);
696+
if (ret)
697+
nfs_clear_verifier_delegated(inode);
692698
}
693699
out:
694700
rcu_read_unlock();

fs/nfs/dir.c

Lines changed: 102 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -986,14 +986,113 @@ static int nfs_fsync_dir(struct file *filp, loff_t start, loff_t end,
986986
* full lookup on all child dentries of 'dir' whenever a change occurs
987987
* on the server that might have invalidated our dcache.
988988
*
989+
* Note that we reserve bit '0' as a tag to let us know when a dentry
990+
* was revalidated while holding a delegation on its inode.
991+
*
989992
* The caller should be holding dir->i_lock
990993
*/
991994
void nfs_force_lookup_revalidate(struct inode *dir)
992995
{
993-
NFS_I(dir)->cache_change_attribute++;
996+
NFS_I(dir)->cache_change_attribute += 2;
994997
}
995998
EXPORT_SYMBOL_GPL(nfs_force_lookup_revalidate);
996999

1000+
/**
1001+
* nfs_verify_change_attribute - Detects NFS remote directory changes
1002+
* @dir: pointer to parent directory inode
1003+
* @verf: previously saved change attribute
1004+
*
1005+
* Return "false" if the verifiers doesn't match the change attribute.
1006+
* This would usually indicate that the directory contents have changed on
1007+
* the server, and that any dentries need revalidating.
1008+
*/
1009+
static bool nfs_verify_change_attribute(struct inode *dir, unsigned long verf)
1010+
{
1011+
return (verf & ~1UL) == nfs_save_change_attribute(dir);
1012+
}
1013+
1014+
static void nfs_set_verifier_delegated(unsigned long *verf)
1015+
{
1016+
*verf |= 1UL;
1017+
}
1018+
1019+
#if IS_ENABLED(CONFIG_NFS_V4)
1020+
static void nfs_unset_verifier_delegated(unsigned long *verf)
1021+
{
1022+
*verf &= ~1UL;
1023+
}
1024+
#endif /* IS_ENABLED(CONFIG_NFS_V4) */
1025+
1026+
static bool nfs_test_verifier_delegated(unsigned long verf)
1027+
{
1028+
return verf & 1;
1029+
}
1030+
1031+
static bool nfs_verifier_is_delegated(struct dentry *dentry)
1032+
{
1033+
return nfs_test_verifier_delegated(dentry->d_time);
1034+
}
1035+
1036+
static void nfs_set_verifier_locked(struct dentry *dentry, unsigned long verf)
1037+
{
1038+
struct inode *inode = d_inode(dentry);
1039+
1040+
if (!nfs_verifier_is_delegated(dentry) &&
1041+
!nfs_verify_change_attribute(d_inode(dentry->d_parent), verf))
1042+
goto out;
1043+
if (inode && NFS_PROTO(inode)->have_delegation(inode, FMODE_READ))
1044+
nfs_set_verifier_delegated(&verf);
1045+
out:
1046+
dentry->d_time = verf;
1047+
}
1048+
1049+
/**
1050+
* nfs_set_verifier - save a parent directory verifier in the dentry
1051+
* @dentry: pointer to dentry
1052+
* @verf: verifier to save
1053+
*
1054+
* Saves the parent directory verifier in @dentry. If the inode has
1055+
* a delegation, we also tag the dentry as having been revalidated
1056+
* while holding a delegation so that we know we don't have to
1057+
* look it up again after a directory change.
1058+
*/
1059+
void nfs_set_verifier(struct dentry *dentry, unsigned long verf)
1060+
{
1061+
1062+
spin_lock(&dentry->d_lock);
1063+
nfs_set_verifier_locked(dentry, verf);
1064+
spin_unlock(&dentry->d_lock);
1065+
}
1066+
EXPORT_SYMBOL_GPL(nfs_set_verifier);
1067+
1068+
#if IS_ENABLED(CONFIG_NFS_V4)
1069+
/**
1070+
* nfs_clear_verifier_delegated - clear the dir verifier delegation tag
1071+
* @inode: pointer to inode
1072+
*
1073+
* Iterates through the dentries in the inode alias list and clears
1074+
* the tag used to indicate that the dentry has been revalidated
1075+
* while holding a delegation.
1076+
* This function is intended for use when the delegation is being
1077+
* returned or revoked.
1078+
*/
1079+
void nfs_clear_verifier_delegated(struct inode *inode)
1080+
{
1081+
struct dentry *alias;
1082+
1083+
if (!inode)
1084+
return;
1085+
spin_lock(&inode->i_lock);
1086+
hlist_for_each_entry(alias, &inode->i_dentry, d_u.d_alias) {
1087+
spin_lock(&alias->d_lock);
1088+
nfs_unset_verifier_delegated(&alias->d_time);
1089+
spin_unlock(&alias->d_lock);
1090+
}
1091+
spin_unlock(&inode->i_lock);
1092+
}
1093+
EXPORT_SYMBOL_GPL(nfs_clear_verifier_delegated);
1094+
#endif /* IS_ENABLED(CONFIG_NFS_V4) */
1095+
9971096
/*
9981097
* A check for whether or not the parent directory has changed.
9991098
* In the case it has, we assume that the dentries are untrustworthy
@@ -1235,7 +1334,7 @@ nfs_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
12351334
goto out_bad;
12361335
}
12371336

1238-
if (NFS_PROTO(dir)->have_delegation(inode, FMODE_READ))
1337+
if (nfs_verifier_is_delegated(dentry))
12391338
return nfs_lookup_revalidate_delegated(dir, dentry, inode);
12401339

12411340
/* Force a full look up iff the parent directory has changed */
@@ -1675,7 +1774,7 @@ nfs4_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
16751774
if (inode == NULL)
16761775
goto full_reval;
16771776

1678-
if (NFS_PROTO(dir)->have_delegation(inode, FMODE_READ))
1777+
if (nfs_verifier_is_delegated(dentry))
16791778
return nfs_lookup_revalidate_delegated(dir, dentry, inode);
16801779

16811780
/* NFS only supports OPEN on regular files */

fs/nfs/inode.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,6 +2114,7 @@ static void init_once(void *foo)
21142114
init_rwsem(&nfsi->rmdir_sem);
21152115
mutex_init(&nfsi->commit_mutex);
21162116
nfs4_init_once(nfsi);
2117+
nfsi->cache_change_attribute = 0;
21172118
}
21182119

21192120
static int __init nfs_init_inodecache(void)

include/linux/nfs_fs.h

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -337,35 +337,17 @@ static inline int nfs_server_capable(struct inode *inode, int cap)
337337
return NFS_SERVER(inode)->caps & cap;
338338
}
339339

340-
static inline void nfs_set_verifier(struct dentry * dentry, unsigned long verf)
341-
{
342-
dentry->d_time = verf;
343-
}
344-
345340
/**
346341
* nfs_save_change_attribute - Returns the inode attribute change cookie
347342
* @dir - pointer to parent directory inode
348-
* The "change attribute" is updated every time we finish an operation
349-
* that will result in a metadata change on the server.
343+
* The "cache change attribute" is updated when we need to revalidate
344+
* our dentry cache after a directory was seen to change on the server.
350345
*/
351346
static inline unsigned long nfs_save_change_attribute(struct inode *dir)
352347
{
353348
return NFS_I(dir)->cache_change_attribute;
354349
}
355350

356-
/**
357-
* nfs_verify_change_attribute - Detects NFS remote directory changes
358-
* @dir - pointer to parent directory inode
359-
* @chattr - previously saved change attribute
360-
* Return "false" if the verifiers doesn't match the change attribute.
361-
* This would usually indicate that the directory contents have changed on
362-
* the server, and that any dentries need revalidating.
363-
*/
364-
static inline int nfs_verify_change_attribute(struct inode *dir, unsigned long chattr)
365-
{
366-
return chattr == NFS_I(dir)->cache_change_attribute;
367-
}
368-
369351
/*
370352
* linux/fs/nfs/inode.c
371353
*/
@@ -495,6 +477,10 @@ extern const struct file_operations nfs_dir_operations;
495477
extern const struct dentry_operations nfs_dentry_operations;
496478

497479
extern void nfs_force_lookup_revalidate(struct inode *dir);
480+
extern void nfs_set_verifier(struct dentry * dentry, unsigned long verf);
481+
#if IS_ENABLED(CONFIG_NFS_V4)
482+
extern void nfs_clear_verifier_delegated(struct inode *inode);
483+
#endif /* IS_ENABLED(CONFIG_NFS_V4) */
498484
extern struct dentry *nfs_add_or_obtain(struct dentry *dentry,
499485
struct nfs_fh *fh, struct nfs_fattr *fattr,
500486
struct nfs4_label *label);

0 commit comments

Comments
 (0)