Re: [PATCH v4 1/2] kexec: Introduce a protection mechanism for the crashkernel reserved memory

[Minfei Huang]

On 01/11/16 at 10:55am, Xunlei Pang wrote:
> For the cases that some kernel (module) path stamps the crash
> reserved memory(already mapped by the kernel) where has been
> loaded the second kernel data, the kdump kernel will probably
> fail to boot when panic happens (or even not happens) leaving
> the culprit at large, this is unacceptable.
> 
> The patch introduces a mechanism for detecting such cases:
> 1) After each crash kexec loading, it simply marks the reserved
> memory regions readonly since we no longer access it after that.
> When someone stamps the region, the first kernel will panic and
> trigger the kdump. The weak arch_kexec_protect_crashkres() is
> introduced to do the actual protection.
> 
> 2) To allow multiple loading, once 1) was done we also need to
> remark the reserved memory to readwrite each time a system call
> related to kdump is made. The weak arch_kexec_unprotect_crashkres()
> is introduced to undo the actual protection.
> 
> The architecture can make its specific implementation by overriding
> arch_kexec_protect_crashkres() and arch_kexec_unprotect_crashkres().
> 
> Signed-off-by: Xunlei Pang <xlpang@xxxxxxxxxx>
> ---
> v3->v4:
> No changes.
> 
>  include/linux/kexec.h | 2 ++
>  kernel/kexec.c        | 9 ++++++++-
>  kernel/kexec_core.c   | 6 ++++++
>  kernel/kexec_file.c   | 8 +++++++-
>  4 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 7b68d27..ebd6950 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -329,6 +329,8 @@ int __weak arch_kexec_apply_relocations_add(const Elf_Ehdr *ehdr,
>  					Elf_Shdr *sechdrs, unsigned int relsec);
>  int __weak arch_kexec_apply_relocations(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs,
>  					unsigned int relsec);
> +void arch_kexec_protect_crashkres(void);
> +void arch_kexec_unprotect_crashkres(void);
>  
>  #else /* !CONFIG_KEXEC_CORE */
>  struct pt_regs;
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index d873b64..3680f9c 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -167,8 +167,12 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments,
>  		return -EBUSY;
>  
>  	dest_image = &kexec_image;
> -	if (flags & KEXEC_ON_CRASH)
> +	if (flags & KEXEC_ON_CRASH) {
>  		dest_image = &kexec_crash_image;
> +		if (kexec_crash_image)
> +			arch_kexec_unprotect_crashkres();
> +	}
> +
>  	if (nr_segments > 0) {
>  		unsigned long i;
>  
> @@ -211,6 +215,9 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments,
>  	image = xchg(dest_image, image);
>  
>  out:
> +	if ((flags & KEXEC_ON_CRASH) && kexec_crash_image)
> +		arch_kexec_protect_crashkres();
> +
>  	mutex_unlock(&kexec_mutex);
>  	kimage_free(image);
>  
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index c823f30..de4dd80 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -1560,3 +1560,9 @@ void __weak crash_map_reserved_pages(void)
>  
>  void __weak crash_unmap_reserved_pages(void)
>  {}
> +
> +void __weak arch_kexec_protect_crashkres(void)
> +{}
> +
> +void __weak arch_kexec_unprotect_crashkres(void)
> +{}
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index b70ada0..5892d6a 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -327,8 +327,11 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>  		return -EBUSY;
>  
>  	dest_image = &kexec_image;
> -	if (flags & KEXEC_FILE_ON_CRASH)
> +	if (flags & KEXEC_FILE_ON_CRASH) {
>  		dest_image = &kexec_crash_image;
> +		if (kexec_crash_image)
> +			arch_kexec_unprotect_crashkres();
> +	}
>  
>  	if (flags & KEXEC_FILE_UNLOAD)
>  		goto exchange;
> @@ -377,6 +380,9 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
>  exchange:
>  	image = xchg(dest_image, image);
>  out:
> +	if ((flags & KEXEC_FILE_ON_CRASH) && kexec_crash_image)
> +		arch_kexec_protect_crashkres();
> +
>  	mutex_unlock(&kexec_mutex);
>  	kimage_free(image);
>  	return ret;
> -- 
> 2.5.0
> 

Reviewed-by: Minfei Huang <mhuang@xxxxxxxxxx>

Thanks
Minfei