From cdead7cf12896c0e50a8be2e52de52c364603095 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 19 Mar 2010 15:36:22 -0400 Subject: SUNRPC: Fix a potential memory leak in auth_gss The function alloc_enc_pages() currently fails to release the pointer rqstp->rq_enc_pages in the error path. Signed-off-by: Trond Myklebust Acked-by: J. Bruce Fields Cc: stable@kernel.org --- net/sunrpc/auth_gss/auth_gss.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'net') diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c index 0cfccc2a..c389ccf 100644 --- a/net/sunrpc/auth_gss/auth_gss.c +++ b/net/sunrpc/auth_gss/auth_gss.c @@ -1280,9 +1280,8 @@ alloc_enc_pages(struct rpc_rqst *rqstp) rqstp->rq_release_snd_buf = priv_release_snd_buf; return 0; out_free: - for (i--; i >= 0; i--) { - __free_page(rqstp->rq_enc_pages[i]); - } + rqstp->rq_enc_pages_num = i; + priv_release_snd_buf(rqstp); out: return -EAGAIN; } -- cgit v1.1 From c9acb42ef1904d15d0fb315061cefbe638f67f3a Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 19 Mar 2010 15:36:22 -0400 Subject: SUNRPC: Fix a use after free bug with the NFSv4.1 backchannel The ->release_request() callback was designed to allow the transport layer to do housekeeping after the RPC call is done. It cannot be used to free the request itself, and doing so leads to a use-after-free bug in xprt_release(). Signed-off-by: Trond Myklebust --- net/sunrpc/bc_svc.c | 15 --------------- net/sunrpc/xprt.c | 22 +++++++++------------- net/sunrpc/xprtsock.c | 3 --- 3 files changed, 9 insertions(+), 31 deletions(-) (limited to 'net') diff --git a/net/sunrpc/bc_svc.c b/net/sunrpc/bc_svc.c index 13f214f..f0c05d3 100644 --- a/net/sunrpc/bc_svc.c +++ b/net/sunrpc/bc_svc.c @@ -37,21 +37,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #define RPCDBG_FACILITY RPCDBG_SVCDSP -void bc_release_request(struct rpc_task *task) -{ - struct rpc_rqst *req = task->tk_rqstp; - - dprintk("RPC: bc_release_request: task= %p\n", task); - - /* - * Release this request only if it's a backchannel - * preallocated request - */ - if (!bc_prealloc(req)) - return; - xprt_free_bc_request(req); -} - /* Empty callback ops */ static const struct rpc_call_ops nfs41_callback_ops = { }; diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c index 469de29..42f09ad 100644 --- a/net/sunrpc/xprt.c +++ b/net/sunrpc/xprt.c @@ -46,6 +46,7 @@ #include #include +#include #include "sunrpc.h" @@ -1032,21 +1033,16 @@ void xprt_release(struct rpc_task *task) if (req->rq_release_snd_buf) req->rq_release_snd_buf(req); - /* - * Early exit if this is a backchannel preallocated request. - * There is no need to have it added to the RPC slot list. - */ - if (is_bc_request) - return; - - memset(req, 0, sizeof(*req)); /* mark unused */ - dprintk("RPC: %5u release request %p\n", task->tk_pid, req); + if (likely(!is_bc_request)) { + memset(req, 0, sizeof(*req)); /* mark unused */ - spin_lock(&xprt->reserve_lock); - list_add(&req->rq_list, &xprt->free); - rpc_wake_up_next(&xprt->backlog); - spin_unlock(&xprt->reserve_lock); + spin_lock(&xprt->reserve_lock); + list_add(&req->rq_list, &xprt->free); + rpc_wake_up_next(&xprt->backlog); + spin_unlock(&xprt->reserve_lock); + } else + xprt_free_bc_request(req); } /** diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index e4839c0..9847c30 100644 --- a/net/sunrpc/xprtsock.c +++ b/net/sunrpc/xprtsock.c @@ -2251,9 +2251,6 @@ static struct rpc_xprt_ops xs_tcp_ops = { .buf_free = rpc_free, .send_request = xs_tcp_send_request, .set_retrans_timeout = xprt_set_retrans_timeout_def, -#if defined(CONFIG_NFS_V4_1) - .release_request = bc_release_request, -#endif /* CONFIG_NFS_V4_1 */ .close = xs_tcp_close, .destroy = xs_destroy, .print_stats = xs_tcp_print_stats, -- cgit v1.1 From ff0901f8036a1586037c30a365c9666e946af0f1 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 19 Mar 2010 16:17:45 -0400 Subject: SUNRPC: Fix the return value of rpc_run_bc_task() Currently rpc_run_bc_task() will return NULL if the task allocation failed. However the only caller is bc_send, which assumes that the return value will be an ERR_PTR. Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net') diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 154034b..19c9983 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -659,6 +659,7 @@ struct rpc_task *rpc_run_bc_task(struct rpc_rqst *req, task = rpc_new_task(&task_setup_data); if (!task) { xprt_free_bc_request(req); + task = ERR_PTR(-ENOMEM); goto out; } task->tk_rqstp = req; -- cgit v1.1 From f1f0abe192a72e75d7c59972e30784d043fd8d73 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sun, 21 Mar 2010 12:10:34 -0400 Subject: sunrpc: handle allocation errors from __rpc_lookup_create() __rpc_lookup_create() can return ERR_PTR(-ENOMEM). Signed-off-by: Dan Carpenter Signed-off-by: Trond Myklebust Cc: stable@kernel.org --- net/sunrpc/rpc_pipe.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'net') diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index 8d63f8f..20e30c6 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -587,6 +587,8 @@ static struct dentry *__rpc_lookup_create_exclusive(struct dentry *parent, struct dentry *dentry; dentry = __rpc_lookup_create(parent, name); + if (IS_ERR(dentry)) + return dentry; if (dentry->d_inode == NULL) return dentry; dput(dentry); -- cgit v1.1