All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf tools: Restore proper cwd on return from mnt ns
@ 2018-11-01 17:00 Jiri Olsa
  2018-11-16 13:03 ` Jiri Olsa
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Jiri Olsa @ 2018-11-01 17:00 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Krister Johansen, lkml, Ingo Molnar, Namhyung Kim,
	Alexander Shishkin, Peter Zijlstra

When reporting on 'record' server we try to retrieve/use
the mnt namespace of the profiled tasks. We use following
API with cookie to hold the return namespace, roughly:

  nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc)
    setns(newns, 0);
  ...
  new ns related open..
  ...
  nsinfo__mountns_exit(struct nscookie *nc)
    setns(nc->oldns)

Once finished we setns to old namespace, which also sets the
current working directory (cwd) to "/", trashing the cwd we had.

This is mostly fine, because we use absolute paths almost everywhere,
but it screws up perf diff:

  # perf diff
  failed to open perf.data: No such file or directory  (try 'perf record' first)
  ...

Adding the current working directory to be part of the cookie
and restoring it in the nsinfo__mountns_exit call.

Cc: Krister Johansen <kjlx@templeofstupid.com>
Fixes: 843ff37bb59e ("perf symbols: Find symbols in different mount namespace")
Link: http://lkml.kernel.org/n/tip-zg3vz7kjr86cco7lo91v8yhj@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/namespaces.c | 18 ++++++++++++++++--
 tools/perf/util/namespaces.h |  1 +
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
index cf8bd123cf73..fb0458b7e6aa 100644
--- a/tools/perf/util/namespaces.c
+++ b/tools/perf/util/namespaces.c
@@ -18,6 +18,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <asm/bug.h>
 
 struct namespaces *namespaces__new(struct namespaces_event *event)
 {
@@ -186,6 +187,7 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
 	char curpath[PATH_MAX];
 	int oldns = -1;
 	int newns = -1;
+	char *oldcwd = NULL;
 
 	if (nc == NULL)
 		return;
@@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
 	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
 		return;
 
+	oldcwd = get_current_dir_name();
+	if (!oldcwd)
+		return;
+
 	oldns = open(curpath, O_RDONLY);
 	if (oldns < 0)
-		return;
+		goto errout;
 
 	newns = open(nsi->mntns_path, O_RDONLY);
 	if (newns < 0)
@@ -210,11 +216,14 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
 	if (setns(newns, CLONE_NEWNS) < 0)
 		goto errout;
 
+	nc->oldcwd = oldcwd;
 	nc->oldns = oldns;
 	nc->newns = newns;
 	return;
 
 errout:
+	if (oldcwd)
+		free(oldcwd);
 	if (oldns > -1)
 		close(oldns);
 	if (newns > -1)
@@ -223,11 +232,16 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
 
 void nsinfo__mountns_exit(struct nscookie *nc)
 {
-	if (nc == NULL || nc->oldns == -1 || nc->newns == -1)
+	if (nc == NULL || nc->oldns == -1 || nc->newns == -1 || !nc->oldcwd)
 		return;
 
 	setns(nc->oldns, CLONE_NEWNS);
 
+	if (nc->oldcwd) {
+		WARN_ON_ONCE(chdir(nc->oldcwd));
+		free(nc->oldcwd);
+	}
+
 	if (nc->oldns > -1) {
 		close(nc->oldns);
 		nc->oldns = -1;
diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h
index cae1a9a39722..d5f46c09ea31 100644
--- a/tools/perf/util/namespaces.h
+++ b/tools/perf/util/namespaces.h
@@ -38,6 +38,7 @@ struct nsinfo {
 struct nscookie {
 	int			oldns;
 	int			newns;
+	char			*oldcwd;
 };
 
 int nsinfo__init(struct nsinfo *nsi);
-- 
2.17.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] perf tools: Restore proper cwd on return from mnt ns
  2018-11-01 17:00 [PATCH] perf tools: Restore proper cwd on return from mnt ns Jiri Olsa
@ 2018-11-16 13:03 ` Jiri Olsa
  2018-11-16 18:31 ` Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Jiri Olsa @ 2018-11-16 13:03 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Krister Johansen, lkml, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra

ping

jirka

On Thu, Nov 01, 2018 at 06:00:01PM +0100, Jiri Olsa wrote:
> When reporting on 'record' server we try to retrieve/use
> the mnt namespace of the profiled tasks. We use following
> API with cookie to hold the return namespace, roughly:
> 
>   nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc)
>     setns(newns, 0);
>   ...
>   new ns related open..
>   ...
>   nsinfo__mountns_exit(struct nscookie *nc)
>     setns(nc->oldns)
> 
> Once finished we setns to old namespace, which also sets the
> current working directory (cwd) to "/", trashing the cwd we had.
> 
> This is mostly fine, because we use absolute paths almost everywhere,
> but it screws up perf diff:
> 
>   # perf diff
>   failed to open perf.data: No such file or directory  (try 'perf record' first)
>   ...
> 
> Adding the current working directory to be part of the cookie
> and restoring it in the nsinfo__mountns_exit call.
> 
> Cc: Krister Johansen <kjlx@templeofstupid.com>
> Fixes: 843ff37bb59e ("perf symbols: Find symbols in different mount namespace")
> Link: http://lkml.kernel.org/n/tip-zg3vz7kjr86cco7lo91v8yhj@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/namespaces.c | 18 ++++++++++++++++--
>  tools/perf/util/namespaces.h |  1 +
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> index cf8bd123cf73..fb0458b7e6aa 100644
> --- a/tools/perf/util/namespaces.c
> +++ b/tools/perf/util/namespaces.c
> @@ -18,6 +18,7 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include <asm/bug.h>
>  
>  struct namespaces *namespaces__new(struct namespaces_event *event)
>  {
> @@ -186,6 +187,7 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  	char curpath[PATH_MAX];
>  	int oldns = -1;
>  	int newns = -1;
> +	char *oldcwd = NULL;
>  
>  	if (nc == NULL)
>  		return;
> @@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
>  		return;
>  
> +	oldcwd = get_current_dir_name();
> +	if (!oldcwd)
> +		return;
> +
>  	oldns = open(curpath, O_RDONLY);
>  	if (oldns < 0)
> -		return;
> +		goto errout;
>  
>  	newns = open(nsi->mntns_path, O_RDONLY);
>  	if (newns < 0)
> @@ -210,11 +216,14 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  	if (setns(newns, CLONE_NEWNS) < 0)
>  		goto errout;
>  
> +	nc->oldcwd = oldcwd;
>  	nc->oldns = oldns;
>  	nc->newns = newns;
>  	return;
>  
>  errout:
> +	if (oldcwd)
> +		free(oldcwd);
>  	if (oldns > -1)
>  		close(oldns);
>  	if (newns > -1)
> @@ -223,11 +232,16 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  
>  void nsinfo__mountns_exit(struct nscookie *nc)
>  {
> -	if (nc == NULL || nc->oldns == -1 || nc->newns == -1)
> +	if (nc == NULL || nc->oldns == -1 || nc->newns == -1 || !nc->oldcwd)
>  		return;
>  
>  	setns(nc->oldns, CLONE_NEWNS);
>  
> +	if (nc->oldcwd) {
> +		WARN_ON_ONCE(chdir(nc->oldcwd));
> +		free(nc->oldcwd);
> +	}
> +
>  	if (nc->oldns > -1) {
>  		close(nc->oldns);
>  		nc->oldns = -1;
> diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h
> index cae1a9a39722..d5f46c09ea31 100644
> --- a/tools/perf/util/namespaces.h
> +++ b/tools/perf/util/namespaces.h
> @@ -38,6 +38,7 @@ struct nsinfo {
>  struct nscookie {
>  	int			oldns;
>  	int			newns;
> +	char			*oldcwd;
>  };
>  
>  int nsinfo__init(struct nsinfo *nsi);
> -- 
> 2.17.2
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] perf tools: Restore proper cwd on return from mnt ns
  2018-11-01 17:00 [PATCH] perf tools: Restore proper cwd on return from mnt ns Jiri Olsa
  2018-11-16 13:03 ` Jiri Olsa
