[Crash-utility] [PATCH 3/3] Fix a KASLR problem of sadump

[Hatayama, Daisuke]

> -----Original Message-----
> From: Takao Indoh [mailto:indou.takao at jp.fujitsu.com]
> Sent: Tuesday, October 10, 2017 6:26 PM
> To: crash-utility at redhat.com; Hatayama, Daisuke 
> <d.hatayama at jp.fujitsu.com>
> Subject: [PATCH 3/3] Fix a KASLR problem of sadump
> 
> This patch fix a problem that crash cannot open a dumpfile which can be
> captured by sadump in KASLR enabled kernel.
> 
> The sadump dumpfile has register information in the header, therefore
> this problem can be solved by the same way as virsh dump.
> 
> Signed-off-by: Takao Indoh <indou.takao at jp.fujitsu.com>
> ---
>  defs.h    |  3 +++
>  netdump.c |  3 +++
>  sadump.c  | 60
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  sadump.h  |  4 ++++
>  symbols.c | 10 ++++++----
>  5 files changed, 75 insertions(+), 5 deletions(-)
> 
> diff --git a/defs.h b/defs.h
> index 558ab7b..2c84d73 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -6263,12 +6263,15 @@ FILE *set_sadump_fp(FILE *);
>  void get_sadump_regs(struct bt_info *bt, ulong *ipp, ulong *spp);
>  void sadump_display_regs(int, FILE *);
>  int sadump_phys_base(ulong *);
> +int sadump_set_phys_base(ulong);
>  void sadump_show_diskset(void);
>  int sadump_is_zero_excluded(void);
>  void sadump_set_zero_excluded(void);
>  void sadump_unset_zero_excluded(void);
>  struct sadump_data;
>  struct sadump_data *get_sadump_data(void);
> +ulong sadump_get_idtr(void);
> +ulong sadump_get_cr3(void);
> 
>  /*
>   * qemu.c
> diff --git a/netdump.c b/netdump.c
> index b03ba13..b0ce160 100644
> --- a/netdump.c
> +++ b/netdump.c
> @@ -5132,6 +5132,9 @@ calc_kaslr_offset(ulong *kaslr_offset, ulong
> *phys_base)
>  	if (vmcore_kaslr_check()) {
>  		idtr = qemu_get_idtr();
>  		cr3 = qemu_get_cr3();
> +	} else if (SADUMP_DUMPFILE()) {
> +		idtr = sadump_get_idtr();
> +		cr3 = sadump_get_cr3();
>  	} else
>  		return FALSE;
> 
> diff --git a/sadump.c b/sadump.c
> index a96ba9c..d2a5c47 100644
> --- a/sadump.c
> +++ b/sadump.c
> @@ -1558,15 +1558,27 @@ sadump_display_regs(int cpu, FILE *ofp)
>   */
>  int sadump_phys_base(ulong *phys_base)
>  {
> -	if (SADUMP_VALID()) {
> +	if (SADUMP_VALID() && !sd->phys_base) {
>  		if (CRASHDEBUG(1))
>  			error(NOTE, "sadump: does not save phys_base.\n");
>  		return FALSE;
>  	}
> 
> +	if (sd->phys_base) {
> +		*phys_base = sd->phys_base;
> +		return TRUE;
> +	}
> +
>  	return FALSE;
>  }
> 
> +int sadump_set_phys_base(ulong phys_base)
> +{
> +	sd->phys_base = phys_base;
> +
> +	return TRUE;
> +}
> +
>  /*
>   *  Used by "sys" command to show diskset disk names.
>   */
> @@ -1649,3 +1661,49 @@ get_sadump_data(void)
>  {
>  	return sd;
>  }
> +
> +static int
> +get_sadump_smram_cpu_state_any(struct sadump_smram_cpu_state *smram)
> +{
> +	ulong offset;
> +	struct sadump_header *sh = sd->dump_header;
> +	int apicid;
> +	struct sadump_smram_cpu_state scs, zero;
> +
> +	offset = sd->sub_hdr_offset + sizeof(uint32_t) +
> +		 sd->dump_header->nr_cpus * sizeof(struct
> sadump_apic_state);
> +
> +	memset(&zero, 0, sizeof(zero));
> +
> +	for (apicid = 0; apicid < sh->nr_cpus; ++apicid) {
> +		if (!read_device(&scs, sizeof(scs), &offset)) {
> +			error(INFO, "sadump: cannot read sub header "
> +			      "cpu_state\n");
> +			return FALSE;
> +		}
> +		if (memcmp(&scs, &zero, sizeof(scs)) != 0) {
> +			*smram = scs;
> +			return TRUE;
> +		}
> +	}
> +
> +	return FALSE;
> +}
> +
> +ulong
> +sadump_get_idtr(void)
> +{
> +	struct sadump_smram_cpu_state scs;
> +	get_sadump_smram_cpu_state_any(&scs);
> +
> +	return ((uint64_t)scs.IdtUpper)<<32 | (uint64_t)scs.IdtLower;
> +}

This returns physical address so should have type physaddr_t in meaning.

Also, I like:

physaddr_t
sadump_get_idtr(void)
{
        struct sadump_smram_cpu_state scs;

        get_sadump_smram_cpu_state_any(&scs);
        return ((uint64_t)scs.IdtUpper)<<32 | (uint64_t)scs.IdtLower;
}

> +
> +ulong
> +sadump_get_cr3(void)
> +{
> +	struct sadump_smram_cpu_state scs;
> +	get_sadump_smram_cpu_state_any(&scs);
> +
> +	return scs.Cr3;
> +}

Just as the above comment:

ulong
sadump_get_cr3(void)
{
    struct sadump_smram_cpu_state scs;

    get_sadump_smram_cpu_state_any(&scs);
    return scs.Cr3;
}

> diff --git a/sadump.h b/sadump.h
> index 7f8e384..66aa17e 100644
> --- a/sadump.h
> +++ b/sadump.h
> @@ -219,6 +219,10 @@ struct sadump_data {
>  	ulonglong backup_offset;
> 
>  	uint64_t max_mapnr;
> +
> +	ulong kaslr_offset;
> +	ulong phys_base;
> +	ulong cr3;
>  };
> 
>  struct sadump_data *sadump_get_sadump_data(void);
> diff --git a/symbols.c b/symbols.c
> index 9e0c9df..38720b5 100644
> --- a/symbols.c
> +++ b/symbols.c
> @@ -625,7 +625,7 @@ kaslr_init(void)
>  		}
>  	}
> 
> -	if (vmcore_kaslr_check())
> +	if (vmcore_kaslr_check() || SADUMP_DUMPFILE())
>  		kt->flags2 |= KASLR_CHECK;
>  }
> 
> @@ -640,7 +640,7 @@ derive_kaslr_offset(bfd *abfd, int dynamic, bfd_byte
> *start, bfd_byte *end,
>  	unsigned long relocate;
>  	ulong _stext_relocated;
> 
> -	if (vmcore_kaslr_check()) {
> +	if (vmcore_kaslr_check() || SADUMP_DUMPFILE()) {
>  		ulong kaslr_offset = 0, phys_base = 0;
> 
>  		calc_kaslr_offset(&kaslr_offset, &phys_base);
> @@ -650,8 +650,10 @@ derive_kaslr_offset(bfd *abfd, int dynamic, bfd_byte
> *start, bfd_byte *end,
>  			kt->flags |= RELOC_SET;
>  		}
> 
> -		if (phys_base)
> +		if (phys_base) {
>  			kdump_set_phys_base(phys_base);
> +			sadump_set_phys_base(phys_base);
> +		}
> 
>  		return;
>  	}
> @@ -12255,7 +12257,7 @@ numeric_forward(const void *P_x, const void *P_y)
>  		}
>  	}
> 
> -	if (vmcore_kaslr_check()) {
> +	if (vmcore_kaslr_check() || SADUMP_DUMPFILE()) {
>  		if (STREQ(x->name, "divide_error"))
>  			st->divide_error_vmlinux = valueof(x);
>  		else if (STREQ(y->name, "divide_error"))
> --
> 2.9.5