Hi Lianbo, > -----Original Message----- > From: [email protected] <[email protected]> On > Behalf Of lijiang > Sent: Monday, December 21, 2020 11:42 PM > To: [email protected]; [email protected] > Subject: Re: [Crash-utility] [PATCH V2] netdump: fix regression for tiny > kdump files > > Hi, Qianli > > Thanks for the patch. > 在 2020年12月01日 14:45, [email protected] 写道: > > Date: Tue, 1 Dec 2020 10:56:02 +0800 > > From: Qianli Zhao <[email protected]> > > To: [email protected], [email protected] > > Subject: [Crash-utility] [PATCH V2] netdump: fix regression for tiny > > kdump files > > Message-ID: > > <[email protected]> > > Content-Type: text/plain; charset="US-ASCII" > > > > From: Qianli Zhao <[email protected]> > > > > 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 <[email protected]> > > --- > > - 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, Kazu > > > 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 > [email protected] > https://www.redhat.com/mailman/listinfo/crash-utility -- Crash-utility mailing list [email protected] https://www.redhat.com/mailman/listinfo/crash-utility
