Re: [PATCH] nsproxy: remove judge of timens_on_fork()'s return val

[Hui Su]

On Tue, Nov 17, 2020 at 12:30:26AM -0800, Andrey Vagin wrote:
> On Sun, Nov 15, 2020 at 10:36 AM Hui Su  wrote:
> >
> > timens_on_fork() always return 0, and maybe not
> > need to judge the return value in copy_namespaces().
> 
> Thank you for cleaning this up. I think we can go even further and
> change timens_on_fork to return nothing:
> 
> -int timens_on_fork(struct nsproxy *nsproxy, struct task_struct *tsk)
> +void timens_on_fork(struct nsproxy *nsproxy, struct task_struct *tsk)
>  {
> struct ns_common *nsc = >time_ns_for_children->ns;
> struct time_namespace *ns = to_time_ns(nsc);
> 
> /* create_new_namespaces() already incremented the ref counter */
> if (nsproxy->time_ns == nsproxy->time_ns_for_children)
> -   return 0;
> +   return;
> 
> get_time_ns(ns);
> put_time_ns(nsproxy->time_ns);
> nsproxy->time_ns = ns;
> 
> timens_commit(tsk, ns);
> -
> -   return 0;
>  }
> 
> 
> >
> > Signed-off-by: Hui Su 
> > ---
> >  kernel/nsproxy.c | 7 +--
> >  1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
> > index 12dd41b39a7f..e2e6c5dc433f 100644
> > --- a/kernel/nsproxy.c
> > +++ b/kernel/nsproxy.c
> > @@ -153,7 +153,6 @@ int copy_namespaces(unsigned long flags, struct 
> > task_struct *tsk)
> > struct nsproxy *old_ns = tsk->nsproxy;
> > struct user_namespace *user_ns = task_cred_xxx(tsk, user_ns);
> > struct nsproxy *new_ns;
> > -   int ret;
> >
> > if (likely(!(flags & (CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC |
> >   CLONE_NEWPID | CLONE_NEWNET |
> > @@ -180,11 +179,7 @@ int copy_namespaces(unsigned long flags, struct 
> > task_struct *tsk)
> > if (IS_ERR(new_ns))
> > return  PTR_ERR(new_ns);
> >
> > -   ret = timens_on_fork(new_ns, tsk);
> > -   if (ret) {
> > -   free_nsproxy(new_ns);
> > -   return ret;
> > -   }
> > +   timens_on_fork(new_ns, tsk);
> >
> > tsk->nsproxy = new_ns;
> > return 0;
> > --
> > 2.29.0
> >
> >

Yeah, thank for your reply, Andrey.

I will resend PATCH V2 with makeing timens_on_fork() return nothing.

Thanks.

Re: [PATCH] nsproxy: remove judge of timens_on_fork()'s return val

[Andrey Vagin]

On Sun, Nov 15, 2020 at 10:36 AM Hui Su  wrote:
>
> timens_on_fork() always return 0, and maybe not
> need to judge the return value in copy_namespaces().

Thank you for cleaning this up. I think we can go even further and
change timens_on_fork to return nothing:

-int timens_on_fork(struct nsproxy *nsproxy, struct task_struct *tsk)
+void timens_on_fork(struct nsproxy *nsproxy, struct task_struct *tsk)
 {
struct ns_common *nsc = >time_ns_for_children->ns;
struct time_namespace *ns = to_time_ns(nsc);

/* create_new_namespaces() already incremented the ref counter */
if (nsproxy->time_ns == nsproxy->time_ns_for_children)
-   return 0;
+   return;

get_time_ns(ns);
put_time_ns(nsproxy->time_ns);
nsproxy->time_ns = ns;

timens_commit(tsk, ns);
-
-   return 0;
 }


>
> Signed-off-by: Hui Su 
> ---
>  kernel/nsproxy.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
> index 12dd41b39a7f..e2e6c5dc433f 100644
> --- a/kernel/nsproxy.c
> +++ b/kernel/nsproxy.c
> @@ -153,7 +153,6 @@ int copy_namespaces(unsigned long flags, struct 
> task_struct *tsk)
> struct nsproxy *old_ns = tsk->nsproxy;
> struct user_namespace *user_ns = task_cred_xxx(tsk, user_ns);
> struct nsproxy *new_ns;
> -   int ret;
>
> if (likely(!(flags & (CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC |
>   CLONE_NEWPID | CLONE_NEWNET |
> @@ -180,11 +179,7 @@ int copy_namespaces(unsigned long flags, struct 
> task_struct *tsk)
> if (IS_ERR(new_ns))
> return  PTR_ERR(new_ns);
>
> -   ret = timens_on_fork(new_ns, tsk);
> -   if (ret) {
> -   free_nsproxy(new_ns);
> -   return ret;
> -   }
> +   timens_on_fork(new_ns, tsk);
>
> tsk->nsproxy = new_ns;
> return 0;
> --
> 2.29.0
>
>

[PATCH] nsproxy: remove judge of timens_on_fork()'s return val

[Hui Su]

timens_on_fork() always return 0, and maybe not
need to judge the return value in copy_namespaces().

Signed-off-by: Hui Su 
---
 kernel/nsproxy.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index 12dd41b39a7f..e2e6c5dc433f 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -153,7 +153,6 @@ int copy_namespaces(unsigned long flags, struct task_struct 
*tsk)
struct nsproxy *old_ns = tsk->nsproxy;
struct user_namespace *user_ns = task_cred_xxx(tsk, user_ns);
struct nsproxy *new_ns;
-   int ret;
 
if (likely(!(flags & (CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC |
  CLONE_NEWPID | CLONE_NEWNET |
@@ -180,11 +179,7 @@ int copy_namespaces(unsigned long flags, struct 
task_struct *tsk)
if (IS_ERR(new_ns))
return  PTR_ERR(new_ns);
 
-   ret = timens_on_fork(new_ns, tsk);
-   if (ret) {
-   free_nsproxy(new_ns);
-   return ret;
-   }
+   timens_on_fork(new_ns, tsk);
 
tsk->nsproxy = new_ns;
return 0;
-- 
2.29.0