lkml.org 
[lkml]   [2020]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] SUNRPC: Fix refcnt leak in rpc_clnt_test_and_add_xprt
Date
On Mon, 2020-04-20 at 13:45 +0800, Xiyu Yang wrote:
> rpc_clnt_test_and_add_xprt() invokes xprt_switch_get() and
> xprt_get(),
> which returns a reference of the rpc_xprt_switch object to "data-
> >xps"
> and a reference of the rpc_xprt object to "data->xprt" with increased
> refcount.
>
> When rpc_clnt_test_and_add_xprt() returns, local variable "data" and
> its
> field "xps" as well as "xprt" becomes invalid, so their refcounts
> should
> be decreased to keep refcount balanced.
>
> The reference counting issue happens in one exception handling paths
> of
> rpc_clnt_test_and_add_xprt(). When rpc_call_null_helper() returns
> IS_ERR, the refcnt increased by xprt_switch_get() and xprt_get() are
> not
> decreased, causing a refcnt leak.
>
> Fix this issue by calling rpc_cb_add_xprt_release() to decrease
> related
> refcounted fields in "data" and then release it when
> rpc_call_null_helper() returns IS_ERR.
>
> Fixes: 7f554890587c ("SUNRPC: Allow addition of new transports to a
> struct rpc_clnt")
> Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
> ---
> net/sunrpc/clnt.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 7324b21f923e..f86d9ae2167f 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -2803,8 +2803,10 @@ int rpc_clnt_test_and_add_xprt(struct rpc_clnt
> *clnt,
> task = rpc_call_null_helper(clnt, xprt, NULL,
> RPC_TASK_SOFT|RPC_TASK_SOFTCONN|RPC_TASK_ASYNC|
> RPC_TASK_NULLCREDS,
> &rpc_cb_add_xprt_call_ops, data);
> - if (IS_ERR(task))
> + if (IS_ERR(task)) {
> + rpc_cb_add_xprt_release(data);
> return PTR_ERR(task);

We should just get rid of the IS_ERR() condition here. It cannot ever
trigger, which is why rpc_run_task() no longer checks for it, and no
longer calls the ->rpc_release() method on error.

> + }
> rpc_put_task(task);
> success:
> return 1;
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com


\
 
 \ /
  Last update: 2020-04-20 13:58    [W:0.039 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site