[Crash-utility] [PATCH] Add file size check at the beginning

[Dave Anderson]

Bernhard Walle wrote:
> I had a bug report because of a truncated vmcore file. The error message just
> was
> 
>     crash: read error: kernel virtual address: ffff8107f3c3a6c0  type: "cpu_pda
>     entry"
> 
> which is very unclear for a 'normal' user. This patch adds checking of the file
> size according to the ELF header at the beginning so that a clear error message
> can be printed.
> 
> Please consider adding the patch to crash.
> 

I'm hesitant about is_netdump() simply returning FALSE, and therefore
essentially saying: "this isn't a netdump or kdump file -- go check
whether it's some other dumpfile type".

And what if the dumpfile is short, but still usable?  We sure
have seen a lot of real netdump files (i.e. not kdumps) that
have been truncated, but still usable.

I understand the concern about the read error message, but maybe
a better way to do it would be to just issue a WARNING message?

Dave


> 
> Signed-off-by: Bernhard Walle <bwalle at suse.de>
> 
> ---
>  defs.h    |    1 +
>  netdump.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> --- a/defs.h
> +++ b/defs.h
> @@ -16,6 +16,7 @@
>   * GNU General Public License for more details. 
>   */
>  
> +#define _LARGEFILE64_SOURCE 1 /* stat64() */
>  #ifndef GDB_COMMON
>  
>  #include <stdio.h>
> --- a/netdump.c
> +++ b/netdump.c
> @@ -33,6 +33,47 @@ static physaddr_t xen_kdump_p2m(physaddr
>  #define ELFREAD  0
>  
>  #define MIN_PAGE_SIZE (4096)
> +
> +
> +static int
> +check_netdump_filesize(char *file)
> +{
> +	uint64_t max_file_offset = 0;
> +	struct pt_load_segment *pls;
> +	struct stat64 stat;
> +	int i, ret;
> +
> +
> +	/* find the maximum file offset */
> +	for (i = 0; i < nd->num_pt_load_segments; i++) {
> +		uint64_t end, size;
> +
> +		pls = &nd->pt_load_segments[i];
> +
> +		size = pls->phys_end - pls->phys_start;
> +		end = pls->file_offset + size;
> +
> +		if (end > max_file_offset)
> +			max_file_offset = end;
> +	}
> +
> +	ret = stat64(file, &stat);
> +	if (ret < 0) {
> +		fprintf(stderr, "Cannot stat64 on %s: %s\n", file,
> +				strerror(errno));
> +		return FALSE;
> +	}
> +
> +	if (max_file_offset > stat.st_size) {
> +		fprintf(stderr, "File %s is too short:\n"
> +				"Must be %lld bytes but is only "
> +				"%lld bytes long.\n",
> +				file, max_file_offset, stat.st_size);
> +		return FALSE;
> +	}
> +
> +	return TRUE;
> +}
>  	
>  /*
>   *  Determine whether a file is a netdump/diskdump/kdump creation, 
> @@ -267,6 +308,9 @@ is_netdump(char *file, ulong source_quer
>  	if (CRASHDEBUG(1))
>  		netdump_memory_dump(fp);
>  
> +	if (!check_netdump_filesize(file))
> +		return FALSE;
> +
>  	return nd->header_size;
>  
>  bailout:
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility