Re: [PATCH v2 3/6] kexec_file: Allow skipping checksum calculation for some segments.

[Dave Young <dyoung@redhat.com>]

On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> Adds checksum argument to kexec_add_buffer specifying whether the given
> segment should be part of the checksum calculation.
> 

Since it is used with add buffer, could it be added to kbuf as a new
field?

Like kbuf.no_checksum, default value is 0 that means checksum is needed
if it is 1 then no need a checksum.

> The next patch will add a way to update segments after a kimage is loaded.
> Segments that will be updated in this way should not be checksummed,
> otherwise they will cause the purgatory checksum verification to fail
> when the machine is rebooted.
> 
> As a bonus, we don't need to special-case the purgatory segment anymore
> to avoid checksumming it.
> 
> Adjust call sites for the new argument.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/kexec_elf_64.c |  6 +++---
>  arch/x86/kernel/crash.c            |  4 ++--
>  arch/x86/kernel/kexec-bzimage64.c  |  6 +++---
>  include/linux/kexec.h              | 10 +++++++---
>  kernel/kexec_file.c                | 23 ++++++++++++-----------
>  5 files changed, 27 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c
> index 22afc7b5ee73..4c528c81b076 100644
> --- a/arch/powerpc/kernel/kexec_elf_64.c
> +++ b/arch/powerpc/kernel/kexec_elf_64.c
> @@ -128,7 +128,7 @@ static int elf_exec_load(struct kimage *image, struct elfhdr *ehdr,
>  		kbuf.memsz = phdr->p_memsz;
>  		kbuf.buf_align = phdr->p_align;
>  		kbuf.buf_min = phdr->p_paddr + base;
> -		ret = kexec_add_buffer(&kbuf);
> +		ret = kexec_add_buffer(&kbuf, true);
>  		if (ret)
>  			goto out;
>  		load_addr = kbuf.mem;
> @@ -188,7 +188,7 @@ void *elf64_load(struct kimage *image, char *kernel_buf,
>  		kbuf.bufsz = kbuf.memsz = initrd_len;
>  		kbuf.buf_align = PAGE_SIZE;
>  		kbuf.top_down = false;
> -		ret = kexec_add_buffer(&kbuf);
> +		ret = kexec_add_buffer(&kbuf, true);
>  		if (ret)
>  			goto out;
>  		initrd_load_addr = kbuf.mem;
> @@ -245,7 +245,7 @@ void *elf64_load(struct kimage *image, char *kernel_buf,
>  	kbuf.bufsz = kbuf.memsz = fdt_size;
>  	kbuf.buf_align = PAGE_SIZE;
>  	kbuf.top_down = true;
> -	ret = kexec_add_buffer(&kbuf);
> +	ret = kexec_add_buffer(&kbuf, true);
>  	if (ret)
>  		goto out;
>  	fdt_load_addr = kbuf.mem;
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 38a1cdf6aa05..634ab16377b1 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -642,7 +642,7 @@ int crash_load_segments(struct kimage *image)
>  		 * copied in purgatory after crash. Just add a zero filled
>  		 * segment for now to make sure checksum logic works fine.
>  		 */
> -		ret = kexec_add_buffer(&kbuf);
> +		ret = kexec_add_buffer(&kbuf, true);
>  		if (ret)
>  			return ret;
>  		image->arch.backup_load_addr = kbuf.mem;
> @@ -661,7 +661,7 @@ int crash_load_segments(struct kimage *image)
>  
>  	kbuf.memsz = kbuf.bufsz;
>  	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> -	ret = kexec_add_buffer(&kbuf);
> +	ret = kexec_add_buffer(&kbuf, true);
>  	if (ret) {
>  		vfree((void *)image->arch.elf_headers);
>  		return ret;
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 4b3a75329fb6..a46e3fbb0639 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -422,7 +422,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
>  	kbuf.memsz = kbuf.bufsz;
>  	kbuf.buf_align = 16;
>  	kbuf.buf_min = MIN_BOOTPARAM_ADDR;
> -	ret = kexec_add_buffer(&kbuf);
> +	ret = kexec_add_buffer(&kbuf, true);
>  	if (ret)
>  		goto out_free_params;
>  	bootparam_load_addr = kbuf.mem;
> @@ -435,7 +435,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
>  	kbuf.memsz = PAGE_ALIGN(header->init_size);
>  	kbuf.buf_align = header->kernel_alignment;
>  	kbuf.buf_min = MIN_KERNEL_LOAD_ADDR;
> -	ret = kexec_add_buffer(&kbuf);
> +	ret = kexec_add_buffer(&kbuf, true);
>  	if (ret)
>  		goto out_free_params;
>  	kernel_load_addr = kbuf.mem;
> @@ -449,7 +449,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
>  		kbuf.bufsz = kbuf.memsz = initrd_len;
>  		kbuf.buf_align = PAGE_SIZE;
>  		kbuf.buf_min = MIN_INITRD_LOAD_ADDR;
> -		ret = kexec_add_buffer(&kbuf);
> +		ret = kexec_add_buffer(&kbuf, true);
>  		if (ret)
>  			goto out_free_params;
>  		initrd_load_addr = kbuf.mem;
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 4559a1a01b0a..37eea32fdff1 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -100,6 +100,9 @@ struct kexec_segment {
>  	size_t bufsz;
>  	unsigned long mem;
>  	size_t memsz;
> +
> +	/* Whether this segment is part of the checksum calculation. */
> +	bool do_checksum;
>  };
>  
>  #ifdef CONFIG_COMPAT
> @@ -175,7 +178,7 @@ struct kexec_buf {
>  
>  int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
>  			       int (*func)(u64, u64, void *));
> -extern int kexec_add_buffer(struct kexec_buf *kbuf);
> +extern int kexec_add_buffer(struct kexec_buf *kbuf, bool checksum);
>  int kexec_locate_mem_hole(struct kexec_buf *kbuf);
>  int __weak arch_kexec_verify_buffer(enum kexec_file_type type, const void *buf,
>  				    unsigned long size);
> @@ -393,7 +396,7 @@ bool __weak kexec_can_hand_over_buffer(void);
>  int __weak arch_kexec_add_handover_buffer(struct kimage *image,
>  					  unsigned long load_addr,
>  					  unsigned long size);
> -int kexec_add_handover_buffer(struct kexec_buf *kbuf);
> +int kexec_add_handover_buffer(struct kexec_buf *kbuf, bool checksum);
>  int __weak kexec_get_handover_buffer(void **addr, unsigned long *size);
>  int __weak kexec_free_handover_buffer(void);
>  #else
> @@ -402,7 +405,8 @@ static inline bool kexec_can_hand_over_buffer(void)
>  	return false;
>  }
>  
> -static inline int kexec_add_handover_buffer(struct kexec_buf *kbuf)
> +static inline int kexec_add_handover_buffer(struct kexec_buf *kbuf,
> +					    bool checksum)
>  {
>  	return -ENOTSUPP;
>  }
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index c8418d62e2fc..aed51175915f 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -161,6 +161,7 @@ int __weak arch_kexec_add_handover_buffer(struct kimage *image,
>  /**
>   * kexec_add_handover_buffer - add buffer to be used by the next kernel
>   * @kbuf:	Buffer contents and memory parameters.
> + * @checksum:	Should the segment checksum be verified by the purgatory?
>   *
>   * This function assumes that kexec_mutex is held.
>   * On successful return, @kbuf->mem will have the physical address of
> @@ -168,14 +169,14 @@ int __weak arch_kexec_add_handover_buffer(struct kimage *image,
>   *
>   * Return: 0 on success, negative errno on error.
>   */
> -int kexec_add_handover_buffer(struct kexec_buf *kbuf)
> +int kexec_add_handover_buffer(struct kexec_buf *kbuf, bool checksum)
>  {
>  	int ret;
>  
>  	if (!kexec_can_hand_over_buffer())
>  		return -ENOTSUPP;
>  
> -	ret = kexec_add_buffer(kbuf);
> +	ret = kexec_add_buffer(kbuf, checksum);
>  	if (ret)
>  		return ret;
>  
> @@ -611,6 +612,7 @@ int kexec_locate_mem_hole(struct kexec_buf *kbuf)
>  /**
>   * kexec_add_buffer - place a buffer in a kexec segment
>   * @kbuf:	Buffer contents and memory parameters.
> + * @checksum:	Should the segment checksum be verified by the purgatory?
>   *
>   * This function assumes that kexec_mutex is held.
>   * On successful return, @kbuf->mem will have the physical address of
> @@ -618,7 +620,7 @@ int kexec_locate_mem_hole(struct kexec_buf *kbuf)
>   *
>   * Return: 0 on success, negative errno on error.
>   */
> -int kexec_add_buffer(struct kexec_buf *kbuf)
> +int kexec_add_buffer(struct kexec_buf *kbuf, bool checksum)
>  {
>  
>  	struct kexec_segment *ksegment;
> @@ -658,6 +660,7 @@ int kexec_add_buffer(struct kexec_buf *kbuf)
>  	ksegment->bufsz = kbuf->bufsz;
>  	ksegment->mem = kbuf->mem;
>  	ksegment->memsz = kbuf->memsz;
> +	ksegment->do_checksum = checksum;
>  	kbuf->image->nr_segments++;
>  	return 0;
>  }
> @@ -672,7 +675,6 @@ static int kexec_calculate_store_digests(struct kimage *image)
>  	char *digest;
>  	void *zero_buf;
>  	struct kexec_sha_region *sha_regions;
> -	struct purgatory_info *pi = &image->purgatory_info;
>  
>  	zero_buf = __va(page_to_pfn(ZERO_PAGE(0)) << PAGE_SHIFT);
>  	zero_buf_sz = PAGE_SIZE;
> @@ -712,11 +714,7 @@ static int kexec_calculate_store_digests(struct kimage *image)
>  		struct kexec_segment *ksegment;
>  
>  		ksegment = &image->segment[i];
> -		/*
> -		 * Skip purgatory as it will be modified once we put digest
> -		 * info in purgatory.
> -		 */
> -		if (ksegment->kbuf == pi->purgatory_buf)
> +		if (!ksegment->do_checksum)
>  			continue;
>  
>  		ret = crypto_shash_update(desc, ksegment->kbuf,
> @@ -893,8 +891,11 @@ static int __kexec_load_purgatory(struct kimage *image, unsigned long min,
>  	if (kbuf.buf_align < bss_align)
>  		kbuf.buf_align = bss_align;
>  
> -	/* Add buffer to segment list */
> -	ret = kexec_add_buffer(&kbuf);
> +	/*
> +	 * Add buffer to segment list. Don't checksum the segment as
> +	 * it will be modified once we put digest info in purgatory.
> +	 */
> +	ret = kexec_add_buffer(&kbuf, false);
>  	if (ret)
>  		goto out;
>  	pi->purgatory_load_addr = kbuf.mem;
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec