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(a)fujitsu.com&gt; --- 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

Any comments? Thanks. HATAYAMA, Daisuke ________________________________________ From: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com&gt; Sent: Thursday, December 31, 2020 17:20 To: crash-utility(a)redhat.com Cc: Hatayama, Daisuke/畑山 大輔 Subject: [PATCH 1/2] netdump: fix illegal read to already freed buffer This issue was detected by valgrind as follows: ==1212== Invalid read of size 8 ==1212== at 0x56C400: resize_elf_header (netdump.c:585) ==1212== by 0x56C400: is_netdump (netdump.c:363) ==1212== by 0x463571: main (main.c:561) ==1212== Address 0x4e8ec10 is 32 bytes inside a block of size 304 free'd ==1212== at 0x483BCE8: realloc (vg_replace_malloc.c:834) ==1212== by 0x56C393: resize_elf_header (netdump.c:547) ==1212== by 0x56C393: is_netdump (netdump.c:363) ==1212== by 0x463571: main (main.c:561) ==1212== Block was alloc'd at ==1212== at 0x4839809: malloc (vg_replace_malloc.c:307) ==1212== by 0x56C078: is_netdump (netdump.c:136) ==1212== by 0x463571: main (main.c:561) ==1212== The issue was introduced by the commit f42db6a33f0e0652df7cce8506352745b4794287 (Support core files with "unusual" layout). In resize_elf_header(), both elf32 and elf64 refer to the same address as eheader, but when reallocating the address pointed at by eheader, elf32 and elf64 are not updated, resulting in referring to the already freed address. To fix this issue, let's update elf32 and elf64 at the realloc(). Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com&gt; --- netdump.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/netdump.c b/netdump.c index 2ca39e2..690f45e 100644 --- a/netdump.c +++ b/netdump.c @@ -550,6 +550,9 @@ resize_elf_header(int fd, char *file, char **eheader_ptr, char **sect0_ptr, } else *eheader_ptr = eheader; + elf32 = (Elf32_Ehdr *)&eheader[0]; + elf64 = (Elf64_Ehdr *)&eheader[0]; + if (FLAT_FORMAT()) { if (!read_flattened_format(fd, 0, eheader, header_size)) return 0; -- 2.29.2