diff options
| author | Trond Myklebust <Trond.Myklebust@netapp.com> | 2013-11-12 17:24:36 -0500 |
|---|---|---|
| committer | Trond Myklebust <Trond.Myklebust@netapp.com> | 2013-11-12 18:56:57 -0500 |
| commit | d07ba8422f1e58be94cc98a1f475946dc1b89f1b (patch) | |
| tree | e83c021ca30a8625b7a4af139c2d7bf16bbfa348 | |
| parent | a6b31d18b02ff9d7915c5898c9b5ca41a798cd73 (diff) | |
SUNRPC: Avoid deep recursion in rpc_release_client
In cases where an rpc client has a parent hierarchy, then
rpc_free_client may end up calling rpc_release_client() on the
parent, thus recursing back into rpc_free_client. If the hierarchy
is deep enough, then we can get into situations where the stack
simply overflows.
The fix is to have rpc_release_client() loop so that it can take
care of the parent rpc client hierarchy without needing to
recurse.
Reported-by: Jeff Layton <jlayton@redhat.com>
Reported-by: Weston Andros Adamson <dros@netapp.com>
Reported-by: Bruce Fields <bfields@fieldses.org>
Link: http://lkml.kernel.org/r/2C73011F-0939-434C-9E4D-13A1EB1403D7@netapp.com
Cc: stable@vger.kernel.org
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
| -rw-r--r-- | net/sunrpc/clnt.c | 29 |
1 files changed, 17 insertions, 12 deletions
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index dab09dac8fc7..f09b7db2c492 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c | |||
| @@ -750,14 +750,16 @@ EXPORT_SYMBOL_GPL(rpc_shutdown_client); | |||
| 750 | /* | 750 | /* |
| 751 | * Free an RPC client | 751 | * Free an RPC client |
| 752 | */ | 752 | */ |
| 753 | static void | 753 | static struct rpc_clnt * |
| 754 | rpc_free_client(struct rpc_clnt *clnt) | 754 | rpc_free_client(struct rpc_clnt *clnt) |
| 755 | { | 755 | { |
| 756 | struct rpc_clnt *parent = NULL; | ||
| 757 | |||
| 756 | dprintk_rcu("RPC: destroying %s client for %s\n", | 758 | dprintk_rcu("RPC: destroying %s client for %s\n", |
| 757 | clnt->cl_program->name, | 759 | clnt->cl_program->name, |
| 758 | rcu_dereference(clnt->cl_xprt)->servername); | 760 | rcu_dereference(clnt->cl_xprt)->servername); |
| 759 | if (clnt->cl_parent != clnt) | 761 | if (clnt->cl_parent != clnt) |
| 760 | rpc_release_client(clnt->cl_parent); | 762 | parent = clnt->cl_parent; |
| 761 | rpc_clnt_remove_pipedir(clnt); | 763 | rpc_clnt_remove_pipedir(clnt); |
| 762 | rpc_unregister_client(clnt); | 764 | rpc_unregister_client(clnt); |
| 763 | rpc_free_iostats(clnt->cl_metrics); | 765 | rpc_free_iostats(clnt->cl_metrics); |
| @@ -766,18 +768,17 @@ rpc_free_client(struct rpc_clnt *clnt) | |||
| 766 | rpciod_down(); | 768 | rpciod_down(); |
| 767 | rpc_free_clid(clnt); | 769 | rpc_free_clid(clnt); |
| 768 | kfree(clnt); | 770 | kfree(clnt); |
| 771 | return parent; | ||
| 769 | } | 772 | } |
| 770 | 773 | ||
| 771 | /* | 774 | /* |
| 772 | * Free an RPC client | 775 | * Free an RPC client |
| 773 | */ | 776 | */ |
| 774 | static void | 777 | static struct rpc_clnt * |
| 775 | rpc_free_auth(struct rpc_clnt *clnt) | 778 | rpc_free_auth(struct rpc_clnt *clnt) |
| 776 | { | 779 | { |
| 777 | if (clnt->cl_auth == NULL) { | 780 | if (clnt->cl_auth == NULL) |
| 778 | rpc_free_client(clnt); | 781 | return rpc_free_client(clnt); |
| 779 | return; | ||
| 780 | } | ||
| 781 | 782 | ||
| 782 | /* | 783 | /* |
| 783 | * Note: RPCSEC_GSS may need to send NULL RPC calls in order to | 784 | * Note: RPCSEC_GSS may need to send NULL RPC calls in order to |
| @@ -788,7 +789,8 @@ rpc_free_auth(struct rpc_clnt *clnt) | |||
| 788 | rpcauth_release(clnt->cl_auth); | 789 | rpcauth_release(clnt->cl_auth); |
| 789 | clnt->cl_auth = NULL; | 790 | clnt->cl_auth = NULL; |
| 790 | if (atomic_dec_and_test(&clnt->cl_count)) | 791 | if (atomic_dec_and_test(&clnt->cl_count)) |
| 791 | rpc_free_client(clnt); | 792 | return rpc_free_client(clnt); |
| 793 | return NULL; | ||
| 792 | } | 794 | } |
| 793 | 795 | ||
| 794 | /* | 796 | /* |
| @@ -799,10 +801,13 @@ rpc_release_client(struct rpc_clnt *clnt) | |||
| 799 | { | 801 | { |
| 800 | dprintk("RPC: rpc_release_client(%p)\n", clnt); | 802 | dprintk("RPC: rpc_release_client(%p)\n", clnt); |
| 801 | 803 | ||
| 802 | if (list_empty(&clnt->cl_tasks)) | 804 | do { |
| 803 | wake_up(&destroy_wait); | 805 | if (list_empty(&clnt->cl_tasks)) |
| 804 | if (atomic_dec_and_test(&clnt->cl_count)) | 806 | wake_up(&destroy_wait); |
| 805 | rpc_free_auth(clnt); | 807 | if (!atomic_dec_and_test(&clnt->cl_count)) |
| 808 | break; | ||
| 809 | clnt = rpc_free_auth(clnt); | ||
| 810 | } while (clnt != NULL); | ||
| 806 | } | 811 | } |
| 807 | EXPORT_SYMBOL_GPL(rpc_release_client); | 812 | EXPORT_SYMBOL_GPL(rpc_release_client); |
| 808 | 813 | ||