@ 2018-11-16 18:31 ` Arnaldo Carvalho de Melo
  2018-11-16 18:42   ` Arnaldo Carvalho de Melo
  2018-11-19 16:10 ` Arnaldo Carvalho de Melo
  2018-11-21 15:02 ` [tip:perf/urgent] perf tools: Restore proper cwd on return from mnt namespace tip-bot for Jiri Olsa
  3 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-11-16 18:31 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Krister Johansen, lkml, Ingo Molnar, Namhyung Kim,
	Alexander Shishkin, Peter Zijlstra

Em Thu, Nov 01, 2018 at 06:00:01PM +0100, Jiri Olsa escreveu:
> When reporting on 'record' server we try to retrieve/use
> the mnt namespace of the profiled tasks. We use following
> API with cookie to hold the return namespace, roughly:
> 
>   nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc)
>     setns(newns, 0);
>   ...
>   new ns related open..
>   ...
>   nsinfo__mountns_exit(struct nscookie *nc)
>     setns(nc->oldns)
> 
> Once finished we setns to old namespace, which also sets the
> current working directory (cwd) to "/", trashing the cwd we had.
> 
> This is mostly fine, because we use absolute paths almost everywhere,
> but it screws up perf diff:
> 
>   # perf diff
>   failed to open perf.data: No such file or directory  (try 'perf record' first)
>   ...
> 
> Adding the current working directory to be part of the cookie
> and restoring it in the nsinfo__mountns_exit call.
 
