RE: [patch 3/3] IA64: verify the base address of crashkernel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: linux-ia64-owner@xxxxxxxxxxxxxxx
> [mailto:linux-ia64-owner@xxxxxxxxxxxxxxx] On Behalf Of Horms
> Sent: 2007年3月6日 15:29
> To: Linux-IA64; fastboot@xxxxxxxxxxxxxx
> Cc: Luck, Tony; Zou, Nanhai; Magnus Damm
> Subject: [patch 3/3] IA64: verify the base address of crashkernel
> 
> When the crashkernel command line argument is supplied, it may optionally
> include the base address at which to locate the region. If this is omitted
> then the kernel attempts to locate an appropriate base address and in
> the course of this checks to make sure it is placed in a region that
> is writable.
> 
> If, however, the base address is supplied on the command line this check is
> not made. This patch adds validation code to:
> 
> * Warn is the base address is not aligned (to 64Mb)
> * Invalidate the crashkernel region if it does not lie with in
>   an appropriate EFI region
> * Invalidate the crash kernel region if it clashes with a reserved region
>   (this was kind of handled already by virtue of the way the resource
>    is inserted into /proc/iomem)
> 
> It also defined CRASHDUMP_ALIGN rather than just hardcoding it
> to 64Mb as needed (previously only in kdump_find_kdump_find_rsvd_region())
> 
> The code came out to be a little longer than I had expected.
> It could be made more compact, likely at the expense of some readability.
> But as its in __init, its not really a big deal IMHO.
> 
> An alternative approach would to be add checks at the time the
> region is inserted to /proc/iomem. I'm not sure how well this would
> work, but I am happy to investigate.
> 
> Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>
> 
>  arch/ia64/kernel/efi.c   |   84
> ++++++++++++++++++++++++++++++++++++++++++++--
>  arch/ia64/kernel/setup.c |   10 ++++-
>  include/asm-ia64/kexec.h |    2 +
>  3 files changed, 91 insertions(+), 5 deletions(-)
> 
> Index: linux-2.6/arch/ia64/kernel/efi.c
> ===================================================================
> --- linux-2.6.orig/arch/ia64/kernel/efi.c	2007-03-06 16:25:46.000000000
> +0900
> +++ linux-2.6/arch/ia64/kernel/efi.c	2007-03-06 16:26:41.000000000 +0900
> @@ -1150,6 +1150,85 @@
>  }
> 
>  #ifdef CONFIG_KEXEC
> +
> +#define CRASHDUMP_ALIGNMENT (1 << _PAGE_SIZE_64M)
> +static int __init
> +kdump_region_verify_efi (unsigned long base, unsigned long size)
> +{
> +	void *efi_map_start, *efi_map_end, *p;
> +	efi_memory_desc_t *md;
> +	unsigned long efi_desc_size;
> +
> +	efi_map_start = __va(ia64_boot_param->efi_memmap);
> +	efi_map_end   = efi_map_start + ia64_boot_param->efi_memmap_size;
> +	efi_desc_size = ia64_boot_param->efi_memdesc_size;
> +
> +	for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) {
> +		md = p;
> +		if (base < md->phys_addr || base + size > efi_md_end(md))
> +			continue;
> +		if (is_memory_available(md))
> +			return 1;
> +		printk(KERN_WARNING "Kdump: crashkernel region 0x%lx-0x%lx is "
> +		       "inside the unusable efi region 0x%lx-0x%lx\n", base,
> +		       base + size - 1, md->phys_addr, efi_md_end(md) - 1);
> +		return 0;
> +	}
> +
> +	printk(KERN_WARNING "Kdump: crashkernel region 0x%lx-0x%lx does not "
> +	       "fit in any efi region\n", base, base + size - 1);
> +	return 0;
> +}
> +
> +
> +/* find a block of memory aligned to 64M exclude reserved regions
> +   rsvd_regions are sorted
> + */
> +static int __init
> +kdump_region_verify_rsvd_region (unsigned long base, unsigned long size,
> +	       			 struct rsvd_region *rsvd_regions, int n)
> +{
> +	int i;
> +
> +	for (i = 0; i < n; i++) {
> +		if (__pa(rsvd_regions[i].start) < base ||
> +		    __pa(rsvd_regions[i].end) >= base + size - 1)
> +			continue;
> +		printk(KERN_WARNING "Kdump: crashkernel region 0x%lx-0x%lx "
> +		       "clashes with reserved region 0x%lx-0x%lx\n", base,
> +		       base + size - 1, __pa(rsvd_regions[i].start),
> +		       __pa(rsvd_regions[i].end));
> +		return 0;
> +	}
> +	return 0;
> +}
> +
> +
> +/* find a block of memory aligned to 64M exclude reserved regions
> +   rsvd_regions are sorted
> + */
> +int __init
> +kdump_region_verify (unsigned long base, unsigned long size,
> +		     struct rsvd_region *rsvd_regions, int n)
> +{
> +	/* This isn't considered to be a failure condition,
> +	 * but it isn't desireable either, so log it */
> +	if (ALIGN(base, CRASHDUMP_ALIGNMENT) != base)
> +		printk(KERN_WARNING "Kdump: warning: crashkernel region "
> +		       "0x%lx-0x%lx is not aligned to 0x%x\n",
> +		       base, base + size - 1, CRASHDUMP_ALIGNMENT);
> +
> +	if (!kdump_region_verify_efi(base, size))
> +		return 0;
> +
> +	if (!kdump_region_verify_rsvd_region(base, size, rsvd_regions, n))
> +		return 0;
> +
> +	printk(KERN_INFO "Kdump: crashkernel region verified\n");
> +		return 1;
> +	return 1;
> +}
> +
>  /* find a block of memory aligned to 64M exclude reserved regions
>     rsvd_regions are sorted
>   */
> @@ -1159,7 +1238,6 @@
>  {
>    int i;
>    u64 start, end;
> -  u64 alignment = 1UL << _PAGE_SIZE_64M;
>    void *efi_map_start, *efi_map_end, *p;
>    efi_memory_desc_t *md;
>    u64 efi_desc_size;
> @@ -1172,13 +1250,13 @@
>  	  md = p;
>  	  if (!efi_wb(md))
>  		  continue;
> -	  start = ALIGN(md->phys_addr, alignment);
> +	  start = ALIGN(md->phys_addr, CRASHDUMP_ALIGNMENT);
>  	  end = efi_md_end(md);
>  	  for (i = 0; i < n; i++) {
>  		if (__pa(r[i].start) >= start && __pa(r[i].end) < end) {
>  			if (__pa(r[i].start) > start + size)
>  				return start;
> -			start = ALIGN(__pa(r[i].end), alignment);
> +			start = ALIGN(__pa(r[i].end), CRASHDUMP_ALIGNMENT);
>  			if (i < n-1 && __pa(r[i+1].start) < start + size)
>  				continue;
>  			else
> Index: linux-2.6/arch/ia64/kernel/setup.c
> ===================================================================
> --- linux-2.6.orig/arch/ia64/kernel/setup.c	2007-03-06
> 16:25:40.000000000 +0900
> +++ linux-2.6/arch/ia64/kernel/setup.c	2007-03-06 16:27:44.000000000
> +0900
> @@ -271,11 +271,17 @@
>  			else
>  				base = 0;
>  			if (size) {
> +				sort_regions(rsvd_region, n);
>  				if (!base) {
> -					sort_regions(rsvd_region, n);
>  					base = kdump_find_rsvd_region(size,
>  							      	rsvd_region, n);
> -					}
> +				}
> +				else {
> +					if (!kdump_region_verify(base, size,
> +								 rsvd_region,
> +								 n))
> +						base = ~0UL;
> +				}
>  				if (base != ~0UL) {
>  					rsvd_region[n].start =
>  						(unsigned long)__va(base);
> Index: linux-2.6/include/asm-ia64/kexec.h
> ===================================================================
> --- linux-2.6.orig/include/asm-ia64/kexec.h	2007-03-06
> 16:25:40.000000000 +0900
> +++ linux-2.6/include/asm-ia64/kexec.h	2007-03-06 16:26:41.000000000
> +0900
> @@ -37,6 +37,8 @@
>  extern void kexec_disable_iosapic(void);
>  extern void crash_save_this_cpu(void);
>  struct rsvd_region;
> +extern int kdump_region_verify(unsigned long base,
> +		unsigned long size, struct rsvd_region *rsvd_regions, int n);
>  extern unsigned long kdump_find_rsvd_region(unsigned long size,
>  		struct rsvd_region *rsvd_regions, int n);
>  extern void kdump_cpu_freeze(struct unw_frame_info *info, void *arg);
> 
> --
> 
> --
> Horms
>   H: http://www.vergenet.net/~horms/
>   W: http://www.valinux.co.jp/en/
> 
> -

Hi Horms,
	I feel this is over-designed.
     I think to specify crash kernel base address in command line is only useful for debug, on platform like SN this feature is totally unusable.At the most of time, user should let kernel to decide where to reserve crashdump region.
    If a user wants to put crash kernel in command line, he should know what he is doing.

Zou Nanhai

> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel]     [Sparc Linux]     [DCCP]     [Linux ARM]     [Yosemite News]     [Linux SCSI]     [Linux x86_64]     [Linux for Ham Radio]

  Powered by Linux