From: Olaf Kirch <[email protected]>
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 <[email protected]>
Signed-off-by: Neil Brown <[email protected]>
### Diffstat output
./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(-)
diff .prev/fs/lockd/svc4proc.c ./fs/lockd/svc4proc.c
--- .prev/fs/lockd/svc4proc.c 2006-09-01 12:11:01.000000000 +1000
+++ ./fs/lockd/svc4proc.c 2006-09-01 12:17:21.000000000 +1000
@@ -455,7 +455,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;
}
diff .prev/fs/lockd/svclock.c ./fs/lockd/svclock.c
--- .prev/fs/lockd/svclock.c 2006-09-01 12:11:01.000000000 +1000
+++ ./fs/lockd/svclock.c 2006-09-01 12:17:21.000000000 +1000
@@ -139,19 +139,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;
}
@@ -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 *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);
@@ -640,17 +645,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) {
diff .prev/fs/lockd/svcproc.c ./fs/lockd/svcproc.c
--- .prev/fs/lockd/svcproc.c 2006-09-01 12:11:01.000000000 +1000
+++ ./fs/lockd/svcproc.c 2006-09-01 12:17:21.000000000 +1000
@@ -484,7 +484,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;
}
diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h 2006-09-01 12:17:03.000000000 +1000
+++ ./include/linux/lockd/lockd.h 2006-09-01 12:17:21.000000000 +1000
@@ -193,7 +193,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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
