From: NeilBrown <neilb@suse.de>
Subject: NFS: limit use of ACCESS cache for negative responses
Patch-mainline: Submitted - linux-nfs 28 apr 2022
References: bsc#1196570

[[
  165110909570.7595.8578730126480600782.stgit@noble.brown
  Upstream maintainer doesn't like this
]]

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 is 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 reduce 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.

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
were 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 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@suse.de>
Acked-by: NeilBrown <neilb@suse.com>

---
 fs/nfs/dir.c           |   14 ++++++++++++--
 include/linux/nfs_fs.h |    1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2769,6 +2769,7 @@ static int nfs_access_get_cached_locked(
 		retry = false;
 	}
 	res->mask = cache->mask;
+	res->jiffies = cache->jiffies;
 	list_move_tail(&cache->lru, &nfsi->access_cache_entry_lru);
 	err = 0;
 out:
@@ -2803,6 +2804,7 @@ static int nfs_access_get_cached_rcu(str
 	if (nfs_check_cache_invalid(inode, NFS_INO_INVALID_ACCESS))
 		goto out;
 	res->mask = cache->mask;
+	res->jiffies = cache->jiffies;
 	err = 0;
 out:
 	rcu_read_unlock();
@@ -2871,6 +2873,7 @@ void nfs_access_add_cache(struct inode *
 	cache->fsgid = cred->fsgid;
 	cache->group_info = get_group_info(cred->group_info);
 	cache->mask = set->mask;
+	cache->jiffies = jiffies;
 
 	/* The above field assignments must be visible
 	 * before this item appears on the lru.  We cannot easily
@@ -2943,8 +2946,15 @@ static int nfs_do_access(struct inode *i
 	trace_nfs_access_enter(inode);
 
 	status = nfs_access_get_cached(inode, cred, &cache, may_block);
-	if (status == 0)
-		goto out_cached;
+	if (status == 0) {
+		if ((mask & ~cache.mask & (MAY_READ | MAY_WRITE | MAY_EXEC)) == 0)
+			/* if access is granted, trust the cache */
+			goto out_cached;
+		if (time_in_range_open(jiffies, cache.jiffies,
+				       cache.jiffies + NFS_MINATTRTIMEO(inode)))
+			/* If cache entry very new, trust even for negative */
+			goto out_cached;
+	}
 
 	status = -ECHILD;
 	if (!may_block)
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -61,6 +61,7 @@ struct nfs_access_entry {
 	struct group_info	*group_info;
 	__u32			mask;
 	struct rcu_head		rcu_head;
+	unsigned long		jiffies;
 };
 
 struct nfs_lock_context {
