[PATCH] knfsd: 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>
Signed-off-by: Neil Brown <neilb@suse.de>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

4 files changed, 16 insertions, 14 deletions

diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index b8525fb62934..fa370f6eb07b 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -455,7 +455,7 @@ nlm4svc_proc_granted_res(struct svc_rqst *rqstp, struct nlm_res  *argp,
 
         dprintk("lockd: GRANTED_RES   called\n");
 
-        nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status);
+        nlmsvc_grant_reply(&argp->cookie, argp->status);
         return rpc_success;
 }
 
diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 1f91567a1b88..3d2b8a831be5 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -139,19 +139,19 @@ static inline int nlm_cookie_match(struct nlm_cookie *a, struct nlm_cookie *b)
  * 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;
 }
@@ -165,6 +165,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,
@@ -197,7 +202,7 @@ nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_file *file,
         /* 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);
 
@@ -640,17 +645,14 @@ static const struct rpc_call_ops nlmsvc_grant_ops = {
  * 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) {
diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
index 66e7b0b3430e..75b2c81bcb93 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -484,7 +484,7 @@ nlmsvc_proc_granted_res(struct svc_rqst *rqstp, struct nlm_res  *argp,
 
         dprintk("lockd: GRANTED_RES   called\n");
 
-        nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status);
+        nlmsvc_grant_reply(&argp->cookie, argp->status);
         return rpc_success;
 }
 
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index a345650e5622..5920ecaeed66 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -193,7 +193,7 @@ u32		  nlmsvc_cancel_blocked(struct nlm_file *, struct nlm_lock *);
 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