NFS currently caches the results of ACCESS lookups indefinitely while the inode doesn't change (i.e. while ctime, changeid, owner/group/mode etc don't change). This is a problem if the result from the server might change. When the result from the server depends purely on the credentials provided by the client and the information in the inode, it is not expected that the result from the server will change, and the current behaviour is safe. However in some configurations the server can include another lookup step. This happens with the Linux NFS server when the "--manage-gids" option is given to rpc.mountd. NetApp servers have similar functionality with "AUTH_SYS Extended Groups" functionality in ONTAP9. With these, the user reported by the client is mapped on the server to a list of group ids. If this mapping changes, the result of ACCESS can change. This is particularly a problem when a new group is added to a user. If they had already tried and failed to access the file (or more commonly a directory) then adding them to the group will not successfully give them access as the failure is cached. Even if the user logs out and back in again to get the new credential on the client, they might try to access the file before the server is aware of the change. By default the Linux server caches group information for 30 minutes. This can be reduced but there will always be a window after the group has been added when the server can still report ACCESS based on old group information. The inverse is less of a problem. Removing someone from a group has never been a guaranteed way to remove any access - at the very least a user normally needs to log off before they lose access to any groups that they were a member of. If a user is removed from a group they may still be granted "access" on the client, but this isn't sufficient to change anything for which write access has been removed, and normally only provides read access to cached content. So caching positive ACCESS rights indefinitely is not a significant security problem. The value of the ACCESS cache is realised primarily for successful tests. These happen often, for example the test for X permissions during filename lookups. Having a quick (even lock-free) result helps this common operation. When the ACCESS cache denies the access there is less cost in taking longer to confirm the access, and this is the case where a stale cache is more problematic. So, this patch changes the way that the access cache is used. - If the requested access is found in the cache, and is granted, then the call uses the cached information no matter how old it is. - If the requested access is found in the cache and is denied, then the cached result is only used if is it newer than the current MINATTRTIMEO for the file. - If the requested access is found in the cache, is denied, and is more than MINATTRTIMEO old, then a new ACCESS request is made to the server - If the requested access is NOT found in the cache, obviously a new ACCESS request is made to the server, and this will be cached. Signed-off-by: NeilBrown <neilb@xxxxxxx> --- fs/nfs/dir.c | 24 +++++++++++++++++++----- fs/nfs/nfs4proc.c | 1 + include/linux/nfs_fs.h | 1 + 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index b4e2c6a34234..c461029515b5 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -2900,7 +2900,8 @@ static struct nfs_access_entry *nfs_access_search_rbtree(struct inode *inode, co return NULL; } -static int nfs_access_get_cached_locked(struct inode *inode, const struct cred *cred, u32 *mask, bool may_block) +static int nfs_access_get_cached_locked(struct inode *inode, const struct cred *cred, + u32 *mask, unsigned long *cjiffies, bool may_block) { struct nfs_inode *nfsi = NFS_I(inode); struct nfs_access_entry *cache; @@ -2931,6 +2932,7 @@ static int nfs_access_get_cached_locked(struct inode *inode, const struct cred * retry = false; } *mask = cache->mask; + *cjiffies = cache->jiffies; list_move_tail(&cache->lru, &nfsi->access_cache_entry_lru); err = 0; out: @@ -2942,7 +2944,8 @@ static int nfs_access_get_cached_locked(struct inode *inode, const struct cred * return -ENOENT; } -static int nfs_access_get_cached_rcu(struct inode *inode, const struct cred *cred, u32 *mask) +static int nfs_access_get_cached_rcu(struct inode *inode, const struct cred *cred, + u32 *mask, unsigned long *cjiffies) { /* Only check the most recently returned cache entry, * but do it without locking. @@ -2965,6 +2968,7 @@ static int nfs_access_get_cached_rcu(struct inode *inode, const struct cred *cre if (nfs_check_cache_invalid(inode, NFS_INO_INVALID_ACCESS)) goto out; *mask = cache->mask; + *cjiffies = cache->jiffies; err = 0; out: rcu_read_unlock(); @@ -2974,16 +2978,24 @@ static int nfs_access_get_cached_rcu(struct inode *inode, const struct cred *cre int nfs_access_check_cached(struct inode *inode, const struct cred *cred, u32 want, bool may_block) { + unsigned long cjiffies; u32 have; int status; - status = nfs_access_get_cached_rcu(inode, cred, &have); + status = nfs_access_get_cached_rcu(inode, cred, &have, &cjiffies); if (status != 0) status = nfs_access_get_cached_locked(inode, cred, &have, - may_block); + &cjiffies, may_block); - if (status == 0 && (want & ~have) != 0) + if (status == 0 && (want & ~have) != 0) { status = -EACCES; + /* Don't trust a negative result beyond MINATTRTIMEO, + * even if we hold a delegation + */ + if (!time_in_range_open(jiffies, cjiffies, + cjiffies + NFS_MINATTRTIMEO(inode))) + status = -ENOENT; + } return status; } EXPORT_SYMBOL_GPL(nfs_access_check_cached); @@ -3036,6 +3048,7 @@ void nfs_access_add_cache(struct inode *inode, struct nfs_access_entry *set, cache->fsgid = cred->fsgid; cache->group_info = get_group_info(cred->group_info); cache->mask = set->mask; + cache->jiffies = set->jiffies; /* The above field assignments must be visible * before this item appears on the lru. We cannot easily @@ -3121,6 +3134,7 @@ static int nfs_do_access(struct inode *inode, const struct cred *cred, int mask) */ cache.mask = NFS_ACCESS_READ | NFS_ACCESS_MODIFY | NFS_ACCESS_EXTEND | nfs_access_xattr_mask(NFS_SERVER(inode)); + cache.jiffies = jiffies; if (S_ISDIR(inode->i_mode)) cache.mask |= NFS_ACCESS_DELETE | NFS_ACCESS_LOOKUP; else diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 1e80d88df588..a78b62c5bad1 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2644,6 +2644,7 @@ static int nfs4_opendata_access(const struct cred *cred, mask = NFS4_ACCESS_READ; nfs_access_set_mask(&cache, opendata->o_res.access_result); + cache.jiffies = jiffies; nfs_access_add_cache(state->inode, &cache, cred); flags = NFS4_ACCESS_READ | NFS4_ACCESS_EXECUTE | NFS4_ACCESS_LOOKUP; diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index 7614f621c42d..e40bd61c80c5 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -56,6 +56,7 @@ struct nfs_access_entry { struct rb_node rb_node; struct list_head lru; + unsigned long jiffies; kuid_t fsuid; kgid_t fsgid; struct group_info *group_info;