From: Olaf Kirch <okir@suse.de>
Subject: [PATCH 17/22] [lockd] match GRANTED_RES replies using cookies
Date: Sat, 5 Aug 2006 15:06:48 +0200
Message-ID: <20060805130648.GA8102@suse.de>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
To: nfs@lists.sourceforge.net
Sender: nfs-bounces@lists.sourceforge.net
Errors-To: nfs-bounces@lists.sourceforge.net

From: Olaf Kirch <okir@suse.de>
Subject: [lockd] match GRANTED_RES replies using cookies

 When we send a GRANTED_MSG call, we current copy the NLM cookie
 provided in the original LOCK call - because in 1996, some broken
 clients seemed to rely on this bug. However, this means the cookies
 are not unique, so that when the client's GRANTED_RES message comes
 back, we cannot simply match it based on the cookie, but have to
 use the client's IP address in addition. Which breaks when you have
 a multi-homed NFS client.
 
 The X/Open spec explicitly mentions that clients should not expect the
 same cookie; so one may hope that any clients that were broken in 1996
 have either been fixed or rendered obsolete.

Signed-off-by: Olaf Kirch <okir@suse.de>

 fs/lockd/svc4proc.c         |    2 +-
 fs/lockd/svclock.c          |   24 +++++++++++++-----------
 fs/lockd/svcproc.c          |    2 +-
 include/linux/lockd/lockd.h |    2 +-
 4 files changed, 16 insertions(+), 14 deletions(-)

Index: build/fs/lockd/svclock.c
===================================================================
--- build.orig/fs/lockd/svclock.c
+++ build/fs/lockd/svclock.c
@@ -136,19 +136,19 @@ static inline int nlm_cookie_match(struc
  * Find a block with a given NLM cookie.
  */
 static inline struct nlm_block *
-nlmsvc_find_block(struct nlm_cookie *cookie,  struct sockaddr_in *sin)
+nlmsvc_find_block(struct nlm_cookie *cookie)
 {
 	struct nlm_block *block;
 
 	list_for_each_entry(block, &nlm_blocked, b_list) {
-		if (nlm_cookie_match(&block->b_call->a_args.cookie,cookie)
-				&& nlm_cmp_addr(sin, &block->b_host->h_addr))
+		if (nlm_cookie_match(&block->b_call->a_args.cookie,cookie))
 			goto found;
 	}
 
 	return NULL;
 
 found:
+	dprintk("nlmsvc_find_block(%s): block=%p\n", nlmdbg_cookie2a(cookie), block);
 	kref_get(&block->b_count);
 	return block;
 }
@@ -162,6 +162,11 @@ found:
  * request, but (as I found out later) that's because some implementations
  * do just this. Never mind the standards comittees, they support our
  * logging industries.
+ *
+ * 10 years later: I hope we can safely ignore these old and broken
+ * clients by now. Let's fix this so we can uniquely identify an incoming
+ * GRANTED_RES message by cookie, without having to rely on the client's IP
+ * address. --okir
  */
 static inline struct nlm_block *
 nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_file *file,
@@ -194,7 +199,7 @@ nlmsvc_create_block(struct svc_rqst *rqs
 	/* Set notifier function for VFS, and init args */
 	call->a_args.lock.fl.fl_flags |= FL_SLEEP;
 	call->a_args.lock.fl.fl_lmops = &nlmsvc_lock_operations;
-	call->a_args.cookie = *cookie;	/* see above */
+	nlmclnt_next_cookie(&call->a_args.cookie);
 
 	dprintk("lockd: created block %p...\n", block);
 
@@ -650,17 +655,14 @@ static const struct rpc_call_ops nlmsvc_
  * block.
  */
 void
-nlmsvc_grant_reply(struct svc_rqst *rqstp, struct nlm_cookie *cookie, u32 status)
+nlmsvc_grant_reply(struct nlm_cookie *cookie, u32 status)
 {
 	struct nlm_block	*block;
-	struct nlm_file		*file;
 
-	dprintk("grant_reply: looking for cookie %x, host (%08x), s=%d \n", 
-		*(unsigned int *)(cookie->data), 
-		ntohl(rqstp->rq_addr.sin_addr.s_addr), status);
-	if (!(block = nlmsvc_find_block(cookie, &rqstp->rq_addr)))
+	dprintk("grant_reply: looking for cookie %x, s=%d \n", 
+		*(unsigned int *)(cookie->data), status);
+	if (!(block = nlmsvc_find_block(cookie)))
 		return;
-	file = block->b_file;
 
 	if (block) {
 		if (status == NLM_LCK_DENIED_GRACE_PERIOD) {
Index: build/fs/lockd/svc4proc.c
===================================================================
--- build.orig/fs/lockd/svc4proc.c
+++ build/fs/lockd/svc4proc.c
@@ -453,7 +453,7 @@ nlm4svc_proc_granted_res(struct svc_rqst
 
         dprintk("lockd: GRANTED_RES   called\n");
 
-        nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status);
+        nlmsvc_grant_reply(&argp->cookie, argp->status);
         return rpc_success;
 }
 
Index: build/fs/lockd/svcproc.c
===================================================================
--- build.orig/fs/lockd/svcproc.c
+++ build/fs/lockd/svcproc.c
@@ -482,7 +482,7 @@ nlmsvc_proc_granted_res(struct svc_rqst 
 
 	dprintk("lockd: GRANTED_RES   called\n");
 
-	nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status);
+	nlmsvc_grant_reply(&argp->cookie, argp->status);
 	return rpc_success;
 }
 
Index: build/include/linux/lockd/lockd.h
===================================================================
--- build.orig/include/linux/lockd/lockd.h
+++ build/include/linux/lockd/lockd.h
@@ -196,7 +196,7 @@ u32		  nlmsvc_cancel_blocked(struct nlm_
 unsigned long	  nlmsvc_retry_blocked(void);
 void		  nlmsvc_traverse_blocks(struct nlm_host *, struct nlm_file *,
 					nlm_host_match_fn_t match);
-void	  nlmsvc_grant_reply(struct svc_rqst *, struct nlm_cookie *, u32);
+void		  nlmsvc_grant_reply(struct nlm_cookie *, u32);
 
 /*
  * File handling for the server personality

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs