Messages in this thread Patch in this message | | | Date | Wed, 18 Nov 2020 14:13:41 +0100 | From | Christian Brauner <> | Subject | Re: [PATCH] nsproxy: use put_nsproxy() in switch_task_namespaces() |
| |
On Mon, Nov 16, 2020 at 02:00:54AM +0800, Hui Su wrote: > Use put_nsproxy() instead of ' > if (atomic_dec_and_test(&ns->count)) { > free_nsproxy(ns); > }' in switch_task_namespaces(). > > and remove the whitespace by the way. > > Signed-off-by: Hui Su <sh_def@163.com> > ---
Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Thanks! I'm picking this up now.
I'll add to the commit message though sm like:
"We already have a dedicated helper that handles reference count checking so stop open-coding the reference count check in switch_task_namespaces() and use the dedicated put_nsproxy() helper instead."
(Looking at put_nsproxy() I feel we should probably start mandating that all free()/put() helpers should gracefully handle when passed NULL to avoid: if (ns) put() calls. But that's independent of this fix.)
> kernel/nsproxy.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c > index 12dd41b39a7f..3ebfd090398a 100644 > --- a/kernel/nsproxy.c > +++ b/kernel/nsproxy.c > @@ -173,7 +173,7 @@ int copy_namespaces(unsigned long flags, struct task_struct *tsk) > * it along with CLONE_NEWIPC. > */ > if ((flags & (CLONE_NEWIPC | CLONE_SYSVSEM)) == > - (CLONE_NEWIPC | CLONE_SYSVSEM)) > + (CLONE_NEWIPC | CLONE_SYSVSEM)) > return -EINVAL; > > new_ns = create_new_namespaces(flags, tsk, user_ns, tsk->fs); > @@ -250,8 +250,8 @@ void switch_task_namespaces(struct task_struct *p, struct nsproxy *new) > p->nsproxy = new; > task_unlock(p); > > - if (ns && atomic_dec_and_test(&ns->count)) > - free_nsproxy(ns); > + if (ns) > + put_nsproxy(ns); > } > > void exit_task_namespaces(struct task_struct *p) > -- > 2.29.0 > >
| |