Hi, Qianli

Thanks for the patch.
在 2020年12月01日 14:45, crash-utility-requ...@redhat.com 写道:
> Date: Tue,  1 Dec 2020 10:56:02 +0800
> From: Qianli Zhao <zhaoqianlig...@gmail.com>
> To: crash-utility@redhat.com, mini...@grsecurity.net
> Subject: [Crash-utility] [PATCH V2] netdump: fix regression for tiny
>       kdump   files
> Message-ID:
>       <1606791362-5604-1-git-send-email-zhaoqianlig...@gmail.com>
> Content-Type: text/plain; charset="US-ASCII"
> 
> From: Qianli Zhao <zhaoqia...@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 <zhaoqia...@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;
                }


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@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Reply via email to