[Crash-utility] [PATCH 2/2] tools: fix potential source and destination overlap with strcpy()

[HAGIO KAZUHITO(萩尾 一仁)]

-----Original Message-----
> valgrind detects the following error:
> 
>     ==14603== Source and destination overlap in strcpy(0x1ffefffe42, 0x1ffefffe44)
>     ==14603==    at 0x483CD70: strcpy (vg_replace_strmem.c:511)
>     ==14603==    by 0x477813: pages_to_size (tools.c:6393)
>     ==14603==    by 0x4F292E: display_sys_stats (kernel.c:5629)
>     ==14603==    by 0x464BC7: main_loop (main.c:797)
>     ==14603==    by 0x6BE352: captured_command_loop (main.c:258)
>     ==14603==    by 0x6BC959: catch_errors (exceptions.c:557)
>     ==14603==    by 0x6BF3D5: captured_main (main.c:1064)
>     ==14603==    by 0x6BC959: catch_errors (exceptions.c:557)
>     ==14603==    by 0x6BF686: gdb_main (main.c:1079)
>     ==14603==    by 0x6BF686: gdb_main_entry (main.c:1099)
>     ==14603==    by 0x46316F: main (main.c:708)
>     ==14603==
> 
> pages_to_size() removes ".0 " if it is contained in the created string
> by overwriting them using strcpy() with the following "MB\0" or
> "GB\0". However, strcpy() doesn't accept such overlapping source and
> destination and thus use of strcpy() in this case is illegal.
> 
> Let's fix this by re-implementing the logic by memmove() where
> destination and source strings may overlap.
> 
> Signed-off-by: HATAYAMA Daisuke <d.hatayama at fujitsu.com>

I thought ".0 " does not need to be removed, but ok we are used to it..

Acked-by: Kazuhito Hagio <k-hagio-ab at nec.com>

Merged these two patches:
https://github.com/crash-utility/crash/commits/3972c86695954d446a6301282a21acc8e6967ea2

Thanks,
Kazu

> ---
>  tools.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/tools.c b/tools.c
> index 89352b1..71bac6d 100644
> --- a/tools.c
> +++ b/tools.c
> @@ -6371,7 +6371,7 @@ char *
>  pages_to_size(ulong pages, char *buf)
>  {
>  	double total;
> -	char *p1, *p2;
> +	char *p;
> 
>  	if (pages == 0) {
>  		sprintf(buf, "0");
> @@ -6387,11 +6387,8 @@ pages_to_size(ulong pages, char *buf)
>          else
>          	sprintf(buf, "%ld KB", (ulong)(total/(double)KILOBYTES(1)));
> 
> -	if ((p1 = strstr(buf, ".0 "))) {
> -		p2 = p1 + 3;
> -		*p1++ = ' ';
> -		strcpy(p1, p2);
> -	}
> +	if ((p = strstr(buf, ".0 ")))
> +		memmove(p, p + 2, sizeof(" GB"));
> 
>  	return buf;
>  }
> --
> 2.29.2
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility