Thread (22 messages) 22 messages, 3 authors, 2016-08-22

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

From: Thiago Jung Bauermann <hidden>
Date: 2016-08-22 22:13:48
Also in: kexec, lkml
Subsystem: kexec, the rest · Maintainers: Andrew Morton, Baoquan He, Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Linus Torvalds

Am Montag, 22 August 2016, 11:36:43 schrieb Dave Young:
On 08/22/16 at 12:25am, Thiago Jung Bauermann wrote:
quoted
Am Montag, 22 August 2016, 11:17:45 schrieb Dave Young:
quoted
On 08/18/16 at 06:09pm, Thiago Jung Bauermann wrote:
quoted
Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
quoted
On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
quoted
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?
I was on the fence about adding it as a new argument to
kexec_add_buffer
or as a new field to struct kexec_buf. Both alternatives make sense
to
me. I implemented your suggestion in the patch below, what do you
think?>
quoted
Like kbuf.no_checksum, default value is 0 that means checksum is
needed
if it is 1 then no need a checksum.
It's an interesting idea and I implemented it that way, though in
practice all current users of struct kexec_buf put it on the stack
so
the field needs to be initialized explicitly.
No need to set it as false because it will be initialized to 0 by
default?
As far as I know, variables on the stack are not initialized. Only
global
and static variables are.
But designated initializers will do it.
Here is the new version, relying on the default value for
kexec_buf.skip_checksum. A nice side effect is that I don't have to
cc the x86 maintainers anymore.

What do you think?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


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

Add skip_checksum member to struct kexec_buf to specify whether the
corresponding segment should be part of the checksum calculation.

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.

Places currently using struct kexec_buf will get false as the default
value for skip_checksum since they all use designated initializers.
Therefore, there will not be any behavior change with this patch and
their buffers will continue being checksummed.

Signed-off-by: Thiago Jung Bauermann <redacted>
---
 include/linux/kexec.h | 23 ++++++++++++++---------
 kernel/kexec_file.c   | 15 +++++++--------
 2 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 4559a1a01b0a..e5b3d99cbe50 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 ignored in the checksum calculation. */
+	bool skip_checksum;
 };
 
 #ifdef CONFIG_COMPAT
@@ -151,15 +154,16 @@ struct kexec_file_ops {
 
 /**
  * struct kexec_buf - parameters for finding a place for a buffer in memory
- * @image:	kexec image in which memory to search.
- * @buffer:	Contents which will be copied to the allocated memory.
- * @bufsz:	Size of @buffer.
- * @mem:	On return will have address of the buffer in memory.
- * @memsz:	Size for the buffer in memory.
- * @buf_align:	Minimum alignment needed.
- * @buf_min:	The buffer can't be placed below this address.
- * @buf_max:	The buffer can't be placed above this address.
- * @top_down:	Allocate from top of memory.
+ * @image:		kexec image in which memory to search.
+ * @buffer:		Contents which will be copied to the allocated memory.
+ * @bufsz:		Size of @buffer.
+ * @mem:		On return will have address of the buffer in memory.
+ * @memsz:		Size for the buffer in memory.
+ * @buf_align:		Minimum alignment needed.
+ * @buf_min:		The buffer can't be placed below this address.
+ * @buf_max:		The buffer can't be placed above this address.
+ * @top_down:		Allocate from top of memory.
+ * @skip_checksum:	Don't verify checksum for this buffer in purgatory.
  */
 struct kexec_buf {
 	struct kimage *image;
@@ -171,6 +175,7 @@ struct kexec_buf {
 	unsigned long buf_min;
 	unsigned long buf_max;
 	bool top_down;
+	bool skip_checksum;
 };
 
 int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index c8418d62e2fc..f6e9958bf578 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -658,6 +658,7 @@ int kexec_add_buffer(struct kexec_buf *kbuf)
 	ksegment->bufsz = kbuf->bufsz;
 	ksegment->mem = kbuf->mem;
 	ksegment->memsz = kbuf->memsz;
+	ksegment->skip_checksum = kbuf->skip_checksum;
 	kbuf->image->nr_segments++;
 	return 0;
 }
@@ -672,7 +673,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 +712,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->skip_checksum)
 			continue;
 
 		ret = crypto_shash_update(desc, ksegment->kbuf,
@@ -788,7 +784,7 @@ static int __kexec_load_purgatory(struct kimage *image, unsigned long min,
 	Elf_Shdr *sechdrs = NULL;
 	struct kexec_buf kbuf = { .image = image, .bufsz = 0, .buf_align = 1,
 				  .buf_min = min, .buf_max = max,
-				  .top_down = top_down };
+				  .top_down = top_down, .skip_checksum = true };
 
 	/*
 	 * sechdrs_c points to section headers in purgatory and are read
@@ -893,7 +889,10 @@ 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 */
+	/*
+	 * 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);
 	if (ret)
 		goto out;
-- 
1.9.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help