[Crash-utility] [PATCH] fix "ps -a" for tasks whose arg_start was moved

[Dave Anderson]

----- Original Message -----
> When a task used prctl(PR_SET_MM, PR_SET_MM_ARG_{START,END}, ...)
> (e.g. the systemd's child "(sd-pam)" in Fedora), the "ps -a" option
> could fail with "ps: cannot allocate any more memory!".

Good catch!  Queued for crash-7.2.2:
  
  https://github.com/crash-utility/crash/commit/4e8c1f3720c7bc3ca074c816332edf2a865dc681

Thanks,
  Dave


>
>   # ./crash -d 1
>   ...
>   crash> ps -a 1591
>   ...
>   PID: 1591   TASK: ffff9c7077d7b1c0  CPU: 1   COMMAND: "(sd-pam)"
>   arg_start: 7f2f544de000 arg_end: 7f2f544de009 (9)
>   env_start: 7ffcff12af88 env_end: 7ffcff12afdf (87)
>   ...
>         smallest: 16
>          largest: 883333320672
>         embedded: 2
>     max_embedded: 3
>          mallocs: 22
>            frees: 22
>       reqs/total: 912/1766672402731
>     average size: 1937140792
>   ps: cannot allocate any more memory!
> 
> This happens because it assumes that the ranges of arg and env are
> contiguous.
> 
>   buf = GETBUF(env_end - arg_start + 1);
> 
> This patch devides the GETBUF() into two times for arg and env.
> 
> Signed-off-by: Kazuhito Hagio <k-hagio at ab.jp.nec.com>
> ---
>  task.c | 94
>  +++++++++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 53 insertions(+), 41 deletions(-)
> 
> diff --git a/task.c b/task.c
> index 2418e4c..55242c5 100644
> --- a/task.c
> +++ b/task.c
> @@ -3865,6 +3865,44 @@ show_milliseconds(struct task_context *tc, struct
> psinfo *psi)
>  	}
>  }
>  
> +static char *
> +read_arg_string(struct task_context *tc, char *buf, ulong start, ulong end)
> +{
> +	physaddr_t paddr;
> +	ulong uvaddr, size, cnt;
> +	char *bufptr;
> +
> +	uvaddr = start;
> +	size = end - start;
> +	bufptr = buf;
> +
> +	while (size > 0) {
> +		if (!uvtop(tc, uvaddr, &paddr, 0)) {
> +			error(INFO, "cannot access user stack address: %lx\n\n",
> +				uvaddr);
> +			return NULL;
> +		}
> +
> +		cnt = PAGESIZE() - PAGEOFFSET(uvaddr);
> +
> +		if (cnt > size)
> +			cnt = size;
> +
> +		if (!readmem(paddr, PHYSADDR, bufptr, cnt,
> +		    "user stack contents", RETURN_ON_ERROR|QUIET)) {
> +			error(INFO, "cannot access user stack address: %lx\n\n",
> +				uvaddr);
> +			return NULL;
> +		}
> +
> +		uvaddr += cnt;
> +		bufptr += cnt;
> +		size -= cnt;
> +	}
> +
> +	return bufptr;
> +}
> +
>  /*
>   *  Show the argv and envp strings pointed to by mm_struct->arg_start
>   *  and mm_struct->env_start.  The user addresses need to broken up
> @@ -3875,10 +3913,7 @@ static void
>  show_task_args(struct task_context *tc)
>  {
>  	ulong arg_start, arg_end, env_start, env_end;
> -	char *buf, *bufptr, *p1;
> -	char *as, *ae, *es, *ee;
> -	physaddr_t paddr;
> -	ulong uvaddr, size, cnt;
> +	char *buf, *p1, *end;
>  	int c, d;
>  
>  	print_task_header(fp, tc, 0);
> @@ -3910,43 +3945,13 @@ show_task_args(struct task_context *tc)
>  			env_start, env_end, env_end - env_start);
>  	}
>  
> -	buf = GETBUF(env_end - arg_start + 1);
> -
> -	uvaddr = arg_start;
> -	size = env_end - arg_start;
> -	bufptr = buf;
> -
> -	while (size > 0) {
> -        	if (!uvtop(tc, uvaddr, &paddr, 0)) {
> -                	error(INFO, "cannot access user stack address: %lx\n\n",
> -                        	uvaddr);
> -			goto bailout;
> -        	}
> -
> -		cnt = PAGESIZE() - PAGEOFFSET(uvaddr);
> -
> -		if (cnt > size)
> -			cnt = size;
> -
> -        	if (!readmem(paddr, PHYSADDR, bufptr, cnt,
> -                    "user stack contents", RETURN_ON_ERROR|QUIET)) {
> -                	error(INFO, "cannot access user stack address: %lx\n\n",
> -                        	uvaddr);
> -			goto bailout;
> -        	}
> -
> -		uvaddr += cnt;
> -                bufptr += cnt;
> -                size -= cnt;
> -	}
> -
> -	as = buf;
> -	ae = &buf[arg_end - arg_start];
> -	es = &buf[env_start - arg_start];
> -	ee = &buf[env_end - arg_start];
> +	buf = GETBUF(arg_end - arg_start + 1);
> +	end = read_arg_string(tc, buf, arg_start, arg_end);
> +	if (!end)
> +		goto bailout;
>  
>  	fprintf(fp, "ARG: ");
> -	for (p1 = as, c = 0; p1 < ae; p1++) {
> +	for (p1 = buf, c = 0; p1 < end; p1++) {
>  		if (*p1 == NULLCHAR) {
>  			if (c)
>  				fprintf(fp, " ");
> @@ -3957,14 +3962,21 @@ show_task_args(struct task_context *tc)
>  		}
>  	}
>  
> +	FREEBUF(buf);
> +
> +	buf = GETBUF(env_end - env_start + 1);
> +	end = read_arg_string(tc, buf, env_start, env_end);
> +	if (!end)
> +		goto bailout;
> +
>  	fprintf(fp, "\nENV: ");
> -	for (p1 = es, c = d = 0; p1 < ee; p1++) {
> +	for (p1 = buf, c = d = 0; p1 < end; p1++) {
>  		if (*p1 == NULLCHAR) {
>  			if (c)
>  				fprintf(fp, "\n");
>  			c = 0;
>  		} else {
> -			fprintf(fp, "%s%c", !c && (p1 != es) ? "     " : "", *p1);
> +			fprintf(fp, "%s%c", !c && (p1 != buf) ? "     " : "", *p1);
>  			c++, d++;
>  		}
>  	}
> --
> 1.8.3.1
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>