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

[lijiang]

在 2020年12月23日 10:31, crash-utility-request at redhat.com 写道:
> Date: Wed, 23 Dec 2020 02:31:24 +0000
> From: ??? <zhaoqianli at xiaomi.com>
> To: =?gb2312?B?SEFHSU8gS0FaVUhJVE8oyGPOsqGh0rvIyik=?=
> 	<k-hagio-ab at nec.com>,	"Discussion list for crash utility usage,
> 	maintenance and development"	<crash-utility at redhat.com>
> Subject: Re: [Crash-utility] [External Mail]RE: [PATCH V2] netdump:
> 	fix regression for tiny	kdump files
> Message-ID: <e4cc6f753b2c428d9f0f0c0362567de5 at xiaomi.com>
> 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 at redhat.>


>>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