This patch series fixes an auth_gss bug that results in netns refcount
leaks when use-gss-proxy is set to 1.
The problem was found in privileged docker containers with gssproxy service
enabled and /proc/net/rpc/use-gss-proxy set to 1, the corresponding
struct net->count ends up at 2 after container gets killed, the consequence
is that the struct net cannot be freed.
It turns out that write_gssp() called gssp_rpc_create() to create a rpc
client, this increases net->count by 2; rpcsec_gss_exit_net() is supposed
to decrease net->count but it never gets called because its call-path is:
net->count==0 -> cleanup_net -> ops_exit_list -> rpcsec_gss_exit_net
Before rpcsec_gss_exit_net() gets called, net->count cannot reach 0, this
is a deadlock situation.
To fix the problem, we must break the deadlock, rpcsec_gss_exit_net()
should move out of the put() path and find another chance to get called,
I think nsfs_evict() is a good place to go, when netns inode gets evicted
we call rpcsec_gss_exit_net() to free the rpc client, this requires a new
callback i.e. evict to be added in struct proc_ns_operations, and add
netns_evict() as one of netns_operations as well.
v1->v2:
* in nsfs_evict(), move ->evict() in front of ->put()
v2->v3:
* rpcsec_gss_evict_net() directly call gss_svc_shutdown_net() regardless
if gssp_clnt is null, this is exactly same to what rpcsec_gss_exit_net()
previously did
Wenbin Zeng (3):
nsfs: add evict callback into struct proc_ns_operations
netns: add netns_evict into netns_operations
auth_gss: fix deadlock that blocks rpcsec_gss_exit_net when
use-gss-proxy==1
fs/nsfs.c | 2 ++
include/linux/proc_ns.h | 1 +
include/net/net_namespace.h | 1 +
net/core/net_namespace.c | 12 ++++++++++++
net/sunrpc/auth_gss/auth_gss.c | 4 ++--
5 files changed, 18 insertions(+), 2 deletions(-)
--
1.8.3.1
The newly added evict callback shall be called by nsfs_evict(). Currently
only put() callback is called in nsfs_evict(), it is not able to release
all netns refcount, for example, a rpc client holds two netns refcounts,
these refcounts are supposed to be released when the rpc client is freed,
but the code to free rpc client is normally triggered by put() callback
only when netns refcount gets to 0, specifically:
refcount=0 -> cleanup_net() -> ops_exit_list -> free rpc client
But netns refcount will never get to 0 before rpc client gets freed, to
break the deadlock, the code to free rpc client can be put into the newly
added evict callback.
Signed-off-by: Wenbin Zeng <[email protected]>
Cc: Al Viro <[email protected]>
---
fs/nsfs.c | 2 ++
include/linux/proc_ns.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/fs/nsfs.c b/fs/nsfs.c
index 60702d6..a122288 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -48,6 +48,8 @@ static void nsfs_evict(struct inode *inode)
{
struct ns_common *ns = inode->i_private;
clear_inode(inode);
+ if (ns->ops->evict)
+ ns->ops->evict(ns);
ns->ops->put(ns);
}
diff --git a/include/linux/proc_ns.h b/include/linux/proc_ns.h
index d31cb62..919f0d4 100644
--- a/include/linux/proc_ns.h
+++ b/include/linux/proc_ns.h
@@ -19,6 +19,7 @@ struct proc_ns_operations {
int type;
struct ns_common *(*get)(struct task_struct *task);
void (*put)(struct ns_common *ns);
+ void (*evict)(struct ns_common *ns);
int (*install)(struct nsproxy *nsproxy, struct ns_common *ns);
struct user_namespace *(*owner)(struct ns_common *ns);
struct ns_common *(*get_parent)(struct ns_common *ns);
--
1.8.3.1
The newly added netns_evict() shall be called when the netns inode being
evicted. It provides another path to release netns refcounts, previously
netns_put() is the only choice, but it is not able to release all netns
refcount, for example, a rpc client holds two netns refcounts, these
refcounts are supposed to be released when the rpc client is freed, but
the code to free rpc client is normally triggered by put() callback only
when netns refcount gets to 0, specifically:
refcount=0 -> cleanup_net() -> ops_exit_list -> free rpc client
But netns refcount will never get to 0 before rpc client gets freed, to
break the deadlock, the code to free rpc client can be put into the newly
added netns_evict.
Signed-off-by: Wenbin Zeng <[email protected]>
Acked-by: David S. Miller <[email protected]>
---
include/net/net_namespace.h | 1 +
net/core/net_namespace.c | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 12689dd..c44306a 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -357,6 +357,7 @@ struct pernet_operations {
int (*init)(struct net *net);
void (*exit)(struct net *net);
void (*exit_batch)(struct list_head *net_exit_list);
+ void (*evict)(struct net *net);
unsigned int *id;
size_t size;
};
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 7e6dcc6..0626fc4 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -1296,6 +1296,17 @@ static void netns_put(struct ns_common *ns)
put_net(to_net_ns(ns));
}
+static void netns_evict(struct ns_common *ns)
+{
+ struct net *net = to_net_ns(ns);
+ const struct pernet_operations *ops;
+
+ list_for_each_entry_reverse(ops, &pernet_list, list) {
+ if (ops->evict)
+ ops->evict(net);
+ }
+}
+
static int netns_install(struct nsproxy *nsproxy, struct ns_common *ns)
{
struct net *net = to_net_ns(ns);
@@ -1319,6 +1330,7 @@ static struct user_namespace *netns_owner(struct ns_common *ns)
.type = CLONE_NEWNET,
.get = netns_get,
.put = netns_put,
+ .evict = netns_evict,
.install = netns_install,
.owner = netns_owner,
};
--
1.8.3.1
When use-gss-proxy is set to 1, write_gssp() creates a rpc client in
gssp_rpc_create(), this increases netns refcount by 2, these refcounts are
supposed to be released in rpcsec_gss_exit_net(), but it will never happen
because rpcsec_gss_exit_net() is triggered only when netns refcount gets
to 0, specifically:
refcount=0 -> cleanup_net() -> ops_exit_list -> rpcsec_gss_exit_net
It is a deadlock situation here, refcount will never get to 0 unless
rpcsec_gss_exit_net() is called.
This fix introduced a new callback i.e. evict in struct proc_ns_operations,
which is called in nsfs_evict. Moving rpcsec_gss_exit_net to evict path
gives it a chance to get called and avoids the above deadlock situation.
Signed-off-by: Wenbin Zeng <[email protected]>
Cc: J. Bruce Fields <[email protected]>
---
net/sunrpc/auth_gss/auth_gss.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 3fd56c0..3e76c8a 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -2136,14 +2136,14 @@ static __net_init int rpcsec_gss_init_net(struct net *net)
return gss_svc_init_net(net);
}
-static __net_exit void rpcsec_gss_exit_net(struct net *net)
+static void rpcsec_gss_evict_net(struct net *net)
{
gss_svc_shutdown_net(net);
}
static struct pernet_operations rpcsec_gss_net_ops = {
.init = rpcsec_gss_init_net,
- .exit = rpcsec_gss_exit_net,
+ .evict = rpcsec_gss_evict_net,
};
/*
--
1.8.3.1
I lost track, what happened to these patches?
--b.
On Wed, Jun 12, 2019 at 08:09:27PM +0800, Wenbin Zeng wrote:
> This patch series fixes an auth_gss bug that results in netns refcount
> leaks when use-gss-proxy is set to 1.
>
> The problem was found in privileged docker containers with gssproxy service
> enabled and /proc/net/rpc/use-gss-proxy set to 1, the corresponding
> struct net->count ends up at 2 after container gets killed, the consequence
> is that the struct net cannot be freed.
>
> It turns out that write_gssp() called gssp_rpc_create() to create a rpc
> client, this increases net->count by 2; rpcsec_gss_exit_net() is supposed
> to decrease net->count but it never gets called because its call-path is:
> net->count==0 -> cleanup_net -> ops_exit_list -> rpcsec_gss_exit_net
> Before rpcsec_gss_exit_net() gets called, net->count cannot reach 0, this
> is a deadlock situation.
>
> To fix the problem, we must break the deadlock, rpcsec_gss_exit_net()
> should move out of the put() path and find another chance to get called,
> I think nsfs_evict() is a good place to go, when netns inode gets evicted
> we call rpcsec_gss_exit_net() to free the rpc client, this requires a new
> callback i.e. evict to be added in struct proc_ns_operations, and add
> netns_evict() as one of netns_operations as well.
>
> v1->v2:
> * in nsfs_evict(), move ->evict() in front of ->put()
> v2->v3:
> * rpcsec_gss_evict_net() directly call gss_svc_shutdown_net() regardless
> if gssp_clnt is null, this is exactly same to what rpcsec_gss_exit_net()
> previously did
>
> Wenbin Zeng (3):
> nsfs: add evict callback into struct proc_ns_operations
> netns: add netns_evict into netns_operations
> auth_gss: fix deadlock that blocks rpcsec_gss_exit_net when
> use-gss-proxy==1
>
> fs/nsfs.c | 2 ++
> include/linux/proc_ns.h | 1 +
> include/net/net_namespace.h | 1 +
> net/core/net_namespace.c | 12 ++++++++++++
> net/sunrpc/auth_gss/auth_gss.c | 4 ++--
> 5 files changed, 18 insertions(+), 2 deletions(-)
>
> --
> 1.8.3.1