Commit 39be4502 authored by Olaf Kirch's avatar Olaf Kirch Committed by Linus Torvalds

[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: default avatarOlaf Kirch <okir@suse.de>
Signed-off-by: default avatarNeil Brown <neilb@suse.de>
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 031d869d
...@@ -455,7 +455,7 @@ nlm4svc_proc_granted_res(struct svc_rqst *rqstp, struct nlm_res *argp, ...@@ -455,7 +455,7 @@ nlm4svc_proc_granted_res(struct svc_rqst *rqstp, struct nlm_res *argp,
dprintk("lockd: GRANTED_RES called\n"); dprintk("lockd: GRANTED_RES called\n");
nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status); nlmsvc_grant_reply(&argp->cookie, argp->status);
return rpc_success; return rpc_success;
} }
......
...@@ -139,19 +139,19 @@ static inline int nlm_cookie_match(struct nlm_cookie *a, struct nlm_cookie *b) ...@@ -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. * Find a block with a given NLM cookie.
*/ */
static inline struct nlm_block * 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; struct nlm_block *block;
list_for_each_entry(block, &nlm_blocked, b_list) { list_for_each_entry(block, &nlm_blocked, b_list) {
if (nlm_cookie_match(&block->b_call->a_args.cookie,cookie) if (nlm_cookie_match(&block->b_call->a_args.cookie,cookie))
&& nlm_cmp_addr(sin, &block->b_host->h_addr))
goto found; goto found;
} }
return NULL; return NULL;
found: found:
dprintk("nlmsvc_find_block(%s): block=%p\n", nlmdbg_cookie2a(cookie), block);
kref_get(&block->b_count); kref_get(&block->b_count);
return block; return block;
} }
...@@ -165,6 +165,11 @@ nlmsvc_find_block(struct nlm_cookie *cookie, struct sockaddr_in *sin) ...@@ -165,6 +165,11 @@ nlmsvc_find_block(struct nlm_cookie *cookie, struct sockaddr_in *sin)
* request, but (as I found out later) that's because some implementations * request, but (as I found out later) that's because some implementations
* do just this. Never mind the standards comittees, they support our * do just this. Never mind the standards comittees, they support our
* logging industries. * 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 * static inline struct nlm_block *
nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_file *file, 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, ...@@ -197,7 +202,7 @@ nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_file *file,
/* Set notifier function for VFS, and init args */ /* Set notifier function for VFS, and init args */
call->a_args.lock.fl.fl_flags |= FL_SLEEP; call->a_args.lock.fl.fl_flags |= FL_SLEEP;
call->a_args.lock.fl.fl_lmops = &nlmsvc_lock_operations; 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); dprintk("lockd: created block %p...\n", block);
...@@ -640,17 +645,14 @@ static const struct rpc_call_ops nlmsvc_grant_ops = { ...@@ -640,17 +645,14 @@ static const struct rpc_call_ops nlmsvc_grant_ops = {
* block. * block.
*/ */
void 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_block *block;
struct nlm_file *file;
dprintk("grant_reply: looking for cookie %x, host (%08x), s=%d \n", dprintk("grant_reply: looking for cookie %x, s=%d \n",
*(unsigned int *)(cookie->data), *(unsigned int *)(cookie->data), status);
ntohl(rqstp->rq_addr.sin_addr.s_addr), status); if (!(block = nlmsvc_find_block(cookie)))
if (!(block = nlmsvc_find_block(cookie, &rqstp->rq_addr)))
return; return;
file = block->b_file;
if (block) { if (block) {
if (status == NLM_LCK_DENIED_GRACE_PERIOD) { if (status == NLM_LCK_DENIED_GRACE_PERIOD) {
......
...@@ -484,7 +484,7 @@ nlmsvc_proc_granted_res(struct svc_rqst *rqstp, struct nlm_res *argp, ...@@ -484,7 +484,7 @@ nlmsvc_proc_granted_res(struct svc_rqst *rqstp, struct nlm_res *argp,
dprintk("lockd: GRANTED_RES called\n"); dprintk("lockd: GRANTED_RES called\n");
nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status); nlmsvc_grant_reply(&argp->cookie, argp->status);
return rpc_success; return rpc_success;
} }
......
...@@ -193,7 +193,7 @@ u32 nlmsvc_cancel_blocked(struct nlm_file *, struct nlm_lock *); ...@@ -193,7 +193,7 @@ u32 nlmsvc_cancel_blocked(struct nlm_file *, struct nlm_lock *);
unsigned long nlmsvc_retry_blocked(void); unsigned long nlmsvc_retry_blocked(void);
void nlmsvc_traverse_blocks(struct nlm_host *, struct nlm_file *, void nlmsvc_traverse_blocks(struct nlm_host *, struct nlm_file *,
nlm_host_match_fn_t match); 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 * File handling for the server personality
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment