|
Hi, Lianbo
> > + 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. > 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?
From 5ec6ac06ba7fd32e96463a54ebc3f733f1054a90 Mon Sep 17 00:00:00 2001
From: Qianli Zhao <zhaoqianli@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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
From: HAGIO KAZUHITO(?尾 一仁) <k-hagio-ab@xxxxxxx>
Sent: Tuesday, December 22, 2020 9:14 To: Discussion list for crash utility usage, maintenance and development Cc: 赵乾利 Subject: [External Mail]RE: [PATCH V2] netdump: fix regression for tiny kdump files *外部邮件，谨慎处理 | This message originated from outside of XIAOMI. Please treat this email with caution*
Hi Lianbo, > -----Original Message----- > From: crash-utility-bounces@xxxxxxxxxx <crash-utility-bounces@xxxxxxxxxx> On Behalf Of lijiang > Sent: Monday, December 21, 2020 11:42 PM > To: crash-utility@xxxxxxxxxx; zhaoqianli@xxxxxxxxxx > Subject: Re: [PATCH V2] netdump: fix regression for tiny kdump files > > Hi, Qianli > > Thanks for the patch. > 在 2020年12月01日 14:45, crash-utility-request@xxxxxxxxxx 写道: > > Date: Tue, 1 Dec 2020 10:56:02 +0800 > > From: Qianli Zhao <zhaoqianligood@xxxxxxxxx> > > To: crash-utility@xxxxxxxxxx, minipli@xxxxxxxxxxxxxx > > Subject: [PATCH V2] netdump: fix regression for tiny > > kdump files > > Message-ID: > > <1606791362-5604-1-git-send-email-zhaoqianligood@xxxxxxxxx> > > Content-Type: text/plain; charset="US-ASCII" > > > > From: Qianli Zhao <zhaoqianli@xxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > --- > > - 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 > Crash-utility@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/crash-utility |
-- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