> Cc: Krister Johansen <kjlx@templeofstupid.com>
> Fixes: 843ff37bb59e ("perf symbols: Find symbols in different mount namespace")
> Link: http://lkml.kernel.org/n/tip-zg3vz7kjr86cco7lo91v8yhj@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/namespaces.c | 18 ++++++++++++++++--
>  tools/perf/util/namespaces.h |  1 +
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> index cf8bd123cf73..fb0458b7e6aa 100644
> --- a/tools/perf/util/namespaces.c
> +++ b/tools/perf/util/namespaces.c
> @@ -18,6 +18,7 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include <asm/bug.h>
>  
>  struct namespaces *namespaces__new(struct namespaces_event *event)
>  {
> @@ -186,6 +187,7 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  	char curpath[PATH_MAX];
>  	int oldns = -1;
>  	int newns = -1;
> +	char *oldcwd = NULL;
>  
>  	if (nc == NULL)
>  		return;
> @@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
>  		return;
>  
> +	oldcwd = get_current_dir_name();
> +	if (!oldcwd)
> +		return;
> +
>  	oldns = open(curpath, O_RDONLY);
>  	if (oldns < 0)
> -		return;
> +		goto errout;
>  
>  	newns = open(nsi->mntns_path, O_RDONLY);
>  	if (newns < 0)
> @@ -210,11 +216,14 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  	if (setns(newns, CLONE_NEWNS) < 0)
>  		goto errout;
>  
> +	nc->oldcwd = oldcwd;
>  	nc->oldns = oldns;
>  	nc->newns = newns;
>  	return;
>  
>  errout:
> +	if (oldcwd)
> +		free(oldcwd);

Applied, and while at it I removed the needless if in the above two
lines.

>  	if (oldns > -1)
>  		close(oldns);
>  	if (newns > -1)
> @@ -223,11 +232,16 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  
>  void nsinfo__mountns_exit(struct nscookie *nc)
>  {
> -	if (nc == NULL || nc->oldns == -1 || nc->newns == -1)
> +	if (nc == NULL || nc->oldns == -1 || nc->newns == -1 || !nc->oldcwd)
>  		return;
>  
>  	setns(nc->oldns, CLONE_NEWNS);
>  
> +	if (nc->oldcwd) {
> +		WARN_ON_ONCE(chdir(nc->oldcwd));
> +		free(nc->oldcwd);
> +	}
> +
>  	if (nc->oldns > -1) {
>  		close(nc->oldns);
>  		nc->oldns = -1;
> diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h
> index cae1a9a39722..d5f46c09ea31 100644
> --- a/tools/perf/util/namespaces.h
> +++ b/tools/perf/util/namespaces.h
> @@ -38,6 +38,7 @@ struct nsinfo {
>  struct nscookie {
>  	int			oldns;
>  	int			newns;
> +	char			*oldcwd;
>  };
>  
>  int nsinfo__init(struct nsinfo *nsi);
> -- 
> 2.17.2

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] perf tools: Restore proper cwd on return from mnt ns
  2018-11-16 18:31 ` Arnaldo Carvalho de Melo
@ 2018-11-16 18:42   ` Arnaldo Carvalho de Melo
  2018-11-17  9:00     ` Jiri Olsa
  0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-11-16 18:42 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Krister Johansen, lkml, Ingo Molnar, Namhyung Kim,
	Alexander Shishkin, Peter Zijlstra

Em Fri, Nov 16, 2018 at 10:31:43AM -0800, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Nov 01, 2018 at 06:00:01PM +0100, Jiri Olsa escreveu:
> > When reporting on 'record' server we try to retrieve/use
> > the mnt namespace of the profiled tasks. We use following
> > API with cookie to hold the return namespace, roughly:
> > 
> >   nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc)
> >     setns(newns, 0);
> >   ...
> >   new ns related open..
> >   ...
> >   nsinfo__mountns_exit(struct nscookie *nc)
> >     setns(nc->oldns)
> > 
> > Once finished we setns to old namespace, which also sets the
> > current working directory (cwd) to "/", trashing the cwd we had.
> > 
> > This is mostly fine, because we use absolute paths almost everywhere,
> > but it screws up perf diff:
> > 
> >   # perf diff
> >   failed to open perf.data: No such file or directory  (try 'perf record' first)
> >   ...
> > 
> > Adding the current working directory to be part of the cookie
> > and restoring it in the nsinfo__mountns_exit call.
>  
> > Cc: Krister Johansen <kjlx@templeofstupid.com>
> > Fixes: 843ff37bb59e ("perf symbols: Find symbols in different mount namespace")
> > Link: http://lkml.kernel.org/n/tip-zg3vz7kjr86cco7lo91v8yhj@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/util/namespaces.c | 18 ++++++++++++++++--
> >  tools/perf/util/namespaces.h |  1 +
> >  2 files changed, 17 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> > index cf8bd123cf73..fb0458b7e6aa 100644
> > --- a/tools/perf/util/namespaces.c
> > +++ b/tools/perf/util/namespaces.c
> > @@ -18,6 +18,7 @@
> >  #include <stdio.h>
> >  #include <string.h>
> >  #include <unistd.h>
> > +#include <asm/bug.h>
> >  
> >  struct namespaces *namespaces__new(struct namespaces_event *event)
> >  {
> > @@ -186,6 +187,7 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> >  	char curpath[PATH_MAX];
> >  	int oldns = -1;
> >  	int newns = -1;
> > +	char *oldcwd = NULL;
> >  
> >  	if (nc == NULL)
> >  		return;
> > @@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> >  	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
> >  		return;
> >  
> > +	oldcwd = get_current_dir_name();
> > +	if (!oldcwd)
> > +		return;
> > +
> >  	oldns = open(curpath, O_RDONLY);
> >  	if (oldns < 0)
> > -		return;
> > +		goto errout;
> >  
> >  	newns = open(nsi->mntns_path, O_RDONLY);
> >  	if (newns < 0)
> > @@ -210,11 +216,14 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> >  	if (setns(newns, CLONE_NEWNS) < 0)
> >  		goto errout;
> >  
> > +	nc->oldcwd = oldcwd;
> >  	nc->oldns = oldns;
> >  	nc->newns = newns;
> >  	return;
> >  
> >  errout:
> > +	if (oldcwd)
> > +		free(oldcwd);
> 
> Applied, and while at it I removed the needless if in the above two
> lines.
> 
> >  	if (oldns > -1)
> >  		close(oldns);
> >  	if (newns > -1)
> > @@ -223,11 +232,16 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> >  
> >  void nsinfo__mountns_exit(struct nscookie *nc)
> >  {
> > -	if (nc == NULL || nc->oldns == -1 || nc->newns == -1)
> > +	if (nc == NULL || nc->oldns == -1 || nc->newns == -1 || !nc->oldcwd)
> >  		return;
> >  
> >  	setns(nc->oldns, CLONE_NEWNS);
> >  
> > +	if (nc->oldcwd) {
> > +		WARN_ON_ONCE(chdir(nc->oldcwd));
> > +		free(nc->oldcwd);
> > +	}
> > +

Also changed the above to zfree(&nc->oldcwd) as this is in a struct
member, that is not freed by the freeing caller
(nsinfo__mountns_exit()), to avoid later possible use-after-free (I
haven't checked if it _actually_ happens with the current codebase, but
it could) also to match the following block, where the resource is
released and then the handler is invalidated.

- Arnaldo

> >  	if (nc->oldns > -1) {
> >  		close(nc->oldns);
> >  		nc->oldns = -1;

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] perf tools: Restore proper cwd on return from mnt ns
  2018-11-16 18:42   ` Arnaldo Carvalho de Melo
@ 2018-11-17  9:00     ` Jiri Olsa
  0 siblings, 0 replies; 11+ messages in thread
From: Jiri Olsa @ 2018-11-17  9:00 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Krister Johansen, lkml, Ingo Molnar, Namhyung Kim,
	Alexander Shishkin, Peter Zijlstra

On Fri, Nov 16, 2018 at 10:42:45AM -0800, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 16, 2018 at 10:31:43AM -0800, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Nov 01, 2018 at 06:00:01PM +0100, Jiri Olsa escreveu:
> > > When reporting on 'record' server we try to retrieve/use
> > > the mnt namespace of the profiled tasks. We use following
> > > API with cookie to hold the return namespace, roughly:
> > > 
> > >   nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc)
> > >     setns(newns, 0);
> > >   ...
> > >   new ns related open..
> > >   ...
> > >   nsinfo__mountns_exit(struct nscookie *nc)
> > >     setns(nc->oldns)
> > > 
> > > Once finished we setns to old namespace, which also sets the
> > > current working directory (cwd) to "/", trashing the cwd we had.
> > > 
> > > This is mostly fine, because we use absolute paths almost everywhere,
> > > but it screws up perf diff:
> > > 
> > >   # perf diff
> > >   failed to open perf.data: No such file or directory  (try 'perf record' first)
> > >   ...
> > > 
> > > Adding the current working directory to be part of the cookie
> > > and restoring it in the nsinfo__mountns_exit call.
> >  
> > > Cc: Krister Johansen <kjlx@templeofstupid.com>
> > > Fixes: 843ff37bb59e ("perf symbols: Find symbols in different mount namespace")
> > > Link: http://lkml.kernel.org/n/tip-zg3vz7kjr86cco7lo91v8yhj@git.kernel.org
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > >  tools/perf/util/namespaces.c | 18 ++++++++++++++++--
> > >  tools/perf/util/namespaces.h |  1 +
> > >  2 files changed, 17 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> > > index cf8bd123cf73..fb0458b7e6aa 100644
> > > --- a/tools/perf/util/namespaces.c
> > > +++ b/tools/perf/util/namespaces.c
> > > @@ -18,6 +18,7 @@
> > >  #include <stdio.h>
> > >  #include <string.h>
> > >  #include <unistd.h>
> > > +#include <asm/bug.h>
> > >  
> > >  struct namespaces *namespaces__new(struct namespaces_event *event)
> > >  {
> > > @@ -186,6 +187,7 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> > >  	char curpath[PATH_MAX];
> > >  	int oldns = -1;
> > >  	int newns = -1;
> > > +	char *oldcwd = NULL;
> > >  
> > >  	if (nc == NULL)
> > >  		return;
> > > @@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> > >  	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
> > >  		return;
> > >  
> > > +	oldcwd = get_current_dir_name();
> > > +	if (!oldcwd)
> > > +		return;
> > > +
> > >  	oldns = open(curpath, O_RDONLY);
> > >  	if (oldns < 0)
> > > -		return;
> > > +		goto errout;
> > >  
> > >  	newns = open(nsi->mntns_path, O_RDONLY);
> > >  	if (newns < 0)
> > > @@ -210,11 +216,14 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> > >  	if (setns(newns, CLONE_NEWNS) < 0)
> > >  		goto errout;
> > >  
> > > +	nc->oldcwd = oldcwd;
> > >  	nc->oldns = oldns;
> > >  	nc->newns = newns;
> > >  	return;
> > >  
> > >  errout:
> > > +	if (oldcwd)
> > > +		free(oldcwd);

right, ok

> > 
> > Applied, and while at it I removed the needless if in the above two
> > lines.
> > 
> > >  	if (oldns > -1)
> > >  		close(oldns);
> > >  	if (newns > -1)
> > > @@ -223,11 +232,16 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> > >  
> > >  void nsinfo__mountns_exit(struct nscookie *nc)
> > >  {
> > > -	if (nc == NULL || nc->oldns == -1 || nc->newns == -1)
> > > +	if (nc == NULL || nc->oldns == -1 || nc->newns == -1 || !nc->oldcwd)
> > >  		return;
> > >  
> > >  	setns(nc->oldns, CLONE_NEWNS);
> > >  
> > > +	if (nc->oldcwd) {
> > > +		WARN_ON_ONCE(chdir(nc->oldcwd));
> > > +		free(nc->oldcwd);
> > > +	}
> > > +
> 
> Also changed the above to zfree(&nc->oldcwd) as this is in a struct
> member, that is not freed by the freeing caller
> (nsinfo__mountns_exit()), to avoid later possible use-after-free (I
> haven't checked if it _actually_ happens with the current codebase, but
> it could) also to match the following block, where the resource is
> released and then the handler is invalidated.

great, thanks

jirka

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] perf tools: Restore proper cwd on return from mnt ns
  2018-11-01 17:00 [PATCH] perf tools: Restore proper cwd on return from mnt ns Jiri Olsa
  2018-11-16 13:03 ` Jiri Olsa
  2018-11-16 18:31 ` Arnaldo Carvalho de Melo
@ 2018-11-19 16:10 ` Arnaldo Carvalho de Melo
  2018-11-20 10:56   ` Jiri Olsa
  2018-11-21 15:02 ` [tip:perf/urgent] perf tools: Restore proper cwd on return from mnt namespace tip-bot for Jiri Olsa
  3 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-11-19 16:10 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Krister Johansen, lkml, Ingo Molnar, Namhyung Kim,
	Alexander Shishkin, Peter Zijlstra

Em Thu, Nov 01, 2018 at 06:00:01PM +0100, Jiri Olsa escreveu:
> Adding the current working directory to be part of the cookie
> and restoring it in the nsinfo__mountns_exit call.
 
> diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> index cf8bd123cf73..fb0458b7e6aa 100644
> --- a/tools/perf/util/namespaces.c
> +++ b/tools/perf/util/namespaces.c

> @@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
>  	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
>  		return;
  
> +	oldcwd = get_current_dir_name();

Breaks the build with bionic (Android), where get_current_dir_name() is
not available:

/tmp/build/perf/libperf.a(libperf-in.o):hist.c:function nsinfo__mountns_enter: error: undefined reference to 'get_current_dir_name'
collect2: error: ld returned 1 exit status
Makefile.perf:534: recipe for target '/tmp/build/perf/perf' failed
make[2]: *** [/tmp/build/perf/perf] Error 1
Makefile.perf:206: recipe for target 'sub-make' failed
make[1]: *** [sub-make] Error 2
Makefile:69: recipe for target 'all' failed
make: *** [all] Error 2
make: Leaving directory '/git/linux/tools/perf'
[root@seventh ~]# 

The container builds are ongoing, its possoble that this becomes an
issue in uCLibc, wasn't on musl libc (Alpine Linux).

- Arnaldo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] perf tools: Restore proper cwd on return from mnt ns
  2018-11-19 16:10 ` Arnaldo Carvalho de Melo
@ 2018-11-20 10:56   ` Jiri Olsa
  2018-11-20 11:23     ` [PATCHv2] " Jiri Olsa
  2018-11-20 15:56     ` [PATCH] " Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 11+ messages in thread
From: Jiri Olsa @ 2018-11-20 10:56 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Krister Johansen, lkml, Ingo Molnar, Namhyung Kim,
	Alexander Shishkin, Peter Zijlstra

On Mon, Nov 19, 2018 at 08:10:06AM -0800, Arnaldo Carvalho de Melo wrote:
> Em Thu, Nov 01, 2018 at 06:00:01PM +0100, Jiri Olsa escreveu:
> > Adding the current working directory to be part of the cookie
> > and restoring it in the nsinfo__mountns_exit call.
>  
> > diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> > index cf8bd123cf73..fb0458b7e6aa 100644
> > --- a/tools/perf/util/namespaces.c
> > +++ b/tools/perf/util/namespaces.c
> 
> > @@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> >  	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
> >  		return;
>   
> > +	oldcwd = get_current_dir_name();
> 
> Breaks the build with bionic (Android), where get_current_dir_name() is
> not available:
> 
> /tmp/build/perf/libperf.a(libperf-in.o):hist.c:function nsinfo__mountns_enter: error: undefined reference to 'get_current_dir_name'
> collect2: error: ld returned 1 exit status
> Makefile.perf:534: recipe for target '/tmp/build/perf/perf' failed
> make[2]: *** [/tmp/build/perf/perf] Error 1
> Makefile.perf:206: recipe for target 'sub-make' failed
> make[1]: *** [sub-make] Error 2
> Makefile:69: recipe for target 'all' failed
> make: *** [all] Error 2
> make: Leaving directory '/git/linux/tools/perf'
> [root@seventh ~]# 
> 
> The container builds are ongoing, its possoble that this becomes an
> issue in uCLibc, wasn't on musl libc (Alpine Linux).

will check, thanks

jirka

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCHv2] perf tools: Restore proper cwd on return from mnt ns
  2018-11-20 10:56   ` Jiri Olsa
@ 2018-11-20 11:23     ` Jiri Olsa
  2018-11-20 15:56     ` [PATCH] " Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 11+ messages in thread
From: Jiri Olsa @ 2018-11-20 11:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Krister Johansen, lkml, Ingo Molnar, Namhyung Kim,
	Alexander Shishkin, Peter Zijlstra

On Tue, Nov 20, 2018 at 11:56:18AM +0100, Jiri Olsa wrote:
> On Mon, Nov 19, 2018 at 08:10:06AM -0800, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Nov 01, 2018 at 06:00:01PM +0100, Jiri Olsa escreveu:
> > > Adding the current working directory to be part of the cookie
> > > and restoring it in the nsinfo__mountns_exit call.
> >  
> > > diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> > > index cf8bd123cf73..fb0458b7e6aa 100644
> > > --- a/tools/perf/util/namespaces.c
> > > +++ b/tools/perf/util/namespaces.c
> > 
> > > @@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> > >  	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
> > >  		return;
> >   
> > > +	oldcwd = get_current_dir_name();
> > 
> > Breaks the build with bionic (Android), where get_current_dir_name() is
> > not available:
> > 
> > /tmp/build/perf/libperf.a(libperf-in.o):hist.c:function nsinfo__mountns_enter: error: undefined reference to 'get_current_dir_name'
> > collect2: error: ld returned 1 exit status
> > Makefile.perf:534: recipe for target '/tmp/build/perf/perf' failed
> > make[2]: *** [/tmp/build/perf/perf] Error 1
> > Makefile.perf:206: recipe for target 'sub-make' failed
> > make[1]: *** [sub-make] Error 2
> > Makefile:69: recipe for target 'all' failed
> > make: *** [all] Error 2
> > make: Leaving directory '/git/linux/tools/perf'
> > [root@seventh ~]# 
> > 
> > The container builds are ongoing, its possoble that this becomes an
> > issue in uCLibc, wasn't on musl libc (Alpine Linux).
> 
> will check, thanks

how about this one?

thanks,
jirka


---
When reporting on 'record' server we try to retrieve/use
the mnt namespace of the profiled tasks. We use following
API with cookie to hold the return namespace, roughly:

  nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc)
    setns(newns, 0);
  ...
  new ns related open..
  ...
  nsinfo__mountns_exit(struct nscookie *nc)
    setns(nc->oldns)

Once finished we setns to old namespace, which also sets the
current working directory (cwd) to "/", trashing the cwd we had.

This is mostly fine, because we use absolute paths almost everywhere,
but it screws up perf diff:

  # perf diff
  failed to open perf.data: No such file or directory  (try 'perf record' first)
  ...

Adding the current working directory to be part of the cookie
and restoring it in the nsinfo__mountns_exit call.

v2: get rid of _GNU_SOURCE get_current_dir_name call [acme]

Cc: Krister Johansen <kjlx@templeofstupid.com>
Fixes: 843ff37bb59e ("perf symbols: Find symbols in different mount namespace")
Link: http://lkml.kernel.org/n/tip-zg3vz7kjr86cco7lo91v8yhj@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/namespaces.c | 25 +++++++++++++++++++++++--
 tools/perf/util/namespaces.h |  1 +
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
index cf8bd123cf73..b759a7ad87e7 100644
--- a/tools/perf/util/namespaces.c
+++ b/tools/perf/util/namespaces.c
@@ -18,6 +18,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <asm/bug.h>
 
 struct namespaces *namespaces__new(struct namespaces_event *event)
 {
@@ -180,12 +181,20 @@ void nsinfo__put(struct nsinfo *nsi)
 		nsinfo__delete(nsi);
 }
 
+static char *get_current_dir(void)
+{
+	char path[PATH_MAX];
+
+	return getcwd(path, PATH_MAX) ? strdup(path) : NULL;
+}
+
 void nsinfo__mountns_enter(struct nsinfo *nsi,
 				  struct nscookie *nc)
 {
 	char curpath[PATH_MAX];
 	int oldns = -1;
 	int newns = -1;
+	char *oldcwd = NULL;
 
 	if (nc == NULL)
 		return;
@@ -199,9 +208,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
 	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
 		return;
 
+	oldcwd = get_current_dir();
+	if (!oldcwd)
+		return;
+
 	oldns = open(curpath, O_RDONLY);
 	if (oldns < 0)
-		return;
+		goto errout;
 
 	newns = open(nsi->mntns_path, O_RDONLY);
 	if (newns < 0)
@@ -210,11 +223,14 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
 	if (setns(newns, CLONE_NEWNS) < 0)
 		goto errout;
 
+	nc->oldcwd = oldcwd;
 	nc->oldns = oldns;
 	nc->newns = newns;
 	return;
 
 errout:
+	if (oldcwd)
+		free(oldcwd);
 	if (oldns > -1)
 		close(oldns);
 	if (newns > -1)
@@ -223,11 +239,16 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
 
 void nsinfo__mountns_exit(struct nscookie *nc)
 {
-	if (nc == NULL || nc->oldns == -1 || nc->newns == -1)
+	if (nc == NULL || nc->oldns == -1 || nc->newns == -1 || !nc->oldcwd)
 		return;
 
 	setns(nc->oldns, CLONE_NEWNS);
 
+	if (nc->oldcwd) {
+		WARN_ON_ONCE(chdir(nc->oldcwd));
+		free(nc->oldcwd);
+	}
+
 	if (nc->oldns > -1) {
 		close(nc->oldns);
 		nc->oldns = -1;
diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h
index cae1a9a39722..d5f46c09ea31 100644
--- a/tools/perf/util/namespaces.h
+++ b/tools/perf/util/namespaces.h
@@ -38,6 +38,7 @@ struct nsinfo {
 struct nscookie {
 	int			oldns;
 	int			newns;
+	char			*oldcwd;
 };
 
 int nsinfo__init(struct nsinfo *nsi);
-- 
2.17.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] perf tools: Restore proper cwd on return from mnt ns
  2018-11-20 10:56   ` Jiri Olsa
  2018-11-20 11:23     ` [PATCHv2] " Jiri Olsa
@ 2018-11-20 15:56     ` Arnaldo Carvalho de Melo
  2018-11-20 17:42       ` Jiri Olsa
  1 sibling, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-11-20 15:56 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Olsa, Krister Johansen, lkml, Ingo Molnar, Namhyung Kim,
	Alexander Shishkin, Peter Zijlstra

Em Tue, Nov 20, 2018 at 11:56:18AM +0100, Jiri Olsa escreveu:
> On Mon, Nov 19, 2018 at 08:10:06AM -0800, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Nov 01, 2018 at 06:00:01PM +0100, Jiri Olsa escreveu:
> > > Adding the current working directory to be part of the cookie
> > > and restoring it in the nsinfo__mountns_exit call.
> >  
> > > diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> > > index cf8bd123cf73..fb0458b7e6aa 100644
> > > --- a/tools/perf/util/namespaces.c
> > > +++ b/tools/perf/util/namespaces.c
> > 
> > > @@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> > >  	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
> > >  		return;
> >   
> > > +	oldcwd = get_current_dir_name();
> > 
> > Breaks the build with bionic (Android), where get_current_dir_name() is
> > not available:
> > 
> > /tmp/build/perf/libperf.a(libperf-in.o):hist.c:function nsinfo__mountns_enter: error: undefined reference to 'get_current_dir_name'
> > collect2: error: ld returned 1 exit status
> > Makefile.perf:534: recipe for target '/tmp/build/perf/perf' failed
> > make[2]: *** [/tmp/build/perf/perf] Error 1
> > Makefile.perf:206: recipe for target 'sub-make' failed
> > make[1]: *** [sub-make] Error 2
> > Makefile:69: recipe for target 'all' failed
> > make: *** [all] Error 2
> > make: Leaving directory '/git/linux/tools/perf'
> > [root@seventh ~]# 
> > 
> > The container builds are ongoing, its possoble that this becomes an
> > issue in uCLibc, wasn't on musl libc (Alpine Linux).
> 
> will check, thanks

I fixed this already, by adding a test for that get_current_dir_name()
function and a simple implementation based on strdup(getcwd(bf)), take a
look at my perf/urgent branch 

- Arnaldo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] perf tools: Restore proper cwd on return from mnt ns
  2018-11-20 15:56     ` [PATCH] " Arnaldo Carvalho de Melo
@ 2018-11-20 17:42       ` Jiri Olsa
  0 siblings, 0 replies; 11+ messages in thread
From: Jiri Olsa @ 2018-11-20 17:42 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Krister Johansen, lkml, Ingo Molnar, Namhyung Kim,
	Alexander Shishkin, Peter Zijlstra

On Tue, Nov 20, 2018 at 12:56:04PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Nov 20, 2018 at 11:56:18AM +0100, Jiri Olsa escreveu:
> > On Mon, Nov 19, 2018 at 08:10:06AM -0800, Arnaldo Carvalho de Melo wrote:
> > > Em Thu, Nov 01, 2018 at 06:00:01PM +0100, Jiri Olsa escreveu:
> > > > Adding the current working directory to be part of the cookie
> > > > and restoring it in the nsinfo__mountns_exit call.
> > >  
> > > > diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> > > > index cf8bd123cf73..fb0458b7e6aa 100644
> > > > --- a/tools/perf/util/namespaces.c
> > > > +++ b/tools/perf/util/namespaces.c
> > > 
> > > > @@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
> > > >  	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
> > > >  		return;
> > >   
> > > > +	oldcwd = get_current_dir_name();
> > > 
> > > Breaks the build with bionic (Android), where get_current_dir_name() is
> > > not available:
> > > 
> > > /tmp/build/perf/libperf.a(libperf-in.o):hist.c:function nsinfo__mountns_enter: error: undefined reference to 'get_current_dir_name'
> > > collect2: error: ld returned 1 exit status
> > > Makefile.perf:534: recipe for target '/tmp/build/perf/perf' failed
> > > make[2]: *** [/tmp/build/perf/perf] Error 1
> > > Makefile.perf:206: recipe for target 'sub-make' failed
> > > make[1]: *** [sub-make] Error 2
> > > Makefile:69: recipe for target 'all' failed
> > > make: *** [all] Error 2
> > > make: Leaving directory '/git/linux/tools/perf'
> > > [root@seventh ~]# 
> > > 
> > > The container builds are ongoing, its possoble that this becomes an
> > > issue in uCLibc, wasn't on musl libc (Alpine Linux).
> > 
> > will check, thanks
> 
> I fixed this already, by adding a test for that get_current_dir_name()
> function and a simple implementation based on strdup(getcwd(bf)), take a
> look at my perf/urgent branch 

ok, I saw it.. but it seems little too much for a single function to me

jirka

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tip:perf/urgent] perf tools: Restore proper cwd on return from mnt namespace
  2018-11-01 17:00 [PATCH] perf tools: Restore proper cwd on return from mnt ns Jiri Olsa
                   ` (2 preceding siblings ...)
  2018-11-19 16:10 ` Arnaldo Carvalho de Melo
@ 2018-11-21 15:02 ` tip-bot for Jiri Olsa
  3 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Jiri Olsa @ 2018-11-21 15:02 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, mingo, linux-kernel, namhyung, jolsa, alexander.shishkin,
	acme, tglx, kjlx, hpa

Commit-ID:  b01c1f69c8660eaeab7d365cd570103c5c073a02
Gitweb:     https://git.kernel.org/tip/b01c1f69c8660eaeab7d365cd570103c5c073a02
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 1 Nov 2018 18:00:01 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 19 Nov 2018 12:12:26 -0800

perf tools: Restore proper cwd on return from mnt namespace

When reporting on 'record' server we try to retrieve/use the mnt
namespace of the profiled tasks. We use following API with cookie to
hold the return namespace, roughly:

  nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc)
    setns(newns, 0);
  ...
  new ns related open..
  ...
  nsinfo__mountns_exit(struct nscookie *nc)
    setns(nc->oldns)

