[PATCH v2] lockd: refcount NLM_SHARE access/deny modes

When an NFSv3/NLM client issues multiple NLM_SHARE calls from a single
host for the same (file, owner) tuple, the current implementation
overwrites the recorded access and deny modes with the latest pair.
A subsequent NLM_UNSHARE then drops the entire entry, even if other
grants were implicitly subsumed by the most recent SHARE.  This is
particularly visible to Windows-style clients that map each open of
a file to a distinct NLM_SHARE, all carrying the same NLM owner
handle.  For example:

    1. SHARE(access=RW, deny=W)   -> entry [RW, deny W]
    2. SHARE(access=R,  deny=N)   -> entry [R, deny N]   (RW/W overwritten)
    3. UNSHARE(access=R, deny=N)  -> entry freed
    4. UNSHARE(access=RW, deny=W) -> nothing to release

Track each of the four valid fsh_access and fsh_mode values with a
small per-value refcount.  On SHARE the appropriate buckets are
incremented; on UNSHARE they are decremented.  s_access / s_mode
are recomputed in both paths as the union of all positive buckets
with a non-NONE / non-DN value (indices 1..3), and the entry is
freed once s_access and s_mode are both zero.

NLM_UNSHARE gains the access and deny modes as arguments so the
correct buckets can be decremented.  The two callers in svcproc.c
and svc4proc.c are updated to forward the decoded values.

Signed-off-by: Oscar Ou <oscarou-UelDjCVBxVpWk0Htik3J/[email protected]>
---
Applies on top of "lockd: Regenerate NLMv4 XDR code" by Chuck Lever.

No Fixes: tag.  The fix depends mechanically on the xdrgen
regeneration, so a Fixes: tag would mislead -stable backporters.
Happy to add one plus an informal Depends-on: line if preferred.

Changes since v1:
- Reworded the "buckets" sentence in the commit message: the entry is
  freed once s_access and s_mode are both zero, not when every bucket
  has reached zero.  Index 0 (fsa_NONE / fsm_DN) contributes no bits
  to the union and does not gate freeing.
- Dropped the incorrect claim that per-file f_mutex serialises the
  new arrays, from both the commit message and the comment on
  nlm_recompute_share().  f_mutex is released by nlm_lookup_file()
  before the share helpers run.

 fs/lockd/share.h    |  6 +++++-
 fs/lockd/svc4proc.c |  4 +++-
 fs/lockd/svcproc.c  |  4 +++-
 fs/lockd/svcshare.c | 40 +++++++++++++++++++++++++++++++++++-----
 4 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/fs/lockd/share.h b/fs/lockd/share.h
index 1ec3ccdb2aef..3b414774c5b3 100644
--- a/fs/lockd/share.h
+++ b/fs/lockd/share.h
@@ -11,6 +11,8 @@
 /* Synthetic svid for lockowner lookup during share operations */
 #define LOCKD_SHARE_SVID	(~(u32)0)
 
+#define LOCKD_FSH_NR		4	/* fsh_access / fsh_mode are in {0..3} */
+
 /*
  * DOS share for a specific file
  */
@@ -21,12 +23,14 @@ struct lockd_share {
 	struct xdr_netobj	s_owner;	/* owner handle */
 	u32			s_access;	/* access mode */
 	u32			s_mode;		/* deny mode */
+	u32			s_access_counts[LOCKD_FSH_NR];
+	u32			s_mode_counts[LOCKD_FSH_NR];
 };
 
 __be32	nlmsvc_share_file(struct nlm_host *host, struct nlm_file *file,
 			  struct xdr_netobj *oh, u32 access, u32 mode);
 __be32	nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file,
-			    struct xdr_netobj *oh);
+			    struct xdr_netobj *oh, u32 access, u32 mode);
 void	nlmsvc_traverse_shares(struct nlm_host *, struct nlm_file *,
 					       nlm_host_match_fn_t);
 
diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index 78e675470c4b..49ae487e3390 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -1078,7 +1078,9 @@ static __be32 nlm4svc_proc_unshare(struct svc_rqst *rqstp)
 	if (resp->xdrgen.stat)
 		goto out;
 
