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

-----Original Message----- From: crash-utility-bounces(a)redhat.com <crash-utility-bounces(a)redhat.com&gt; On Behalf Of lijiang Sent: Monday, December 21, 2020 11:42 PM To: crash-utility(a)redhat.com; zhaoqianli(a)xiaomi.com Subject: Re: [Crash-utility] [PATCH V2] netdump: fix regression for tiny kdump files Hi, Qianli Thanks for the patch. 在 2020年12月01日 14:45, crash-utility-request(a)redhat.com 写道: > 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; }

In addition, would you mind updating another error output in the is_netdump()? For example: size = SAFE_NETDUMP_ELF_HEADER_SIZE; if ((eheader = (char *)malloc(size)) == NULL) { - fprintf(stderr, "cannot malloc minimum ELF header buffer\n"); + fprintf(stderr, "cannot malloc ELF header buffer\n"); clean_exit(1); } Thanks. Lianbo > + fprintf(stderr, "%s: file too small!\n", file); > + goto bailout; > } > } > > -- 2.7.4 -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility