From d343fce148a4eee24a907a05c4101d3268045aae Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Tue, 17 Oct 2006 00:10:18 -0700 Subject: [PATCH] knfsd: Allow lockd to drop replies as appropriate It is possible for the ->fopen callback from lockd into nfsd to find that an answer cannot be given straight away (an upcall is needed) and so the request has to be 'dropped', to be retried later. That error status is not currently propagated back. So: Change nlm_fopen to return nlm error codes (rather than a private protocol) and define a new nlm_drop_reply code. Cause nlm_drop_reply to cause the rpc request to get rpc_drop_reply when this error comes back. Cause svc_process to drop a request which returns a status of rpc_drop_reply. [akpm@osdl.org: fix warning storm] Cc: Marc Eshel Signed-off-by: Neil Brown Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- net/sunrpc/svc.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'net/sunrpc') diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index 2807fa0eab40..eb44ec929ca1 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -828,6 +828,11 @@ svc_process(struct svc_rqst *rqstp) *statp = procp->pc_func(rqstp, rqstp->rq_argp, rqstp->rq_resp); /* Encode reply */ + if (*statp == rpc_drop_reply) { + if (procp->pc_release) + procp->pc_release(rqstp, NULL, rqstp->rq_resp); + goto dropit; + } if (*statp == rpc_success && (xdr = procp->pc_encode) && !xdr(rqstp, resv->iov_base+resv->iov_len, rqstp->rq_resp)) { dprintk("svc: failed to encode reply\n"); -- cgit v1.2.3 From 71bdcf8056f910dc57ea3d0def80a9329e7dc52d Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 19 Oct 2006 23:28:43 -0700 Subject: [PATCH] SUNRPC: fix race in in-kernel RPC portmapper client When submitting a request to a fast portmapper (such as the local rpcbind daemon), the request can complete before the parent task is even queued up on xprt->binding. Fix this by queuing before submitting the rpcbind request. Test plan: Connectathon locking test with UDP. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- net/sunrpc/pmap_clnt.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'net/sunrpc') diff --git a/net/sunrpc/pmap_clnt.c b/net/sunrpc/pmap_clnt.c index 919d5ba7ca0a..e52afab413de 100644 --- a/net/sunrpc/pmap_clnt.c +++ b/net/sunrpc/pmap_clnt.c @@ -101,11 +101,13 @@ void rpc_getport(struct rpc_task *task) /* Autobind on cloned rpc clients is discouraged */ BUG_ON(clnt->cl_parent != clnt); - if (xprt_test_and_set_binding(xprt)) { - task->tk_status = -EACCES; /* tell caller to check again */ - rpc_sleep_on(&xprt->binding, task, NULL, NULL); - return; - } + /* Put self on queue before sending rpcbind request, in case + * pmap_getport_done completes before we return from rpc_run_task */ + rpc_sleep_on(&xprt->binding, task, NULL, NULL); + + status = -EACCES; /* tell caller to check again */ + if (xprt_test_and_set_binding(xprt)) + goto bailout_nofree; /* Someone else may have bound if we slept */ status = 0; @@ -134,8 +136,6 @@ void rpc_getport(struct rpc_task *task) goto bailout; rpc_release_task(child); - rpc_sleep_on(&xprt->binding, task, NULL, NULL); - task->tk_xprt->stat.bind_count++; return; -- cgit v1.2.3 From b7766da7f7395b74dec9e52005b7dac0d09391a4 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 19 Oct 2006 23:28:44 -0700 Subject: [PATCH] SUNRPC: fix a typo Yes, this actually passed tests the way it was. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- net/sunrpc/xprtsock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sunrpc') diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index 28100e019225..757fc91ef25d 100644 --- a/net/sunrpc/xprtsock.c +++ b/net/sunrpc/xprtsock.c @@ -1366,7 +1366,7 @@ int xs_setup_udp(struct rpc_xprt *xprt, struct rpc_timeout *to) if (xprt->slot == NULL) return -ENOMEM; - if (ntohs(addr->sin_port != 0)) + if (ntohs(addr->sin_port) != 0) xprt_set_bound(xprt); xprt->port = xs_get_random_port(); -- cgit v1.2.3 From 1a047060a99f274a7c52cfea8159e4142a14b8a7 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Thu, 19 Oct 2006 23:29:13 -0700 Subject: [PATCH] knfsd: fix race that can disable NFS server This patch is suitable for just about any 2.6 kernel. It should go in 2.6.19 and 2.6.18.2 and possible even the .17 and .16 stable series. This is a long standing bug that seems to have only recently become apparent, presumably due to increasing use of NFS over TCP - many distros seem to be making it the default. The SK_CONN bit gets set when a listening socket may be ready for an accept, just as SK_DATA is set when data may be available. It is entirely possible for svc_tcp_accept to be called with neither of these set. It doesn't happen often but there is a small race in svc_sock_enqueue as SK_CONN and SK_DATA are tested outside the spin_lock. They could be cleared immediately after the test and before the lock is gained. This normally shouldn't be a problem. The sockets are non-blocking so trying to read() or accept() when ther is nothing to do is not a problem. However: svc_tcp_recvfrom makes the decision "Should I accept() or should I read()" based on whether SK_CONN is set or not. This usually works but is not safe. The decision should be based on whether it is a TCP_LISTEN socket or a TCP_CONNECTED socket. Signed-off-by: Neil Brown Cc: Adrian Bunk Cc: Cc: Trond Myklebust Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- net/sunrpc/svcsock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sunrpc') diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c index 61e307cca13d..96521f16342b 100644 --- a/net/sunrpc/svcsock.c +++ b/net/sunrpc/svcsock.c @@ -973,7 +973,7 @@ svc_tcp_recvfrom(struct svc_rqst *rqstp) return 0; } - if (test_bit(SK_CONN, &svsk->sk_flags)) { + if (svsk->sk_sk->sk_state == TCP_LISTEN) { svc_tcp_accept(svsk); svc_sock_received(svsk); return 0; -- cgit v1.2.3 From d6740df98e12a8e49ef3a699dcc1e2913f22c51b Mon Sep 17 00:00:00 2001 From: Neil Brown Date: Sun, 29 Oct 2006 22:46:45 -0800 Subject: [PATCH] sunrpc: fix refcounting problems in rpc servers A recent patch fixed a problem which would occur when the refcount on an auth_domain reached zero. This problem has not been reported in practice despite existing in two major kernel releases because the refcount can never reach zero. This patch fixes the problems that stop the refcount reaching zero. 1/ We were adding to the refcount when inserting in the hash table, but only removing from the hashtable when the refcount reached zero. Obviously it never would. So don't count the implied reference of being in the hash table. 2/ There are two paths on which a socket can be destroyed. One called svcauth_unix_info_release(). The other didn't. So when the other was taken, we can lose a reference to an ip_map which in-turn holds a reference to an auth_domain So unify the exit paths into svc_sock_put. This highlights the fact that svc_delete_socket has slightly odd semantics - it does not drop a reference but probably should. Fixing this need a bit more thought and testing. Signed-off-by: Neil Brown Cc: Trond Myklebust Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- net/sunrpc/svcauth.c | 4 +--- net/sunrpc/svcsock.c | 30 ++++++++++++++---------------- 2 files changed, 15 insertions(+), 19 deletions(-) (limited to 'net/sunrpc') diff --git a/net/sunrpc/svcauth.c b/net/sunrpc/svcauth.c index 8f2320aded5c..0004c1f0ef04 100644 --- a/net/sunrpc/svcauth.c +++ b/net/sunrpc/svcauth.c @@ -147,10 +147,8 @@ auth_domain_lookup(char *name, struct auth_domain *new) return hp; } } - if (new) { + if (new) hlist_add_head(&new->hash, head); - kref_get(&new->ref); - } spin_unlock(&auth_domain_lock); return new; } diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c index 96521f16342b..db0d1048d466 100644 --- a/net/sunrpc/svcsock.c +++ b/net/sunrpc/svcsock.c @@ -300,8 +300,13 @@ static inline void svc_sock_put(struct svc_sock *svsk) { if (atomic_dec_and_test(&svsk->sk_inuse) && test_bit(SK_DEAD, &svsk->sk_flags)) { - dprintk("svc: releasing dead socket\n"); - sock_release(svsk->sk_sock); + printk("svc: releasing dead socket\n"); + if (svsk->sk_sock->file) + sockfd_put(svsk->sk_sock); + else + sock_release(svsk->sk_sock); + if (svsk->sk_info_authunix != NULL) + svcauth_unix_info_release(svsk->sk_info_authunix); kfree(svsk); } } @@ -1604,20 +1609,13 @@ svc_delete_socket(struct svc_sock *svsk) if (test_bit(SK_TEMP, &svsk->sk_flags)) serv->sv_tmpcnt--; - if (!atomic_read(&svsk->sk_inuse)) { - spin_unlock_bh(&serv->sv_lock); - if (svsk->sk_sock->file) - sockfd_put(svsk->sk_sock); - else - sock_release(svsk->sk_sock); - if (svsk->sk_info_authunix != NULL) - svcauth_unix_info_release(svsk->sk_info_authunix); - kfree(svsk); - } else { - spin_unlock_bh(&serv->sv_lock); - dprintk(KERN_NOTICE "svc: server socket destroy delayed\n"); - /* svsk->sk_server = NULL; */ - } + /* This atomic_inc should be needed - svc_delete_socket + * should have the semantic of dropping a reference. + * But it doesn't yet.... + */ + atomic_inc(&svsk->sk_inuse); + spin_unlock_bh(&serv->sv_lock); + svc_sock_put(svsk); } /* -- cgit v1.2.3 From 202dd450246cd658e298a86dddca5592fcb898cd Mon Sep 17 00:00:00 2001 From: Andrew Morton Date: Sun, 29 Oct 2006 22:57:57 -0800 Subject: [PATCH] fix "sunrpc: fix refcounting problems in rpc servers" - printk should remain dprintk - fix coding-style. Cc: Neil Brown Cc: Trond Myklebust Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- net/sunrpc/svcsock.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'net/sunrpc') diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c index db0d1048d466..64ca1f61dd94 100644 --- a/net/sunrpc/svcsock.c +++ b/net/sunrpc/svcsock.c @@ -299,8 +299,9 @@ void svc_reserve(struct svc_rqst *rqstp, int space) static inline void svc_sock_put(struct svc_sock *svsk) { - if (atomic_dec_and_test(&svsk->sk_inuse) && test_bit(SK_DEAD, &svsk->sk_flags)) { - printk("svc: releasing dead socket\n"); + if (atomic_dec_and_test(&svsk->sk_inuse) && + test_bit(SK_DEAD, &svsk->sk_flags)) { + dprintk("svc: releasing dead socket\n"); if (svsk->sk_sock->file) sockfd_put(svsk->sk_sock); else -- cgit v1.2.3 From 0c7bb31db0e35d4b772fac452b722460ca368acf Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sun, 5 Nov 2006 23:52:13 -0800 Subject: [PATCH] sunrpc: add missing spin_unlock auth_domain_put() forgot to unlock acquired spinlock. Cc: Olaf Kirch Cc: Andy Adamson Cc: J. Bruce Fields Acked-by: Trond Myklebust Signed-off-by: Akinobu Mita Cc: Neil Brown Acked-by: Peter Zijlstra Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- net/sunrpc/svcauth.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net/sunrpc') diff --git a/net/sunrpc/svcauth.c b/net/sunrpc/svcauth.c index 0004c1f0ef04..ee9bb1522d5e 100644 --- a/net/sunrpc/svcauth.c +++ b/net/sunrpc/svcauth.c @@ -126,6 +126,7 @@ void auth_domain_put(struct auth_domain *dom) if (atomic_dec_and_lock(&dom->ref.refcount, &auth_domain_lock)) { hlist_del(&dom->hash); dom->flavour->domain_release(dom); + spin_unlock(&auth_domain_lock); } } -- cgit v1.2.3