[PATCH] kexec_file: skip checksum verification when relocations aren't needed

[PATCH] kexec_file: skip checksum verification when relocations aren't needed

[Michal Clapinski]

Checksum verification is needed
1. for crash kernels. In a crash, we can't be sure the kernel is
   intact.
2. if we're worried about relocating the kernel into a region used by
   some DMA that wasn't properly cancelled.

If we used CMA to allocate segments then
1. we're not working with a crash kernel.
2. relocations are not going to happen.

Therefore, we can safely disable checksum verification.

Instead of adding a new variable to purgatory, just skip adding regions
and save the default value of SHA256 hash.

Saves ~250ms on my 4.0 GHz CPU.

Signed-off-by: Michal Clapinski <mclapinski@google.com>
---
 kernel/kexec_file.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 2bfbb2d144e6..2dc8b0435fe6 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -808,6 +808,7 @@ static int kexec_calculate_store_digests(struct kimage *image)
 	void *zero_buf;
 	struct kexec_sha_region *sha_regions;
 	struct purgatory_info *pi = &image->purgatory_info;
+	bool can_skip_checksum = true;
 
 	if (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY))
 		return 0;
@@ -822,6 +823,23 @@ static int kexec_calculate_store_digests(struct kimage *image)
 
 	sha256_init(&sctx);
 
+	/*
+	 * If all segments were loaded into contiguous memory, there will be no
+	 * relocations. In that case there is no risk of memory corruption by
+	 * uncancelled DMA and we can skip checksum calculation.
+	 */
+	for (i = 0; i < image->nr_segments; i++) {
+		if (!image->segment_cma[i]) {
+			can_skip_checksum = false;
+			break;
+		}
+	}
+
+	if (can_skip_checksum) {
+		pr_info("disabling checksum verification in purgatory\n");
+		goto skip_checksum;
+	}
+
 	for (j = i = 0; i < image->nr_segments; i++) {
 		struct kexec_segment *ksegment;
 
@@ -867,6 +885,7 @@ static int kexec_calculate_store_digests(struct kimage *image)
 		j++;
 	}
 
+skip_checksum:
 	sha256_final(&sctx, digest);
 
 	ret = kexec_purgatory_get_set_symbol(image, "purgatory_sha_regions",
-- 
2.54.0.929.g9b7fa37559-goog

Re: [PATCH] kexec_file: skip checksum verification when relocations aren't needed

[Pasha Tatashin]

Nit: The crash kernel also does not perform relocations, yet a checksum 
is still required. The subject should be something like:
kexec_file: skip purgatory checksum if all segments are CMA allocated


On 06-01 21:11, Michal Clapinski wrote:
> Checksum verification is needed
> 1. for crash kernels. In a crash, we can't be sure the kernel is
>    intact.
> 2. if we're worried about relocating the kernel into a region used by
>    some DMA that wasn't properly cancelled.

Nit: Please add a little background information about CMA segments being 
recently added, as well as the necessity for a fast reboot due to the 
live update use case.

> 
> If we used CMA to allocate segments then
> 1. we're not working with a crash kernel.
> 2. relocations are not going to happen.
> 
> Therefore, we can safely disable checksum verification.
> 
> Instead of adding a new variable to purgatory, just skip adding regions
> and save the default value of SHA256 hash.
> 
> Saves ~250ms on my 4.0 GHz CPU.
> 
> Signed-off-by: Michal Clapinski <mclapinski@google.com>
> ---
>  kernel/kexec_file.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 2bfbb2d144e6..2dc8b0435fe6 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -808,6 +808,7 @@ static int kexec_calculate_store_digests(struct kimage *image)
>  	void *zero_buf;
>  	struct kexec_sha_region *sha_regions;
>  	struct purgatory_info *pi = &image->purgatory_info;
> +	bool can_skip_checksum = true;
>  
>  	if (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY))
>  		return 0;
> @@ -822,6 +823,23 @@ static int kexec_calculate_store_digests(struct kimage *image)
>  
>  	sha256_init(&sctx);
>  
> +	/*
> +	 * If all segments were loaded into contiguous memory, there will be no
> +	 * relocations. In that case there is no risk of memory corruption by
> +	 * uncancelled DMA and we can skip checksum calculation.
> +	 */
> +	for (i = 0; i < image->nr_segments; i++) {
> +		if (!image->segment_cma[i]) {
> +			can_skip_checksum = false;
> +			break;
> +		}
> +	}
> +
> +	if (can_skip_checksum) {
> +		pr_info("disabling checksum verification in purgatory\n");
> +		goto skip_checksum;
> +	}
> +
>  	for (j = i = 0; i < image->nr_segments; i++) {
>  		struct kexec_segment *ksegment;
>  
> @@ -867,6 +885,7 @@ static int kexec_calculate_store_digests(struct kimage *image)
>  		j++;
>  	}
>  
> +skip_checksum:
>  	sha256_final(&sctx, digest);

With the few nits:

Reviewed-by: Pasha Tatashin <pasha.tatashin@soleen.com> 

>  
>  	ret = kexec_purgatory_get_set_symbol(image, "purgatory_sha_regions",
> -- 
> 2.54.0.929.g9b7fa37559-goog
> 

Re: [PATCH] kexec_file: skip checksum verification when relocations aren't needed

[Baoquan He]

On 06/01/26 at 09:11pm, Michal Clapinski wrote:
...snip... 
> +	/*
> +	 * If all segments were loaded into contiguous memory, there will be no
> +	 * relocations. In that case there is no risk of memory corruption by
> +	 * uncancelled DMA and we can skip checksum calculation.
> +	 */
> +	for (i = 0; i < image->nr_segments; i++) {
> +		if (!image->segment_cma[i]) {
> +			can_skip_checksum = false;
> +			break;
> +		}
> +	}
> +
> +	if (can_skip_checksum) {
> +		pr_info("disabling checksum verification in purgatory\n");

Use pr_debug() or kexec_dprintk() instead because this is unnecessary to
note users if it's a normal action?

Except of this, the overral looks good to me.

Acked-by: Baoquan He <baoquan.he@linux.dev>

> +		goto skip_checksum;
> +	}
> +
>  	for (j = i = 0; i < image->nr_segments; i++) {
>  		struct kexec_segment *ksegment;
>  
> @@ -867,6 +885,7 @@ static int kexec_calculate_store_digests(struct kimage *image)
>  		j++;
>  	}
>  
> +skip_checksum:
>  	sha256_final(&sctx, digest);
>  
>  	ret = kexec_purgatory_get_set_symbol(image, "purgatory_sha_regions",
> -- 
> 2.54.0.929.g9b7fa37559-goog
>