[Crash-utility] [PATCH V2] netdump: fix regression for tiny kdump files

[lijiang]

在 2020年12月23日 01:00, crash-utility-request at redhat.com 写道:
>>> Date: Tue,  1 Dec 2020 10:56:02 +0800
>>> From: Qianli Zhao <zhaoqianligood at gmail.com>
>>> To: crash-utility at redhat.com, minipli at grsecurity.net
>>> Subject: [Crash-utility] [PATCH V2] netdump: fix regression for tiny
>>> 	kdump	files
>>> Message-ID:
>>> 	<1606791362-5604-1-git-send-email-zhaoqianligood at gmail.com>
>>> Content-Type: text/plain; charset="US-ASCII"
>>>
>>> From: Qianli Zhao <zhaoqianli at xiaomi.com>
>>>
>>> Commit f42db6a33f0e ("Support core files with "unusual" layout")
>>> increased the minimal file size from MIN_NETDUMP_ELF_HEADER_SIZE to
>>> SAFE_NETDUMP_ELF_HEADER_SIZE which lead to crash rejecting very
>>> small kdump files.
>>>
>> Good findings.
>>
>>> Fix that by erroring out only if we get less than
>>> MIN_NETDUMP_ELF_HEADER_SIZE bytes.
>>>
>>> Signed-off-by: Qianli Zhao <zhaoqianli at xiaomi.com>
>>> ---
>>> - Update commit message
>>> - Add more accurate judgment of read() return value
>>> ---
>>>  netdump.c | 9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/netdump.c b/netdump.c
>>> index c76d9dd..9a36931 100644
>>> --- a/netdump.c
>>> +++ b/netdump.c
>>> @@ -119,7 +119,8 @@ is_netdump(char *file, ulong source_query)
>>>  	Elf64_Phdr *load64;
>>>  	char *eheader, *sect0;
>>>  	char buf[BUFSIZE];
>>> -	size_t size, len, tot;
>>> +	ssize_t size;
>>> +	size_t len, tot;
>>>          Elf32_Off offset32;
>>>          Elf64_Off offset64;
>>>  	ulong format;
>>> @@ -142,10 +143,14 @@ is_netdump(char *file, ulong source_query)
>>>  		if (!read_flattened_format(fd, 0, eheader, size))
>>>  			goto bailout;
>>>  	} else {
>>> -		if (read(fd, eheader, size) != size) {
>>> +		size = read(fd, eheader, size);
>>> +		if (size < 0) {
>>>  			sprintf(buf, "%s: ELF header read", file);
>>>  			perror(buf);
>>>  			goto bailout;
>>> +		} else if (size < MIN_NETDUMP_ELF_HEADER_SIZE) {
>> For the checking condition, I would recommend using the following methods, what do you think?
>>
>> +               if (size != SAFE_NETDUMP_ELF_HEADER_SIZE &&
>> +                   size != MIN_NETDUMP_ELF_HEADER_SIZE) {
>>                         sprintf(buf, "%s: ELF header read", file);
>>                         perror(buf);
>>                         goto bailout;
>>                 }
> Do you mean putting "size < 0" and "size < MIN_NETDUMP_ELF_HEADER SIZE"
> together?  I think it would be good to separate an read error and a format
> error for better debugging.
> 
> And according to ramdump_to_elf(), the size of an ELF header from a RAM
> dumpfile varies depending on the number of nodes, and is equal to or more
> than MIN_NETDUMP_ELF_HEADER_SIZE if valid.  Actually, the value that Qianli
> showed before was 232 [1], which is not either SAFE_NETDUMP_ELF_HEADER_SIZE(304)
> or MIN_NETDUMP_ELF_HEADER_SIZE(176).
> 
> [1] https://www.redhat.com/archives/crash-utility/2020-November/msg00080.html
> 

Thank you for sharing the debug information, I already known what it happened.

Thanks.
Lianbo

> Thanks,