>> Date: Tue, 1 Dec 2020 10:56:02 +0800 >> From: Qianli Zhao <zhaoqianligood(a)gmail.com&gt; >> To: crash-utility(a)redhat.com, minipli(a)grsecurity.net >> Subject: [Crash-utility] [PATCH V2] netdump: fix regression for tiny >> kdump files >> Message-ID: >> <1606791362-5604-1-git-send-email-zhaoqianligood(a)gmail.com&gt; >> Content-Type: text/plain; charset="US-ASCII" >> >> From: Qianli Zhao <zhaoqianli(a)xiaomi.com&gt; >> >> 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(a)xiaomi.com&gt; >> --- >> - 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

Thanks,