| Message ID | 1556692945-3996-2-git-send-email-wenbinzeng@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | auth_gss: netns refcount leaks when use-gss-proxy==1 | expand |
On Wed, May 01, 2019 at 02:42:23PM +0800, Wenbin Zeng wrote: > 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 <wenbinzeng@tencent.com> > --- > 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..5939b12 100644 > --- a/fs/nsfs.c > +++ b/fs/nsfs.c > @@ -49,6 +49,8 @@ static void nsfs_evict(struct inode *inode) > struct ns_common *ns = inode->i_private; > clear_inode(inode); > ns->ops->put(ns); > + if (ns->ops->evict) > + ns->ops->evict(ns); What's to guarantee that ns will not be freed by ->put()? Confused...
On Thu, May 02, 2019 at 04:04:06AM +0100, Al Viro wrote: > On Wed, May 01, 2019 at 02:42:23PM +0800, Wenbin Zeng wrote: > > 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 <wenbinzeng@tencent.com> > > --- > > 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..5939b12 100644 > > --- a/fs/nsfs.c > > +++ b/fs/nsfs.c > > @@ -49,6 +49,8 @@ static void nsfs_evict(struct inode *inode) > > struct ns_common *ns = inode->i_private; > > clear_inode(inode); > > ns->ops->put(ns); > > + if (ns->ops->evict) > > + ns->ops->evict(ns); > > What's to guarantee that ns will not be freed by ->put()? > Confused... Hi Al, thank you very much. You are absolutely right. ->evict() should be called before ->put(), i.e.: @@ -49,6 +49,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); } Does this look good?
diff --git a/fs/nsfs.c b/fs/nsfs.c index 60702d6..5939b12 100644 --- a/fs/nsfs.c +++ b/fs/nsfs.c @@ -49,6 +49,8 @@ static void nsfs_evict(struct inode *inode) struct ns_common *ns = inode->i_private; clear_inode(inode); ns->ops->put(ns); + if (ns->ops->evict) + ns->ops->evict(ns); } static void *__ns_get_path(struct path *path, struct ns_common *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);
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 <wenbinzeng@tencent.com> --- fs/nsfs.c | 2 ++ include/linux/proc_ns.h | 1 + 2 files changed, 3 insertions(+)