[Crash-utility] [PATCH 1/2] netdump: fix illegal read to already freed buffer

[lijiang]

Hi, HATAYAMA

在 2021年01月01日 01:00, crash-utility-request at redhat.com 写道:
> Date: Thu, 31 Dec 2020 17:20:52 +0900
> From: HATAYAMA Daisuke <d.hatayama at fujitsu.com>
> To: crash-utility at redhat.com
> Cc: HATAYAMA Daisuke <d.hatayama at fujitsu.com>
> Subject: [Crash-utility] [PATCH 1/2] netdump: fix illegal read to
> 	already	freed buffer
> Message-ID: <1609402853-11183-1-git-send-email-d.hatayama at fujitsu.com>
> Content-Type: text/plain; charset="US-ASCII"
> 
> 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().
> 

Thanks for the fix and improvement:
[PATCH 1/2] netdump: fix illegal read to already freed buffer
[PATCH 2/2] tools: fix potential source anddestination overlap with strcpy()

Acked-by: Lianbo Jiang <lijiang at redhat.com>

> Signed-off-by: HATAYAMA Daisuke <d.hatayama at fujitsu.com>
> ---
>  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