[Crash-utility] [External Mail]RE: [PATCH V2] netdump: fix regression for tiny kdump files
赵乾利
zhaoqianli at xiaomi.com
Wed Dec 23 02:31:24 UTC 2020
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.
> 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 at xiaomi.com>
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 at xiaomi.com>
---
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(Ècβ¡¡Ò»ÈÊ) <k-hagio-ab at nec.com>
Sent: Tuesday, December 22, 2020 9:14
To: Discussion list for crash utility usage, maintenance and development
Cc: ÕÔǬÀû
Subject: [External Mail]RE: [Crash-utility] [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 at redhat.com <crash-utility-bounces at redhat.com> On Behalf Of lijiang
> Sent: Monday, December 21, 2020 11:42 PM
> To: crash-utility at redhat.com; zhaoqianli at 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 at redhat.com дµÀ:
> > Date: Tue, 1 Dec 2020 10:56:02 +0800
> > From: Qianli Zhao <zhaoqianligood at gmail.com>
> > To: crash-utility at redhat.com, minipli at grsecurity.net
> > Subject: [Crash-utility] [PATCH V2] netdump: fix regression for tiny
> > kdump files
> > Message-ID:
> > <1606791362-5604-1-git-send-email-zhaoqianligood at gmail.com>
> > Content-Type: text/plain; charset="US-ASCII"
> >
> > From: Qianli Zhao <zhaoqianli at xiaomi.com>
> >
> > 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 at xiaomi.com>
> > ---
> > - 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
#/******±¾Óʼþ¼°Æ丽¼þº¬ÓÐСÃ×¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚ·¢Ë͸øÉÏÃæµØÖ·ÖÐÁгöµÄ¸öÈË»òȺ×é¡£½ûÖ¹ÈκÎÆäËûÈËÒÔÈκÎÐÎʽʹÓ㨰üÀ¨µ«²»ÏÞÓÚÈ«²¿»ò²¿·ÖµØй¶¡¢¸´ÖÆ¡¢»òÉ¢·¢£©±¾ÓʼþÖеÄÐÅÏ¢¡£Èç¹ûÄú´íÊÕÁ˱¾Óʼþ£¬ÇëÄúÁ¢¼´µç»°»òÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ£¡ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20201223/f59d3ea8/attachment.htm>
More information about the Crash-utility
mailing list