Once finished we setns to old namespace, which also sets the current
working directory (cwd) to "/", trashing the cwd we had.

This is mostly fine, because we use absolute paths almost everywhere,
but it screws up 'perf diff':

  # perf diff
  failed to open perf.data: No such file or directory  (try 'perf record' first)
  ...

Adding the current working directory to be part of the cookie and
restoring it in the nsinfo__mountns_exit call.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Krister Johansen <kjlx@templeofstupid.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Fixes: 843ff37bb59e ("perf symbols: Find symbols in different mount namespace")
Link: http://lkml.kernel.org/r/20181101170001.30019-1-jolsa@kernel.org
[ No need to check for NULL args for free(), use zfree() for struct members ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/namespaces.c | 17 +++++++++++++++--
 tools/perf/util/namespaces.h |  1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
index cf8bd123cf73..aed170bd4384 100644
--- a/tools/perf/util/namespaces.c
+++ b/tools/perf/util/namespaces.c
@@ -18,6 +18,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <asm/bug.h>
 
 struct namespaces *namespaces__new(struct namespaces_event *event)
 {
@@ -186,6 +187,7 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
 	char curpath[PATH_MAX];
 	int oldns = -1;
 	int newns = -1;
+	char *oldcwd = NULL;
 
 	if (nc == NULL)
 		return;
@@ -199,9 +201,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
 	if (snprintf(curpath, PATH_MAX, "/proc/self/ns/mnt") >= PATH_MAX)
 		return;
 
+	oldcwd = get_current_dir_name();
+	if (!oldcwd)
+		return;
+
 	oldns = open(curpath, O_RDONLY);
 	if (oldns < 0)
-		return;
+		goto errout;
 
 	newns = open(nsi->mntns_path, O_RDONLY);
 	if (newns < 0)
@@ -210,11 +216,13 @@ void nsinfo__mountns_enter(struct nsinfo *nsi,
 	if (setns(newns, CLONE_NEWNS) < 0)
 		goto errout;
 
+	nc->oldcwd = oldcwd;
 	nc->oldns = oldns;
 	nc->newns = newns;
 	return;
 
 errout:
+	free(oldcwd);
 	if (oldns > -1)
 		close(oldns);
 	if (newns > -1)
@@ -223,11 +231,16 @@ errout:
 
 void nsinfo__mountns_exit(struct nscookie *nc)
 {
-	if (nc == NULL || nc->oldns == -1 || nc->newns == -1)
+	if (nc == NULL || nc->oldns == -1 || nc->newns == -1 || !nc->oldcwd)
 		return;
 
 	setns(nc->oldns, CLONE_NEWNS);
 
+	if (nc->oldcwd) {
+		WARN_ON_ONCE(chdir(nc->oldcwd));
+		zfree(&nc->oldcwd);
+	}
+
 	if (nc->oldns > -1) {
 		close(nc->oldns);
 		nc->oldns = -1;
diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h
index cae1a9a39722..d5f46c09ea31 100644
--- a/tools/perf/util/namespaces.h
+++ b/tools/perf/util/namespaces.h
@@ -38,6 +38,7 @@ struct nsinfo {
 struct nscookie {
 	int			oldns;
 	int			newns;
+	char			*oldcwd;
 };
 
 int nsinfo__init(struct nsinfo *nsi);

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-11-21 15:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-01 17:00 [PATCH] perf tools: Restore proper cwd on return from mnt ns Jiri Olsa
2018-11-16 13:03 ` Jiri Olsa
2018-11-16 18:31 ` Arnaldo Carvalho de Melo
2018-11-16 18:42   ` Arnaldo Carvalho de Melo
2018-11-17  9:00     ` Jiri Olsa
2018-11-19 16:10 ` Arnaldo Carvalho de Melo
2018-11-20 10:56   ` Jiri Olsa
2018-11-20 11:23     ` [PATCHv2] " Jiri Olsa
2018-11-20 15:56     ` [PATCH] " Arnaldo Carvalho de Melo
2018-11-20 17:42       ` Jiri Olsa
2018-11-21 15:02 ` [tip:perf/urgent] perf tools: Restore proper cwd on return from mnt namespace tip-bot for Jiri Olsa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.