-	resp->xdrgen.stat = nlmsvc_unshare_file(host, file, &lock->oh);
+	resp->xdrgen.stat = nlmsvc_unshare_file(host, file, &lock->oh,
+						argp->xdrgen.share.access,
+						argp->xdrgen.share.mode);
 
 	nlmsvc_release_lockowner(lock);
 
diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
index 4836887f11ef..54845e52d31e 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -1097,7 +1097,9 @@ static __be32 nlmsvc_proc_unshare(struct svc_rqst *rqstp)
 	if (resp->xdrgen.stat)
 		goto out;
 
-	resp->xdrgen.stat = nlmsvc_unshare_file(host, file, &lock->oh);
+	resp->xdrgen.stat = nlmsvc_unshare_file(host, file, &lock->oh,
+						argp->xdrgen.share.access,
+						argp->xdrgen.share.mode);
 
 	nlmsvc_release_lockowner(lock);
 
diff --git a/fs/lockd/svcshare.c b/fs/lockd/svcshare.c
index 5ac0ec25d62d..b7372094d397 100644
--- a/fs/lockd/svcshare.c
+++ b/fs/lockd/svcshare.c
@@ -25,6 +25,24 @@ nlm_cmp_owner(struct lockd_share *share, struct xdr_netobj *oh)
 	    && !memcmp(share->s_owner.data, oh->data, oh->len);
 }
 
+/*
+ * Recompute s_access / s_mode as the union of all positive refcount
+ * buckets.
+ */
+static void nlm_recompute_share(struct lockd_share *share)
+{
+	u32 new_access = 0, new_mode = 0, v;
+
+	for (v = 1; v < LOCKD_FSH_NR; v++) {
+		if (share->s_access_counts[v])
+			new_access |= v;
+		if (share->s_mode_counts[v])
+			new_mode |= v;
+	}
+	share->s_access = new_access;
+	share->s_mode = new_mode;
+}
+
 /**
  * nlmsvc_share_file - create a share
  * @host: Network client peer
@@ -64,12 +82,15 @@ nlmsvc_share_file(struct nlm_host *host, struct nlm_file *file,
 	share->s_host       = host;
 	share->s_owner.data = ohdata;
 	share->s_owner.len  = oh->len;
+	memset(share->s_access_counts, 0, sizeof(share->s_access_counts));
+	memset(share->s_mode_counts, 0, sizeof(share->s_mode_counts));
 	share->s_next       = file->f_shares;
 	file->f_shares      = share;
 
 update:
-	share->s_access = access;
-	share->s_mode = mode;
+	share->s_access_counts[access]++;
+	share->s_mode_counts[mode]++;
+	nlm_recompute_share(share);
 	return nlm_granted;
 }
 
@@ -78,12 +99,14 @@ nlmsvc_share_file(struct nlm_host *host, struct nlm_file *file,
  * @host: Network client peer
  * @file: File to be unshared
  * @oh: Share owner handle
+ * @access: Access mode of the SHARE being released
+ * @mode: Deny mode of the SHARE being released
  *
  * Returns an NLM status code.
  */
 __be32
 nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file,
-		    struct xdr_netobj *oh)
+		    struct xdr_netobj *oh, u32 access, u32 mode)
 {
 	struct lockd_share	*share, **shpp;
 
@@ -93,8 +116,15 @@ nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file,
 	for (shpp = &file->f_shares; (share = *shpp) != NULL;
 					shpp = &share->s_next) {
 		if (share->s_host == host && nlm_cmp_owner(share, oh)) {
-			*shpp = share->s_next;
-			kfree(share);
+			if (share->s_access_counts[access])
+				share->s_access_counts[access]--;
+			if (share->s_mode_counts[mode])
+				share->s_mode_counts[mode]--;
+			nlm_recompute_share(share);
+			if (!share->s_access && !share->s_mode) {
+				*shpp = share->s_next;
+				kfree(share);
+			}
 			return nlm_granted;
 		}
 	}
-- 
2.34.1


Disclaimer: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or phone and delete this message and its attachments, if any.