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: Wednesday, December 23, 2020 12:10 PM To: crash-utility(a)redhat.com Subject: Re: [Crash-utility] [PATCH V2] netdump: fix regression for tiny kdump files 在 2020年12月23日 10:31, crash-utility-request(a)redhat.com 写道: > Date: Wed, 23 Dec 2020 02:31:24 +0000 > From: ??? <zhaoqianli(a)xiaomi.com&gt; > To: =?gb2312?B?SEFHSU8gS0FaVUhJVE8oyGPOsqGh0rvIyik=?= > <k-hagio-ab(a)nec.com&gt;, "Discussion list for crash utility usage, > maintenance and development" <crash-utility(a)redhat.com&gt; > Subject: Re: [Crash-utility] [External Mail]RE: [PATCH V2] netdump: > fix regression for tiny kdump files > Message-ID: <e4cc6f753b2c428d9f0f0c0362567de5(a)xiaomi.com&gt; > Content-Type: text/plain; charset="gb2312" > > Hi, Lianbo > > >> 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) { > I agrre with Kazu, if use "size != MIN_NETDUMP_ELF_HEADER_SIZE/SAFE_NETDUMP_ELF_HEADER_SIZE" this issue can not be fixed, size is equal to or more than MIN_NETDUMP_ELF_HEADER_SIZE is valid. > That's true, I just knew what it happened. Thanks for the explanation. >> 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); >> } > Ok. > > How about below patch set? > Looks good to me. Acked by: Lianbo Jiang <lijian(a)redhat.&gt; >>From 5ec6ac06ba7fd32e96463a54ebc3f733f1054a90 Mon Sep 17 00:00:00 2001 > From: Qianli Zhao <zhaoqianli(a)xiaomi.com&gt; > Date: Mon, 30 Nov 2020 17:17:32 +0800 > Subject: [PATCH] netdump: fix regression for tiny kdump files > > 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 can lead to crash rejecting > raw RAM dumpfiles > > 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; > --- > netdump.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/netdump.c b/netdump.c > index c76d9dd..ca9b459 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; > @@ -134,7 +135,7 @@ is_netdump(char *file, ulong source_query) > > 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); > } > > @@ -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) { > + 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