[PATCH v6 3/5] Use get_proc_pid for /proc paths

[Dmitry V. Levin]

On Thu, Aug 06, 2020 at 09:02:09PM +0200, Ákos Uzonyi wrote:
> * mmap_cache.c (mmap_cache_rebuild_if_invalid): Use proc pid instead of
> tcp->pid for /proc path.
> * util.c (getfdproto): Likewise.
> (pidfd_get_pid): Likewise.
> * pathtrace.c (getfdpath_pid): Likewise.
> * strace.c (attach_tcb): Likewise.
> ---
>  mmap_cache.c | 2 +-
>  pathtrace.c  | 5 ++++-
>  strace.c     | 2 +-
>  util.c       | 7 +++++--
>  4 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/mmap_cache.c b/mmap_cache.c
> index 89c62254..9825df26 100644
> --- a/mmap_cache.c
> +++ b/mmap_cache.c
> @@ -84,7 +84,7 @@ mmap_cache_rebuild_if_invalid(struct tcb *tcp, const char *caller)
>  		return MMAP_CACHE_REBUILD_READY;
>  
>  	char filename[sizeof("/proc/4294967296/maps")];
> -	xsprintf(filename, "/proc/%u/maps", tcp->pid);
> +	xsprintf(filename, "/proc/%u/maps", get_proc_pid(tcp));
>  
>  	FILE *fp = fopen_stream(filename, "r");
>  	if (!fp) {
> diff --git a/pathtrace.c b/pathtrace.c
> index 5b60762b..74717a8a 100644
> --- a/pathtrace.c
> +++ b/pathtrace.c
> @@ -87,7 +87,10 @@ getfdpath_pid(pid_t pid, int fd, char *buf, unsigned bufsize)
>  	if (fd < 0)
>  		return -1;
>  
> -	xsprintf(linkpath, "/proc/%u/fd/%u", pid, fd);
> +	int proc_pid = 0;
> +	translate_pid(NULL, pid, PT_TID, &proc_pid);
> +
> +	xsprintf(linkpath, "/proc/%u/fd/%u", proc_pid, fd);

This looks like an emulation of get_proc_pid(tcp) for the case
when we have no tcp.

Fortunately, we have a tcp argument, it's in function getfdpath.
Since there are no users of getfdpath_pid that couldn't use getfdpath
instead, I suggest removing getfdpath_pid and defining just getfdpath,
this would make the code clearer.

>  	n = readlink(linkpath, buf, bufsize - 1);
>  	/*
>  	 * NB: if buf is too small, readlink doesn't fail,
> diff --git a/util.c b/util.c
> index 094e5818..d87d022e 100644
> --- a/util.c
> +++ b/util.c
> @@ -582,8 +582,11 @@ printdev(struct tcb *tcp, int fd, const char *path)
>  pid_t
>  pidfd_get_pid(pid_t pid_of_fd, int fd)
>  {
> +	int proc_pid = 0;
> +	translate_pid(NULL, pid_of_fd, PT_TID, &proc_pid);
> +
>  	char fdi_path[sizeof("/proc/%u/fdinfo/%u") + 2 * sizeof(int) * 3];
> -	xsprintf(fdi_path, "/proc/%u/fdinfo/%u", pid_of_fd, fd);
> +	xsprintf(fdi_path, "/proc/%u/fdinfo/%u", proc_pid, fd);
>  
>  	FILE *f = fopen_stream(fdi_path, "r");
>  	if (!f)

Likewise, this would invoke the whole translate_pid machinery even for the
case when is_proc_ours() returns true.  Could something be done about it?

The idea is not to bring all these heavy translation mechanisms up
unnecessarily, that is, unless /proc is messed up or --pidns-translation
is specified.


-- 
ldv