[PATCH v2 0/6] kexec_file: Add buffer hand-over for the next kernel

[PATCH v2 0/6] kexec_file: Add buffer hand-over for the next kernel

[Thiago Jung Bauermann]

Hello,

This patch series implements a mechanism which allows the kernel to pass
on a buffer to the kernel that will be kexec'd. This buffer is passed
as a segment which is added to the kimage when it is being prepared
by kexec_file_load.

How the second kernel is informed of this buffer is architecture-specific.
On powerpc, this is done via the device tree, by checking
the properties /chosen/linux,kexec-handover-buffer-start and
/chosen/linux,kexec-handover-buffer-end, which is analogous to how the
kernel finds the initrd.

This is needed because the Integrity Measurement Architecture subsystem
needs to preserve its measurement list accross the kexec reboot. The
following patch series for the IMA subsystem uses this feature for that
purpose:

https://lists.infradead.org/pipermail/kexec/2016-August/016745.html

This is so that IMA can implement trusted boot support on the OpenPower
platform, because on such systems an intermediary Linux instance running
as part of the firmware is used to boot the target operating system via
kexec. Using this mechanism, IMA on this intermediary instance can
hand over to the target OS the measurements of the components that were
used to boot it.

Because there could be additional measurement events between the
kexec_file_load call and the actual reboot, IMA needs a way to update the
buffer with those additional events before rebooting. One can minimize
the interval between the kexec_file_load and the reboot syscalls, but as
small as it can be, there is always the possibility that the measurement
list will be out of date at the time of reboot.

To address this issue, this patch series also introduces
kexec_update_segment, which allows a reboot notifier to change the
contents of the image segment during the reboot process.

Patch 5 makes kimage_load_normal_segment and kexec_update_segment share
code. It's not much code that they can share though, so I'm not sure if
the result is actually better.

The last patch is not intended to be merged, it just demonstrates how
this feature can be used.

This series applies on top of v5 of the "kexec_file_load implementation
for PowerPC" patch series (which applies on top of v4.8-rc1):

https://lists.infradead.org/pipermail/kexec/2016-August/016843.html

Changes for v2:
- Rebased on v5 of kexec_file_load implementation for PowerPC patch series.
- Patch "kexec_file: Add buffer hand-over support for the next kernel"
  - Changed kexec_add_handover_buffer to receive a struct kexec_buf, as
    suggested by Dave Young.
- Patch "powerpc: kexec_file: Add buffer hand-over support for the next kernel"
  - Moved setup_handover_buffer from kexec_elf_64.c to machine_kexec_64.c.
  - Call setup_handover_buffer from setup_new_fdt instead of elf64_load.
  - Changed kexec_get_handover_buffer to read from the expanded device tree
    instead of the flattened device tree.
- Patch "kexec_file: Add mechanism to update kexec segments.":
  - Removed unnecessary "#include <linux/highmem.h>" in kexec_file.c.
  - Round up memsz argument to PAGE_SIZE.
  - Check if kexec_image is NULL in kexec_update_segment.
- Patch "IMA: Demonstration code for kexec buffer passing."
  - Avoid registering reboot notifier again if kexec_file_load is called
    more than once.


Thiago Jung Bauermann (6):
  kexec_file: Add buffer hand-over support for the next kernel
  powerpc: kexec_file: Add buffer hand-over support for the next kernel
  kexec_file: Allow skipping checksum calculation for some segments.
  kexec_file: Add mechanism to update kexec segments.
  kexec: Share logic to copy segment page contents.
  IMA: Demonstration code for kexec buffer passing.

 arch/powerpc/include/asm/kexec.h       |  12 +-
 arch/powerpc/kernel/kexec_elf_64.c     |   8 +-
 arch/powerpc/kernel/machine_kexec_64.c | 114 ++++++++++++++++-
 arch/x86/kernel/crash.c                |   4 +-
 arch/x86/kernel/kexec-bzimage64.c      |   6 +-
 include/linux/ima.h                    |  11 ++
 include/linux/kexec.h                  |  37 +++++-
 kernel/kexec_core.c                    | 216 ++++++++++++++++++++++++++-------
 kernel/kexec_file.c                    |  91 ++++++++++++--
 security/integrity/ima/ima.h           |   5 +
 security/integrity/ima/ima_init.c      |  26 ++++
 security/integrity/ima/ima_template.c  |  85 +++++++++++++
 12 files changed, 546 insertions(+), 69 deletions(-)

-- 
1.9.1

[PATCH v2 0/6] kexec_file: Add buffer hand-over for the next kernel

[Thiago Jung Bauermann]

Hello,

This patch series implements a mechanism which allows the kernel to pass
on a buffer to the kernel that will be kexec'd. This buffer is passed
as a segment which is added to the kimage when it is being prepared
by kexec_file_load.

How the second kernel is informed of this buffer is architecture-specific.
On powerpc, this is done via the device tree, by checking
the properties /chosen/linux,kexec-handover-buffer-start and
/chosen/linux,kexec-handover-buffer-end, which is analogous to how the
kernel finds the initrd.

This is needed because the Integrity Measurement Architecture subsystem
needs to preserve its measurement list accross the kexec reboot. The
following patch series for the IMA subsystem uses this feature for that
purpose:

https://lists.infradead.org/pipermail/kexec/2016-August/016745.html

This is so that IMA can implement trusted boot support on the OpenPower
platform, because on such systems an intermediary Linux instance running
as part of the firmware is used to boot the target operating system via
kexec. Using this mechanism, IMA on this intermediary instance can
hand over to the target OS the measurements of the components that were
used to boot it.

Because there could be additional measurement events between the
kexec_file_load call and the actual reboot, IMA needs a way to update the
buffer with those additional events before rebooting. One can minimize
the interval between the kexec_file_load and the reboot syscalls, but as
small as it can be, there is always the possibility that the measurement
list will be out of date at the time of reboot.

To address this issue, this patch series also introduces
kexec_update_segment, which allows a reboot notifier to change the
contents of the image segment during the reboot process.

Patch 5 makes kimage_load_normal_segment and kexec_update_segment share
code. It's not much code that they can share though, so I'm not sure if
the result is actually better.

The last patch is not intended to be merged, it just demonstrates how
this feature can be used.

This series applies on top of v5 of the "kexec_file_load implementation
for PowerPC" patch series (which applies on top of v4.8-rc1):

https://lists.infradead.org/pipermail/kexec/2016-August/016843.html

Changes for v2:
- Rebased on v5 of kexec_file_load implementation for PowerPC patch series.
- Patch "kexec_file: Add buffer hand-over support for the next kernel"
  - Changed kexec_add_handover_buffer to receive a struct kexec_buf, as
    suggested by Dave Young.
- Patch "powerpc: kexec_file: Add buffer hand-over support for the next kernel"
  - Moved setup_handover_buffer from kexec_elf_64.c to machine_kexec_64.c.
  - Call setup_handover_buffer from setup_new_fdt instead of elf64_load.
  - Changed kexec_get_handover_buffer to read from the expanded device tree
    instead of the flattened device tree.
- Patch "kexec_file: Add mechanism to update kexec segments.":
  - Removed unnecessary "#include <linux/highmem.h>" in kexec_file.c.
  - Round up memsz argument to PAGE_SIZE.
  - Check if kexec_image is NULL in kexec_update_segment.
- Patch "IMA: Demonstration code for kexec buffer passing."
  - Avoid registering reboot notifier again if kexec_file_load is called
    more than once.


Thiago Jung Bauermann (6):
  kexec_file: Add buffer hand-over support for the next kernel
  powerpc: kexec_file: Add buffer hand-over support for the next kernel
  kexec_file: Allow skipping checksum calculation for some segments.
  kexec_file: Add mechanism to update kexec segments.
  kexec: Share logic to copy segment page contents.
  IMA: Demonstration code for kexec buffer passing.

 arch/powerpc/include/asm/kexec.h       |  12 +-
 arch/powerpc/kernel/kexec_elf_64.c     |   8 +-
 arch/powerpc/kernel/machine_kexec_64.c | 114 ++++++++++++++++-
 arch/x86/kernel/crash.c                |   4 +-
 arch/x86/kernel/kexec-bzimage64.c      |   6 +-
 include/linux/ima.h                    |  11 ++
 include/linux/kexec.h                  |  37 +++++-
 kernel/kexec_core.c                    | 216 ++++++++++++++++++++++++++-------
 kernel/kexec_file.c                    |  91 ++++++++++++--
 security/integrity/ima/ima.h           |   5 +
 security/integrity/ima/ima_init.c      |  26 ++++
 security/integrity/ima/ima_template.c  |  85 +++++++++++++
 12 files changed, 546 insertions(+), 69 deletions(-)

-- 
1.9.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 1/6] kexec_file: Add buffer hand-over support for the next kernel

[Thiago Jung Bauermann]

The buffer hand-over mechanism allows the currently running kernel to pass
data to kernel that will be kexec'd via a kexec segment. The second kernel
can check whether the previous kernel sent data and retrieve it.

This is the architecture-independent part of the feature.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 include/linux/kexec.h | 29 ++++++++++++++++++++++
 kernel/kexec_file.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index ceccc5856aab..4559a1a01b0a 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -388,6 +388,35 @@ static inline void *boot_phys_to_virt(unsigned long entry)
 	return phys_to_virt(boot_phys_to_phys(entry));
 }
 
+#ifdef CONFIG_KEXEC_FILE
+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 __weak kexec_get_handover_buffer(void **addr, unsigned long *size);
+int __weak kexec_free_handover_buffer(void);
+#else
+static inline bool kexec_can_hand_over_buffer(void)
+{
+	return false;
+}
+
+static inline int kexec_add_handover_buffer(struct kexec_buf *kbuf)
+{
+	return -ENOTSUPP;
+}
+
+static inline int kexec_get_handover_buffer(void **addr, unsigned long *size)
+{
+	return -ENOTSUPP;
+}
+
+static inline int kexec_free_handover_buffer(void)
+{
+	return -ENOTSUPP;
+}
+#endif /* CONFIG_KEXEC_FILE */
 #else /* !CONFIG_KEXEC_CORE */
 struct pt_regs;
 struct task_struct;
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 772cb491715e..c8418d62e2fc 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -135,6 +135,74 @@ int __weak arch_kexec_verify_buffer(enum kexec_file_type type, const void *buf,
 	return -EINVAL;
 }
 
+/**
+ * kexec_can_hand_over_buffer - can we pass data to the kexec'd kernel?
+ */
+bool __weak kexec_can_hand_over_buffer(void)
+{
+	return false;
+}
+
+/**
+ * arch_kexec_add_handover_buffer - do arch-specific steps to handover buffer
+ *
+ * Architectures should use this function to pass on the handover buffer
+ * information to the next kernel.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int __weak arch_kexec_add_handover_buffer(struct kimage *image,
+					  unsigned long load_addr,
+					  unsigned long size)
+{
+	return -ENOTSUPP;
+}
+
+/**
+ * kexec_add_handover_buffer - add buffer to be used by the next kernel
+ * @kbuf:	Buffer contents and memory parameters.
+ *
+ * This function assumes that kexec_mutex is held.
+ * On successful return, @kbuf->mem will have the physical address of
+ * the buffer in the next kernel.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int kexec_add_handover_buffer(struct kexec_buf *kbuf)
+{
+	int ret;
+
+	if (!kexec_can_hand_over_buffer())
+		return -ENOTSUPP;
+
+	ret = kexec_add_buffer(kbuf);
+	if (ret)
+		return ret;
+
+	return arch_kexec_add_handover_buffer(kbuf->image, kbuf->mem,
+					      kbuf->memsz);
+}
+
+/**
+ * kexec_get_handover_buffer - get the handover buffer from the previous kernel
+ * @addr:	On successful return, set to point to the buffer contents.
+ * @size:	On successful return, set to the buffer size.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int __weak kexec_get_handover_buffer(void **addr, unsigned long *size)
+{
+	return -ENOTSUPP;
+}
+
+/**
+ * kexec_free_handover_buffer - free memory used by the handover buffer
+ */
+int __weak kexec_free_handover_buffer(void)
+{
+	return -ENOTSUPP;
+}
+
 /*
  * In file mode list of segments is prepared by kernel. Copy relevant
  * data from user space, do error checking, prepare segment list
-- 
1.9.1

[PATCH v2 1/6] kexec_file: Add buffer hand-over support for the next kernel

[Thiago Jung Bauermann]

The buffer hand-over mechanism allows the currently running kernel to pass
data to kernel that will be kexec'd via a kexec segment. The second kernel
can check whether the previous kernel sent data and retrieve it.

This is the architecture-independent part of the feature.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 include/linux/kexec.h | 29 ++++++++++++++++++++++
 kernel/kexec_file.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index ceccc5856aab..4559a1a01b0a 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -388,6 +388,35 @@ static inline void *boot_phys_to_virt(unsigned long entry)
 	return phys_to_virt(boot_phys_to_phys(entry));
 }
 
+#ifdef CONFIG_KEXEC_FILE
+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 __weak kexec_get_handover_buffer(void **addr, unsigned long *size);
+int __weak kexec_free_handover_buffer(void);
+#else
+static inline bool kexec_can_hand_over_buffer(void)
+{
+	return false;
+}
+
+static inline int kexec_add_handover_buffer(struct kexec_buf *kbuf)
+{
+	return -ENOTSUPP;
+}
+
+static inline int kexec_get_handover_buffer(void **addr, unsigned long *size)
+{
+	return -ENOTSUPP;
+}
+
+static inline int kexec_free_handover_buffer(void)
+{
+	return -ENOTSUPP;
+}
+#endif /* CONFIG_KEXEC_FILE */
 #else /* !CONFIG_KEXEC_CORE */
 struct pt_regs;
 struct task_struct;
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 772cb491715e..c8418d62e2fc 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -135,6 +135,74 @@ int __weak arch_kexec_verify_buffer(enum kexec_file_type type, const void *buf,
 	return -EINVAL;
 }
 
+/**
+ * kexec_can_hand_over_buffer - can we pass data to the kexec'd kernel?
+ */
+bool __weak kexec_can_hand_over_buffer(void)
+{
+	return false;
+}
+
+/**
+ * arch_kexec_add_handover_buffer - do arch-specific steps to handover buffer
+ *
+ * Architectures should use this function to pass on the handover buffer
+ * information to the next kernel.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int __weak arch_kexec_add_handover_buffer(struct kimage *image,
+					  unsigned long load_addr,
+					  unsigned long size)
+{
+	return -ENOTSUPP;
+}
+
+/**
+ * kexec_add_handover_buffer - add buffer to be used by the next kernel
+ * @kbuf:	Buffer contents and memory parameters.
+ *
+ * This function assumes that kexec_mutex is held.
+ * On successful return, @kbuf->mem will have the physical address of
+ * the buffer in the next kernel.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int kexec_add_handover_buffer(struct kexec_buf *kbuf)
+{
+	int ret;
+
+	if (!kexec_can_hand_over_buffer())
+		return -ENOTSUPP;
+
+	ret = kexec_add_buffer(kbuf);
+	if (ret)
+		return ret;
+
+	return arch_kexec_add_handover_buffer(kbuf->image, kbuf->mem,
+					      kbuf->memsz);
+}
+
+/**
+ * kexec_get_handover_buffer - get the handover buffer from the previous kernel
+ * @addr:	On successful return, set to point to the buffer contents.
+ * @size:	On successful return, set to the buffer size.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int __weak kexec_get_handover_buffer(void **addr, unsigned long *size)
+{
+	return -ENOTSUPP;
+}
+
+/**
+ * kexec_free_handover_buffer - free memory used by the handover buffer
+ */
+int __weak kexec_free_handover_buffer(void)
+{
+	return -ENOTSUPP;
+}
+
 /*
  * In file mode list of segments is prepared by kernel. Copy relevant
  * data from user space, do error checking, prepare segment list
-- 
1.9.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 2/6] powerpc: kexec_file: Add buffer hand-over support for the next kernel

[Thiago Jung Bauermann]

The buffer hand-over mechanism allows the currently running kernel to pass
data to kernel that will be kexec'd via a kexec segment. The second kernel
can check whether the previous kernel sent data and retrieve it.

This is the architecture-specific part.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/kexec.h       |  12 +++-
 arch/powerpc/kernel/kexec_elf_64.c     |   2 +-
 arch/powerpc/kernel/machine_kexec_64.c | 114 +++++++++++++++++++++++++++++++--
 3 files changed, 120 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 31bc64e07c8f..b20738df26f8 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -92,12 +92,20 @@ static inline bool kdump_in_progress(void)
 }
 
 #ifdef CONFIG_KEXEC_FILE
+#define ARCH_HAS_KIMAGE_ARCH
+
+struct kimage_arch {
+	phys_addr_t handover_buffer_addr;
+	unsigned long handover_buffer_size;
+};
+
 int setup_purgatory(struct kimage *image, const void *slave_code,
 		    const void *fdt, unsigned long kernel_load_addr,
 		    unsigned long fdt_load_addr, unsigned long stack_top,
 		    int debug);
-int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
-		  unsigned long initrd_len, const char *cmdline);
+int setup_new_fdt(const struct kimage *image, void *fdt,
+		  unsigned long initrd_load_addr, unsigned long initrd_len,
+		  const char *cmdline);
 bool find_debug_console(const void *fdt, int chosen_node);
 int merge_partial_dtb(void *to, const void *from);
 #endif /* CONFIG_KEXEC_FILE */
diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c
index 1b902ad66e2a..22afc7b5ee73 100644
--- a/arch/powerpc/kernel/kexec_elf_64.c
+++ b/arch/powerpc/kernel/kexec_elf_64.c
@@ -219,7 +219,7 @@ void *elf64_load(struct kimage *image, char *kernel_buf,
 		}
 	}
 
-	ret = setup_new_fdt(fdt, initrd_load_addr, initrd_len, cmdline);
+	ret = setup_new_fdt(image, fdt, initrd_load_addr, initrd_len, cmdline);
 	if (ret)
 		goto out;
 
diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
index a484a6346146..190c652e49b7 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -490,6 +490,60 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
 	return image->fops->cleanup(image->image_loader_data);
 }
 
+bool kexec_can_hand_over_buffer(void)
+{
+	return true;
+}
+
+int arch_kexec_add_handover_buffer(struct kimage *image,
+				   unsigned long load_addr, unsigned long size)
+{
+	image->arch.handover_buffer_addr = load_addr;
+	image->arch.handover_buffer_size = size;
+
+	return 0;
+}
+
+int kexec_get_handover_buffer(void **addr, unsigned long *size)
+{
+	int ret;
+	u64 start_addr, end_addr;
+
+	ret = of_property_read_u64(of_chosen,
+				   "linux,kexec-handover-buffer-start",
+				   &start_addr);
+	if (ret == -EINVAL)
+		return -ENOENT;
+	else if (ret)
+		return -EINVAL;
+
+	ret = of_property_read_u64(of_chosen, "linux,kexec-handover-buffer-end",
+				   &end_addr);
+	if (ret == -EINVAL)
+		return -ENOENT;
+	else if (ret)
+		return -EINVAL;
+
+	*addr =  __va(start_addr);
+	/* -end is the first address after the buffer. */
+	*size = end_addr - start_addr;
+
+	return 0;
+}
+
+int kexec_free_handover_buffer(void)
+{
+	int ret;
+	void *addr;
+	unsigned long size;
+
+	ret = kexec_get_handover_buffer(&addr, &size);
+	if (ret)
+		return ret;
+
+	return memblock_free((phys_addr_t) addr, size);
+}
+
 /**
  * arch_kexec_walk_mem() - call func(data) for each unreserved memory block
  * @kbuf:	Context info for the search. Also passed to @func.
@@ -687,9 +741,52 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
 	return 0;
 }
 
-/*
- * setup_new_fdt() - modify /chosen and memory reservation for the next kernel
- * @fdt:
+/**
+ * setup_handover_buffer() - add properties and reservation for the handover buffer
+ * @image:		kexec image being loaded.
+ * @fdt:		Flattened device tree for the next kernel.
+ * @chosen_node:	Offset to the chosen node.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+static int setup_handover_buffer(const struct kimage *image, void *fdt,
+				 int chosen_node)
+{
+	int ret;
+
+	if (image->arch.handover_buffer_addr == 0)
+		return 0;
+
+	ret = fdt_setprop_u64(fdt, chosen_node,
+			      "linux,kexec-handover-buffer-start",
+			      image->arch.handover_buffer_addr);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* -end is the first address after the buffer. */
+	ret = fdt_setprop_u64(fdt, chosen_node,
+			      "linux,kexec-handover-buffer-end",
+			      image->arch.handover_buffer_addr +
+			      image->arch.handover_buffer_size);
+	if (ret < 0)
+		return -EINVAL;
+
+	ret = fdt_add_mem_rsv(fdt, image->arch.handover_buffer_addr,
+			      image->arch.handover_buffer_size);
+	if (ret)
+		return -EINVAL;
+
+	pr_debug("kexec handover buffer at 0x%llx, size = 0x%lx\n",
+		 image->arch.handover_buffer_addr,
+		 image->arch.handover_buffer_size);
+
+	return 0;
+}
+
+/**
+ * setup_new_fdt() - modify /chosen and memory reservations for the next kernel
+ * @image:		kexec image being loaded.
+ * @fdt:		Flattened device tree for the next kernel.
  * @initrd_load_addr:	Address where the next initrd will be loaded.
  * @initrd_len:		Size of the next initrd, or 0 if there will be none.
  * @cmdline:		Command line for the next kernel, or NULL if there will
@@ -697,8 +794,9 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
  *
  * Return: 0 on success, or negative errno on error.
  */
-int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
-		  unsigned long initrd_len, const char *cmdline)
+int setup_new_fdt(const struct kimage *image, void *fdt,
+		  unsigned long initrd_load_addr, unsigned long initrd_len,
+		  const char *cmdline)
 {
 	uint64_t oldfdt_addr;
 	int i, ret, chosen_node;
@@ -847,6 +945,12 @@ int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
 		}
 	}
 
+	ret = setup_handover_buffer(image, fdt, chosen_node);
+	if (ret) {
+		pr_err("Error setting up the new device tree.\n");
+		return ret;
+	}
+
 	ret = fdt_setprop(fdt, chosen_node, "linux,booted-from-kexec", NULL, 0);
 	if (ret) {
 		pr_err("Error setting up the new device tree.\n");
-- 
1.9.1

[PATCH v2 2/6] powerpc: kexec_file: Add buffer hand-over support for the next kernel

[Thiago Jung Bauermann]

The buffer hand-over mechanism allows the currently running kernel to pass
data to kernel that will be kexec'd via a kexec segment. The second kernel
can check whether the previous kernel sent data and retrieve it.

This is the architecture-specific part.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/kexec.h       |  12 +++-
 arch/powerpc/kernel/kexec_elf_64.c     |   2 +-
 arch/powerpc/kernel/machine_kexec_64.c | 114 +++++++++++++++++++++++++++++++--
 3 files changed, 120 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 31bc64e07c8f..b20738df26f8 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -92,12 +92,20 @@ static inline bool kdump_in_progress(void)
 }
 
 #ifdef CONFIG_KEXEC_FILE
+#define ARCH_HAS_KIMAGE_ARCH
+
+struct kimage_arch {
+	phys_addr_t handover_buffer_addr;
+	unsigned long handover_buffer_size;
+};
+
 int setup_purgatory(struct kimage *image, const void *slave_code,
 		    const void *fdt, unsigned long kernel_load_addr,
 		    unsigned long fdt_load_addr, unsigned long stack_top,
 		    int debug);
-int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
-		  unsigned long initrd_len, const char *cmdline);
+int setup_new_fdt(const struct kimage *image, void *fdt,
+		  unsigned long initrd_load_addr, unsigned long initrd_len,
+		  const char *cmdline);
 bool find_debug_console(const void *fdt, int chosen_node);
 int merge_partial_dtb(void *to, const void *from);
 #endif /* CONFIG_KEXEC_FILE */
diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c
index 1b902ad66e2a..22afc7b5ee73 100644
--- a/arch/powerpc/kernel/kexec_elf_64.c
+++ b/arch/powerpc/kernel/kexec_elf_64.c
@@ -219,7 +219,7 @@ void *elf64_load(struct kimage *image, char *kernel_buf,
 		}
 	}
 
-	ret = setup_new_fdt(fdt, initrd_load_addr, initrd_len, cmdline);
+	ret = setup_new_fdt(image, fdt, initrd_load_addr, initrd_len, cmdline);
 	if (ret)
 		goto out;
 
diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
index a484a6346146..190c652e49b7 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -490,6 +490,60 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
 	return image->fops->cleanup(image->image_loader_data);
 }
 
+bool kexec_can_hand_over_buffer(void)
+{
+	return true;
+}
+
+int arch_kexec_add_handover_buffer(struct kimage *image,
+				   unsigned long load_addr, unsigned long size)
+{
+	image->arch.handover_buffer_addr = load_addr;
+	image->arch.handover_buffer_size = size;
+
+	return 0;
+}
+
+int kexec_get_handover_buffer(void **addr, unsigned long *size)
+{
+	int ret;
+	u64 start_addr, end_addr;
+
+	ret = of_property_read_u64(of_chosen,
+				   "linux,kexec-handover-buffer-start",
+				   &start_addr);
+	if (ret == -EINVAL)
+		return -ENOENT;
+	else if (ret)
+		return -EINVAL;
+
+	ret = of_property_read_u64(of_chosen, "linux,kexec-handover-buffer-end",
+				   &end_addr);
+	if (ret == -EINVAL)
+		return -ENOENT;
+	else if (ret)
+		return -EINVAL;
+
+	*addr =  __va(start_addr);
+	/* -end is the first address after the buffer. */
+	*size = end_addr - start_addr;
+
+	return 0;
+}
+
+int kexec_free_handover_buffer(void)
+{
+	int ret;
+	void *addr;
+	unsigned long size;
+
+	ret = kexec_get_handover_buffer(&addr, &size);
+	if (ret)
+		return ret;
+
+	return memblock_free((phys_addr_t) addr, size);
+}
+
 /**
  * arch_kexec_walk_mem() - call func(data) for each unreserved memory block
  * @kbuf:	Context info for the search. Also passed to @func.
@@ -687,9 +741,52 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
 	return 0;
 }
 
-/*
- * setup_new_fdt() - modify /chosen and memory reservation for the next kernel
- * @fdt:
+/**
+ * setup_handover_buffer() - add properties and reservation for the handover buffer
+ * @image:		kexec image being loaded.
+ * @fdt:		Flattened device tree for the next kernel.
+ * @chosen_node:	Offset to the chosen node.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+static int setup_handover_buffer(const struct kimage *image, void *fdt,
+				 int chosen_node)
+{
+	int ret;
+
+	if (image->arch.handover_buffer_addr == 0)
+		return 0;
+
+	ret = fdt_setprop_u64(fdt, chosen_node,
+			      "linux,kexec-handover-buffer-start",
+			      image->arch.handover_buffer_addr);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* -end is the first address after the buffer. */
+	ret = fdt_setprop_u64(fdt, chosen_node,
+			      "linux,kexec-handover-buffer-end",
+			      image->arch.handover_buffer_addr +
+			      image->arch.handover_buffer_size);
+	if (ret < 0)
+		return -EINVAL;
+
+	ret = fdt_add_mem_rsv(fdt, image->arch.handover_buffer_addr,
+			      image->arch.handover_buffer_size);
+	if (ret)
+		return -EINVAL;
+
+	pr_debug("kexec handover buffer at 0x%llx, size = 0x%lx\n",
+		 image->arch.handover_buffer_addr,
+		 image->arch.handover_buffer_size);
+
+	return 0;
+}
+
+/**
+ * setup_new_fdt() - modify /chosen and memory reservations for the next kernel
+ * @image:		kexec image being loaded.
+ * @fdt:		Flattened device tree for the next kernel.
  * @initrd_load_addr:	Address where the next initrd will be loaded.
  * @initrd_len:		Size of the next initrd, or 0 if there will be none.
  * @cmdline:		Command line for the next kernel, or NULL if there will
@@ -697,8 +794,9 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
  *
  * Return: 0 on success, or negative errno on error.
  */
-int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
-		  unsigned long initrd_len, const char *cmdline)
+int setup_new_fdt(const struct kimage *image, void *fdt,
+		  unsigned long initrd_load_addr, unsigned long initrd_len,
+		  const char *cmdline)
 {
 	uint64_t oldfdt_addr;
 	int i, ret, chosen_node;
@@ -847,6 +945,12 @@ int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
 		}
 	}
 
+	ret = setup_handover_buffer(image, fdt, chosen_node);
+	if (ret) {
+		pr_err("Error setting up the new device tree.\n");
+		return ret;
+	}
+
 	ret = fdt_setprop(fdt, chosen_node, "linux,booted-from-kexec", NULL, 0);
 	if (ret) {
 		pr_err("Error setting up the new device tree.\n");
-- 
1.9.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Thiago Jung Bauermann]

Adds checksum argument to kexec_add_buffer specifying whether the given
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.

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

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

[Thiago Jung Bauermann]

Adds checksum argument to kexec_add_buffer specifying whether the given
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.

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

[PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.

[Thiago Jung Bauermann]

kexec_update_segment allows a given segment in kexec_image to have
its contents updated. This is useful if the current kernel wants to
send information to the next kernel that is up-to-date at the time of
reboot.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 include/linux/kexec.h |  2 ++
 kernel/kexec_core.c   | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 37eea32fdff1..14dda81e3e01 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -259,6 +259,8 @@ extern int kexec_purgatory_get_set_symbol(struct kimage *image,
 					  unsigned int size, bool get_value);
 extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
 					     const char *name);
+int kexec_update_segment(const char *buffer, unsigned long bufsz,
+			 unsigned long load_addr, unsigned long memsz);
 extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 561675589511..806735201de6 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -721,6 +721,105 @@ static struct page *kimage_alloc_page(struct kimage *image,
 	return page;
 }
 
+/**
+ * kexec_update_segment - update the contents of a kimage segment
+ * @buffer:	New contents of the segment.
+ * @bufsz:	@buffer size.
+ * @load_addr:	Segment's physical address in the next kernel.
+ * @memsz:	Segment size.
+ *
+ * This function assumes kexec_mutex is held.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int kexec_update_segment(const char *buffer, unsigned long bufsz,
+			 unsigned long load_addr, unsigned long memsz)
+{
+	int i;
+	unsigned long entry;
+	unsigned long *ptr = NULL;
+	void *dest = NULL;
+
+	if (kexec_image == NULL) {
+		pr_err("Can't update segment: no kexec image loaded.\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * kexec_add_buffer rounds up segment sizes to PAGE_SIZE, so
+	 * we have to do it here as well.
+	 */
+	memsz = ALIGN(memsz, PAGE_SIZE);
+
+	for (i = 0; i < kexec_image->nr_segments; i++)
+		/* We only support updating whole segments. */
+		if (load_addr == kexec_image->segment[i].mem &&
+		    memsz == kexec_image->segment[i].memsz) {
+			if (kexec_image->segment[i].do_checksum) {
+				pr_err("Trying to update non-modifiable segment.\n");
+				return -EINVAL;
+			}
+
+			break;
+		}
+	if (i == kexec_image->nr_segments) {
+		pr_err("Couldn't find segment to update: 0x%lx, size 0x%lx\n",
+		       load_addr, memsz);
+		return -EINVAL;
+	}
+
+	for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
+	     entry = *ptr++) {
+		void *addr = (void *) (entry & PAGE_MASK);
+
+		switch (entry & IND_FLAGS) {
+		case IND_DESTINATION:
+			dest = addr;
+			break;
+		case IND_INDIRECTION:
+			ptr = __va(addr);
+			break;
+		case IND_SOURCE:
+			/* Shouldn't happen, but verify just to be safe. */
+			if (dest == NULL) {
+				pr_err("Invalid kexec entries list.");
+				return -EINVAL;
+			}
+
+			if (dest == (void *) load_addr) {
+				struct page *page;
+				char *ptr;
+				size_t uchunk, mchunk;
+
+				page = kmap_to_page(addr);
+
+				ptr = kmap(page);
+				ptr += load_addr & ~PAGE_MASK;
+				mchunk = min_t(size_t, memsz,
+					       PAGE_SIZE - (load_addr & ~PAGE_MASK));
+				uchunk = min(bufsz, mchunk);
+				memcpy(ptr, buffer, uchunk);
+
+				kunmap(page);
+
+				bufsz -= uchunk;
+				load_addr += mchunk;
+				buffer += mchunk;
+				memsz -= mchunk;
+			}
+			dest += PAGE_SIZE;
+		}
+
+		/* Shouldn't happen, but verify just to be safe. */
+		if (ptr == NULL) {
+			pr_err("Invalid kexec entries list.");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int kimage_load_normal_segment(struct kimage *image,
 					 struct kexec_segment *segment)
 {
-- 
1.9.1

[PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.

[Thiago Jung Bauermann]

kexec_update_segment allows a given segment in kexec_image to have
its contents updated. This is useful if the current kernel wants to
send information to the next kernel that is up-to-date at the time of
reboot.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 include/linux/kexec.h |  2 ++
 kernel/kexec_core.c   | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 37eea32fdff1..14dda81e3e01 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -259,6 +259,8 @@ extern int kexec_purgatory_get_set_symbol(struct kimage *image,
 					  unsigned int size, bool get_value);
 extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
 					     const char *name);
+int kexec_update_segment(const char *buffer, unsigned long bufsz,
+			 unsigned long load_addr, unsigned long memsz);
 extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 561675589511..806735201de6 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -721,6 +721,105 @@ static struct page *kimage_alloc_page(struct kimage *image,
 	return page;
 }
 
+/**
+ * kexec_update_segment - update the contents of a kimage segment
+ * @buffer:	New contents of the segment.
+ * @bufsz:	@buffer size.
+ * @load_addr:	Segment's physical address in the next kernel.
+ * @memsz:	Segment size.
+ *
+ * This function assumes kexec_mutex is held.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int kexec_update_segment(const char *buffer, unsigned long bufsz,
+			 unsigned long load_addr, unsigned long memsz)
+{
+	int i;
+	unsigned long entry;
+	unsigned long *ptr = NULL;
+	void *dest = NULL;
+
+	if (kexec_image == NULL) {
+		pr_err("Can't update segment: no kexec image loaded.\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * kexec_add_buffer rounds up segment sizes to PAGE_SIZE, so
+	 * we have to do it here as well.
+	 */
+	memsz = ALIGN(memsz, PAGE_SIZE);
+
+	for (i = 0; i < kexec_image->nr_segments; i++)
+		/* We only support updating whole segments. */
+		if (load_addr == kexec_image->segment[i].mem &&
+		    memsz == kexec_image->segment[i].memsz) {
+			if (kexec_image->segment[i].do_checksum) {
+				pr_err("Trying to update non-modifiable segment.\n");
+				return -EINVAL;
+			}
+
+			break;
+		}
+	if (i == kexec_image->nr_segments) {
+		pr_err("Couldn't find segment to update: 0x%lx, size 0x%lx\n",
+		       load_addr, memsz);
+		return -EINVAL;
+	}
+
+	for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
+	     entry = *ptr++) {
+		void *addr = (void *) (entry & PAGE_MASK);
+
+		switch (entry & IND_FLAGS) {
+		case IND_DESTINATION:
+			dest = addr;
+			break;
+		case IND_INDIRECTION:
+			ptr = __va(addr);
+			break;
+		case IND_SOURCE:
+			/* Shouldn't happen, but verify just to be safe. */
+			if (dest == NULL) {
+				pr_err("Invalid kexec entries list.");
+				return -EINVAL;
+			}
+
+			if (dest == (void *) load_addr) {
+				struct page *page;
+				char *ptr;
+				size_t uchunk, mchunk;
+
+				page = kmap_to_page(addr);
+
+				ptr = kmap(page);
+				ptr += load_addr & ~PAGE_MASK;
+				mchunk = min_t(size_t, memsz,
+					       PAGE_SIZE - (load_addr & ~PAGE_MASK));
+				uchunk = min(bufsz, mchunk);
+				memcpy(ptr, buffer, uchunk);
+
+				kunmap(page);
+
+				bufsz -= uchunk;
+				load_addr += mchunk;
+				buffer += mchunk;
+				memsz -= mchunk;
+			}
+			dest += PAGE_SIZE;
+		}
+
+		/* Shouldn't happen, but verify just to be safe. */
+		if (ptr == NULL) {
+			pr_err("Invalid kexec entries list.");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int kimage_load_normal_segment(struct kimage *image,
 					 struct kexec_segment *segment)
 {
-- 
1.9.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 5/6] kexec: Share logic to copy segment page contents.

[Thiago Jung Bauermann]

Make kimage_load_normal_segment and kexec_update_segment share code
which they currently duplicate.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 kernel/kexec_core.c | 159 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 95 insertions(+), 64 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 806735201de6..68b5b245e457 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -721,6 +721,65 @@ static struct page *kimage_alloc_page(struct kimage *image,
 	return page;
 }
 
+struct kimage_update_buffer_state {
+	/* Destination memory address currently being copied to. */
+	unsigned long maddr;
+
+	/* Bytes in buffer still left to copy. */
+	size_t ubytes;
+
+	/* Bytes in memory still left to copy. */
+	size_t mbytes;
+
+	/* If true, copy from kbuf. */
+	bool from_kernel;
+
+	/* Clear pages before copying? */
+	bool clear_pages;
+
+	/* Buffer position to continue copying from. */
+	const unsigned char *kbuf;
+	const unsigned char __user *buf;
+};
+
+static int kimage_update_page(struct page *page,
+			      struct kimage_update_buffer_state *state)
+{
+	char *ptr;
+	int result = 0;
+	size_t uchunk, mchunk;
+
+	ptr = kmap(page);
+
+	/* Start with a clear page */
+	if (state->clear_pages)
+		clear_page(ptr);
+
+	ptr += state->maddr & ~PAGE_MASK;
+	mchunk = min_t(size_t, state->mbytes,
+		       PAGE_SIZE - (state->maddr & ~PAGE_MASK));
+	uchunk = min(state->ubytes, mchunk);
+
+	if (state->from_kernel)
+		memcpy(ptr, state->kbuf, uchunk);
+	else
+		result = copy_from_user(ptr, state->buf, uchunk);
+
+	kunmap(page);
+	if (result)
+		return -EFAULT;
+
+	state->ubytes -= uchunk;
+	state->maddr += mchunk;
+	if (state->from_kernel)
+		state->kbuf += mchunk;
+	else
+		state->buf += mchunk;
+	state->mbytes -= mchunk;
+
+	return 0;
+}
+
 /**
  * kexec_update_segment - update the contents of a kimage segment
  * @buffer:	New contents of the segment.
@@ -739,6 +798,7 @@ int kexec_update_segment(const char *buffer, unsigned long bufsz,
 	unsigned long entry;
 	unsigned long *ptr = NULL;
 	void *dest = NULL;
+	struct kimage_update_buffer_state state;
 
 	if (kexec_image == NULL) {
 		pr_err("Can't update segment: no kexec image loaded.\n");
@@ -768,8 +828,15 @@ int kexec_update_segment(const char *buffer, unsigned long bufsz,
 		return -EINVAL;
 	}
 
-	for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
-	     entry = *ptr++) {
+	state.maddr = load_addr;
+	state.ubytes = bufsz;
+	state.mbytes = memsz;
+	state.kbuf = buffer;
+	state.from_kernel = true;
+	state.clear_pages = false;
+
+	for (entry = kexec_image->head; !(entry & IND_DONE) &&
+					state.mbytes; entry = *ptr++) {
 		void *addr = (void *) (entry & PAGE_MASK);
 
 		switch (entry & IND_FLAGS) {
@@ -786,26 +853,13 @@ int kexec_update_segment(const char *buffer, unsigned long bufsz,
 				return -EINVAL;
 			}
 
-			if (dest == (void *) load_addr) {
-				struct page *page;
-				char *ptr;
-				size_t uchunk, mchunk;
-
-				page = kmap_to_page(addr);
-
-				ptr = kmap(page);
-				ptr += load_addr & ~PAGE_MASK;
-				mchunk = min_t(size_t, memsz,
-					       PAGE_SIZE - (load_addr & ~PAGE_MASK));
-				uchunk = min(bufsz, mchunk);
-				memcpy(ptr, buffer, uchunk);
-
-				kunmap(page);
+			if (dest == (void *) state.maddr) {
+				int ret;
 
-				bufsz -= uchunk;
-				load_addr += mchunk;
-				buffer += mchunk;
-				memsz -= mchunk;
+				ret = kimage_update_page(kmap_to_page(addr),
+							 &state);
+				if (ret)
+					return ret;
 			}
 			dest += PAGE_SIZE;
 		}
@@ -823,31 +877,30 @@ int kexec_update_segment(const char *buffer, unsigned long bufsz,
 static int kimage_load_normal_segment(struct kimage *image,
 					 struct kexec_segment *segment)
 {
-	unsigned long maddr;
-	size_t ubytes, mbytes;
-	int result;
-	unsigned char __user *buf = NULL;
-	unsigned char *kbuf = NULL;
-
-	result = 0;
-	if (image->file_mode)
-		kbuf = segment->kbuf;
-	else
-		buf = segment->buf;
-	ubytes = segment->bufsz;
-	mbytes = segment->memsz;
-	maddr = segment->mem;
+	int result = 0;
+	struct kimage_update_buffer_state state;
+
+	/* For file based kexec, source pages are in kernel memory */
+	if (image->file_mode) {
+		state.kbuf = segment->kbuf;
+		state.from_kernel = true;
+	} else {
+		state.buf = segment->buf;
+		state.from_kernel = false;
+	}
+	state.ubytes = segment->bufsz;
+	state.mbytes = segment->memsz;
+	state.maddr = segment->mem;
+	state.clear_pages = true;
 
-	result = kimage_set_destination(image, maddr);
+	result = kimage_set_destination(image, state.maddr);
 	if (result < 0)
 		goto out;
 
-	while (mbytes) {
+	while (state.mbytes) {
 		struct page *page;
-		char *ptr;
-		size_t uchunk, mchunk;
 
-		page = kimage_alloc_page(image, GFP_HIGHUSER, maddr);
+		page = kimage_alloc_page(image, GFP_HIGHUSER, state.maddr);
 		if (!page) {
 			result  = -ENOMEM;
 			goto out;
@@ -857,31 +910,9 @@ static int kimage_load_normal_segment(struct kimage *image,
 		if (result < 0)
 			goto out;
 
-		ptr = kmap(page);
-		/* Start with a clear page */
-		clear_page(ptr);
-		ptr += maddr & ~PAGE_MASK;
-		mchunk = min_t(size_t, mbytes,
-				PAGE_SIZE - (maddr & ~PAGE_MASK));
-		uchunk = min(ubytes, mchunk);
-
-		/* For file based kexec, source pages are in kernel memory */
-		if (image->file_mode)
-			memcpy(ptr, kbuf, uchunk);
-		else
-			result = copy_from_user(ptr, buf, uchunk);
-		kunmap(page);
-		if (result) {
-			result = -EFAULT;
+		result = kimage_update_page(page, &state);
+		if (result)
 			goto out;
-		}
-		ubytes -= uchunk;
-		maddr  += mchunk;
-		if (image->file_mode)
-			kbuf += mchunk;
-		else
-			buf += mchunk;
-		mbytes -= mchunk;
 	}
 out:
 	return result;
-- 
1.9.1

[PATCH v2 5/6] kexec: Share logic to copy segment page contents.

[Thiago Jung Bauermann]

Make kimage_load_normal_segment and kexec_update_segment share code
which they currently duplicate.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 kernel/kexec_core.c | 159 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 95 insertions(+), 64 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 806735201de6..68b5b245e457 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -721,6 +721,65 @@ static struct page *kimage_alloc_page(struct kimage *image,
 	return page;
 }
 
+struct kimage_update_buffer_state {
+	/* Destination memory address currently being copied to. */
+	unsigned long maddr;
+
+	/* Bytes in buffer still left to copy. */
+	size_t ubytes;
+
+	/* Bytes in memory still left to copy. */
+	size_t mbytes;
+
+	/* If true, copy from kbuf. */
+	bool from_kernel;
+
+	/* Clear pages before copying? */
+	bool clear_pages;
+
+	/* Buffer position to continue copying from. */
+	const unsigned char *kbuf;
+	const unsigned char __user *buf;
+};
+
+static int kimage_update_page(struct page *page,
+			      struct kimage_update_buffer_state *state)
+{
+	char *ptr;
+	int result = 0;
+	size_t uchunk, mchunk;
+
+	ptr = kmap(page);
+
+	/* Start with a clear page */
+	if (state->clear_pages)
+		clear_page(ptr);
+
+	ptr += state->maddr & ~PAGE_MASK;
+	mchunk = min_t(size_t, state->mbytes,
+		       PAGE_SIZE - (state->maddr & ~PAGE_MASK));
+	uchunk = min(state->ubytes, mchunk);
+
+	if (state->from_kernel)
+		memcpy(ptr, state->kbuf, uchunk);
+	else
+		result = copy_from_user(ptr, state->buf, uchunk);
+
+	kunmap(page);
+	if (result)
+		return -EFAULT;
+
+	state->ubytes -= uchunk;
+	state->maddr += mchunk;
+	if (state->from_kernel)
+		state->kbuf += mchunk;
+	else
+		state->buf += mchunk;
+	state->mbytes -= mchunk;
+
+	return 0;
+}
+
 /**
  * kexec_update_segment - update the contents of a kimage segment
  * @buffer:	New contents of the segment.
@@ -739,6 +798,7 @@ int kexec_update_segment(const char *buffer, unsigned long bufsz,
 	unsigned long entry;
 	unsigned long *ptr = NULL;
 	void *dest = NULL;
+	struct kimage_update_buffer_state state;
 
 	if (kexec_image == NULL) {
 		pr_err("Can't update segment: no kexec image loaded.\n");
@@ -768,8 +828,15 @@ int kexec_update_segment(const char *buffer, unsigned long bufsz,
 		return -EINVAL;
 	}
 
-	for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
-	     entry = *ptr++) {
+	state.maddr = load_addr;
+	state.ubytes = bufsz;
+	state.mbytes = memsz;
+	state.kbuf = buffer;
+	state.from_kernel = true;
+	state.clear_pages = false;
+
+	for (entry = kexec_image->head; !(entry & IND_DONE) &&
+					state.mbytes; entry = *ptr++) {
 		void *addr = (void *) (entry & PAGE_MASK);
 
 		switch (entry & IND_FLAGS) {
@@ -786,26 +853,13 @@ int kexec_update_segment(const char *buffer, unsigned long bufsz,
 				return -EINVAL;
 			}
 
-			if (dest == (void *) load_addr) {
-				struct page *page;
-				char *ptr;
-				size_t uchunk, mchunk;
-
-				page = kmap_to_page(addr);
-
-				ptr = kmap(page);
-				ptr += load_addr & ~PAGE_MASK;
-				mchunk = min_t(size_t, memsz,
-					       PAGE_SIZE - (load_addr & ~PAGE_MASK));
-				uchunk = min(bufsz, mchunk);
-				memcpy(ptr, buffer, uchunk);
-
-				kunmap(page);
+			if (dest == (void *) state.maddr) {
+				int ret;
 
-				bufsz -= uchunk;
-				load_addr += mchunk;
-				buffer += mchunk;
-				memsz -= mchunk;
+				ret = kimage_update_page(kmap_to_page(addr),
+							 &state);
+				if (ret)
+					return ret;
 			}
 			dest += PAGE_SIZE;
 		}
@@ -823,31 +877,30 @@ int kexec_update_segment(const char *buffer, unsigned long bufsz,
 static int kimage_load_normal_segment(struct kimage *image,
 					 struct kexec_segment *segment)
 {
-	unsigned long maddr;
-	size_t ubytes, mbytes;
-	int result;
-	unsigned char __user *buf = NULL;
-	unsigned char *kbuf = NULL;
-
-	result = 0;
-	if (image->file_mode)
-		kbuf = segment->kbuf;
-	else
-		buf = segment->buf;
-	ubytes = segment->bufsz;
-	mbytes = segment->memsz;
-	maddr = segment->mem;
+	int result = 0;
+	struct kimage_update_buffer_state state;
+
+	/* For file based kexec, source pages are in kernel memory */
+	if (image->file_mode) {
+		state.kbuf = segment->kbuf;
+		state.from_kernel = true;
+	} else {
+		state.buf = segment->buf;
+		state.from_kernel = false;
+	}
+	state.ubytes = segment->bufsz;
+	state.mbytes = segment->memsz;
+	state.maddr = segment->mem;
+	state.clear_pages = true;
 
-	result = kimage_set_destination(image, maddr);
+	result = kimage_set_destination(image, state.maddr);
 	if (result < 0)
 		goto out;
 
-	while (mbytes) {
+	while (state.mbytes) {
 		struct page *page;
-		char *ptr;
-		size_t uchunk, mchunk;
 
-		page = kimage_alloc_page(image, GFP_HIGHUSER, maddr);
+		page = kimage_alloc_page(image, GFP_HIGHUSER, state.maddr);
 		if (!page) {
 			result  = -ENOMEM;
 			goto out;
@@ -857,31 +910,9 @@ static int kimage_load_normal_segment(struct kimage *image,
 		if (result < 0)
 			goto out;
 
-		ptr = kmap(page);
-		/* Start with a clear page */
-		clear_page(ptr);
-		ptr += maddr & ~PAGE_MASK;
-		mchunk = min_t(size_t, mbytes,
-				PAGE_SIZE - (maddr & ~PAGE_MASK));
-		uchunk = min(ubytes, mchunk);
-
-		/* For file based kexec, source pages are in kernel memory */
-		if (image->file_mode)
-			memcpy(ptr, kbuf, uchunk);
-		else
-			result = copy_from_user(ptr, buf, uchunk);
-		kunmap(page);
-		if (result) {
-			result = -EFAULT;
+		result = kimage_update_page(page, &state);
+		if (result)
 			goto out;
-		}
-		ubytes -= uchunk;
-		maddr  += mchunk;
-		if (image->file_mode)
-			kbuf += mchunk;
-		else
-			buf += mchunk;
-		mbytes -= mchunk;
 	}
 out:
 	return result;
-- 
1.9.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 6/6] IMA: Demonstration code for kexec buffer passing.

[Thiago Jung Bauermann]

This patch is not intended to be committed.

It shows how kernel code can use the kexec buffer passing mechanism
to pass information to the next kernel.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 include/linux/ima.h                   | 11 +++++
 kernel/kexec_file.c                   |  4 ++
 security/integrity/ima/ima.h          |  5 +++
 security/integrity/ima/ima_init.c     | 26 +++++++++++
 security/integrity/ima/ima_template.c | 85 +++++++++++++++++++++++++++++++++++
 5 files changed, 131 insertions(+)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0eb7c2e7f0d6..96528d007139 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -11,6 +11,7 @@
 #define _LINUX_IMA_H
 
 #include <linux/fs.h>
+#include <linux/kexec.h>
 struct linux_binprm;
 
 #ifdef CONFIG_IMA
@@ -23,6 +24,10 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
 extern void ima_post_path_mknod(struct dentry *dentry);
 
+#ifdef CONFIG_KEXEC_FILE
+extern void ima_add_kexec_buffer(struct kimage *image);
+#endif
+
 #else
 static inline int ima_bprm_check(struct linux_binprm *bprm)
 {
@@ -60,6 +65,12 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
 	return;
 }
 
+#ifdef CONFIG_KEXEC_FILE
+static inline void ima_add_kexec_buffer(struct kimage *image)
+{
+}
+#endif
+
 #endif /* CONFIG_IMA */
 
 #ifdef CONFIG_IMA_APPRAISE
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index aed51175915f..bf8f61c20c11 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -19,6 +19,7 @@
 #include <linux/mutex.h>
 #include <linux/list.h>
 #include <linux/fs.h>
+#include <linux/ima.h>
 #include <crypto/hash.h>
 #include <crypto/sha.h>
 #include <linux/syscalls.h>
@@ -321,6 +322,9 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 		}
 	}
 
+	/* IMA needs to pass the measurement list to the next kernel. */
+	ima_add_kexec_buffer(image);
+
 	/* Call arch image load handlers */
 	ldata = arch_kexec_kernel_image_load(image);
 
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index db25f54a04fe..0334001055d7 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -102,6 +102,11 @@ struct ima_queue_entry {
 };
 extern struct list_head ima_measurements;	/* list of all measurements */
 
+#ifdef CONFIG_KEXEC_FILE
+extern void *kexec_buffer;
+extern size_t kexec_buffer_size;
+#endif
+
 /* Internal IMA function definitions */
 int ima_init(void);
 int ima_fs_init(void);
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 32912bd54ead..a1924d0f3b2b 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -21,6 +21,7 @@
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <linux/kexec.h>
 
 #include "ima.h"
 
@@ -104,6 +105,29 @@ void __init ima_load_x509(void)
 }
 #endif
 
+#ifdef CONFIG_KEXEC_FILE
+static void ima_load_kexec_buffer(void)
+{
+	int rc;
+
+	/* Fetch the buffer from the previous kernel, if any. */
+	rc = kexec_get_handover_buffer(&kexec_buffer, &kexec_buffer_size);
+	if (rc == 0) {
+		/* Demonstrate that buffer handover works. */
+		pr_err("kexec buffer contents: %s\n", (char *) kexec_buffer);
+		pr_err("kexec buffer contents after update: %s\n",
+		       (char *) kexec_buffer + 4 * PAGE_SIZE + 10);
+
+		kexec_free_handover_buffer();
+	} else if (rc == -ENOENT)
+		pr_debug("No kexec buffer from the previous kernel.\n");
+	else
+		pr_debug("Error restoring kexec buffer: %d\n", rc);
+}
+#else
+static void ima_load_kexec_buffer(void) { }
+#endif
+
 int __init ima_init(void)
 {
 	u8 pcr_i[TPM_DIGEST_SIZE];
@@ -134,5 +158,7 @@ int __init ima_init(void)
 
 	ima_init_policy();
 
+	ima_load_kexec_buffer();
+
 	return ima_fs_init();
 }
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index febd12ed9b55..a8609f3a13d2 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -15,6 +15,8 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/kexec.h>
+#include <linux/reboot.h>
 #include "ima.h"
 #include "ima_template_lib.h"
 
@@ -182,6 +184,89 @@ static int template_desc_init_fields(const char *template_fmt,
 	return 0;
 }
 
+#ifdef CONFIG_KEXEC_FILE
+void *kexec_buffer = NULL;
+size_t kexec_buffer_size = 0;
+
+/* Physical address of the measurement buffer in the next kernel. */
+unsigned long kexec_buffer_load_addr = 0;
+
+/*
+ * Called during reboot. IMA can add here new events that were generated after
+ * the kexec image was loaded.
+ */
+static int ima_update_kexec_buffer(struct notifier_block *self,
+				   unsigned long action, void *data)
+{
+	int ret;
+
+	if (!kexec_in_progress)
+		return NOTIFY_OK;
+
+	/*
+	 * Add content deep in the buffer to show that we can update
+	 * all of it.
+	 */
+	strcpy(kexec_buffer + 4 * PAGE_SIZE + 10,
+	       "Updated kexec buffer contents.");
+
+	ret = kexec_update_segment(kexec_buffer, kexec_buffer_size,
+				   kexec_buffer_load_addr, kexec_buffer_size);
+	if (ret)
+		pr_err("Error updating kexec buffer: %d\n", ret);
+
+	return NOTIFY_OK;
+}
+
+struct notifier_block update_buffer_nb = {
+	.notifier_call = ima_update_kexec_buffer,
+};
+
+/*
+ * Called during kexec_file_load so that IMA can add a segment to the kexec
+ * image with the measurement event log for the next kernel.
+ */
+void ima_add_kexec_buffer(struct kimage *image)
+{
+	struct kexec_buf kbuf = { .image = image, .buf_align = PAGE_SIZE,
+				  .buf_min = 0, .buf_max = ULONG_MAX,
+				  .top_down = true };
+	bool first_kexec_load = kexec_buffer_load_addr == 0;
+	int ret;
+
+	if (!kexec_can_hand_over_buffer())
+		return;
+
+	if (!first_kexec_load)
+		kfree(kexec_buffer);
+
+	/* Create a relatively big buffer, for testing. */
+	kexec_buffer_size = kbuf.bufsz = kbuf.memsz = 5 * PAGE_SIZE;
+	kexec_buffer = kbuf.buffer = kzalloc(kexec_buffer_size, GFP_KERNEL);
+	if (!kexec_buffer) {
+		pr_err("Not enough memory for the kexec measurement buffer.\n");
+		return;
+	}
+
+	/* Add some content for demonstration purposes. */
+	strcpy(kexec_buffer, "Buffer contents at kexec load time.");
+
+	/* Ask not to checksum the segment, we may have to update it later. */
+	ret = kexec_add_handover_buffer(&kbuf, false);
+	if (ret) {
+		pr_err("Error passing over kexec measurement buffer.\n");
+		return;
+	}
+	kexec_buffer_load_addr = kbuf.mem;
+
+	if (first_kexec_load)
+		register_reboot_notifier(&update_buffer_nb);
+
+	pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
+		 kexec_buffer_load_addr);
+}
+#endif /* CONFIG_KEXEC_FILE */
+
 struct ima_template_desc *ima_template_desc_current(void)
 {
 	if (!ima_template)
-- 
1.9.1

[PATCH v2 6/6] IMA: Demonstration code for kexec buffer passing.

[Thiago Jung Bauermann]

This patch is not intended to be committed.

It shows how kernel code can use the kexec buffer passing mechanism
to pass information to the next kernel.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 include/linux/ima.h                   | 11 +++++
 kernel/kexec_file.c                   |  4 ++
 security/integrity/ima/ima.h          |  5 +++
 security/integrity/ima/ima_init.c     | 26 +++++++++++
 security/integrity/ima/ima_template.c | 85 +++++++++++++++++++++++++++++++++++
 5 files changed, 131 insertions(+)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0eb7c2e7f0d6..96528d007139 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -11,6 +11,7 @@
 #define _LINUX_IMA_H
 
 #include <linux/fs.h>
+#include <linux/kexec.h>
 struct linux_binprm;
 
 #ifdef CONFIG_IMA
@@ -23,6 +24,10 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
 extern void ima_post_path_mknod(struct dentry *dentry);
 
+#ifdef CONFIG_KEXEC_FILE
+extern void ima_add_kexec_buffer(struct kimage *image);
+#endif
+
 #else
 static inline int ima_bprm_check(struct linux_binprm *bprm)
 {
@@ -60,6 +65,12 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
 	return;
 }
 
+#ifdef CONFIG_KEXEC_FILE
+static inline void ima_add_kexec_buffer(struct kimage *image)
+{
+}
+#endif
+
 #endif /* CONFIG_IMA */
 
 #ifdef CONFIG_IMA_APPRAISE
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index aed51175915f..bf8f61c20c11 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -19,6 +19,7 @@
 #include <linux/mutex.h>
 #include <linux/list.h>
 #include <linux/fs.h>
+#include <linux/ima.h>
 #include <crypto/hash.h>
 #include <crypto/sha.h>
 #include <linux/syscalls.h>
@@ -321,6 +322,9 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 		}
 	}
 
+	/* IMA needs to pass the measurement list to the next kernel. */
+	ima_add_kexec_buffer(image);
+
 	/* Call arch image load handlers */
 	ldata = arch_kexec_kernel_image_load(image);
 
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index db25f54a04fe..0334001055d7 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -102,6 +102,11 @@ struct ima_queue_entry {
 };
 extern struct list_head ima_measurements;	/* list of all measurements */
 
+#ifdef CONFIG_KEXEC_FILE
+extern void *kexec_buffer;
+extern size_t kexec_buffer_size;
+#endif
+
 /* Internal IMA function definitions */
 int ima_init(void);
 int ima_fs_init(void);
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 32912bd54ead..a1924d0f3b2b 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -21,6 +21,7 @@
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <linux/kexec.h>
 
 #include "ima.h"
 
@@ -104,6 +105,29 @@ void __init ima_load_x509(void)
 }
 #endif
 
+#ifdef CONFIG_KEXEC_FILE
+static void ima_load_kexec_buffer(void)
+{
+	int rc;
+
+	/* Fetch the buffer from the previous kernel, if any. */
+	rc = kexec_get_handover_buffer(&kexec_buffer, &kexec_buffer_size);
+	if (rc == 0) {
+		/* Demonstrate that buffer handover works. */
+		pr_err("kexec buffer contents: %s\n", (char *) kexec_buffer);
+		pr_err("kexec buffer contents after update: %s\n",
+		       (char *) kexec_buffer + 4 * PAGE_SIZE + 10);
+
+		kexec_free_handover_buffer();
+	} else if (rc == -ENOENT)
+		pr_debug("No kexec buffer from the previous kernel.\n");
+	else
+		pr_debug("Error restoring kexec buffer: %d\n", rc);
+}
+#else
+static void ima_load_kexec_buffer(void) { }
+#endif
+
 int __init ima_init(void)
 {
 	u8 pcr_i[TPM_DIGEST_SIZE];
@@ -134,5 +158,7 @@ int __init ima_init(void)
 
 	ima_init_policy();
 
+	ima_load_kexec_buffer();
+
 	return ima_fs_init();
 }
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index febd12ed9b55..a8609f3a13d2 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -15,6 +15,8 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/kexec.h>
+#include <linux/reboot.h>
 #include "ima.h"
 #include "ima_template_lib.h"
 
@@ -182,6 +184,89 @@ static int template_desc_init_fields(const char *template_fmt,
 	return 0;
 }
 
+#ifdef CONFIG_KEXEC_FILE
+void *kexec_buffer = NULL;
+size_t kexec_buffer_size = 0;
+
+/* Physical address of the measurement buffer in the next kernel. */
+unsigned long kexec_buffer_load_addr = 0;
+
+/*
+ * Called during reboot. IMA can add here new events that were generated after
+ * the kexec image was loaded.
+ */
+static int ima_update_kexec_buffer(struct notifier_block *self,
+				   unsigned long action, void *data)
+{
+	int ret;
+
+	if (!kexec_in_progress)
+		return NOTIFY_OK;
+
+	/*
+	 * Add content deep in the buffer to show that we can update
+	 * all of it.
+	 */
+	strcpy(kexec_buffer + 4 * PAGE_SIZE + 10,
+	       "Updated kexec buffer contents.");
+
+	ret = kexec_update_segment(kexec_buffer, kexec_buffer_size,
+				   kexec_buffer_load_addr, kexec_buffer_size);
+	if (ret)
+		pr_err("Error updating kexec buffer: %d\n", ret);
+
+	return NOTIFY_OK;
+}
+
+struct notifier_block update_buffer_nb = {
+	.notifier_call = ima_update_kexec_buffer,
+};
+
+/*
+ * Called during kexec_file_load so that IMA can add a segment to the kexec
+ * image with the measurement event log for the next kernel.
+ */
+void ima_add_kexec_buffer(struct kimage *image)
+{
+	struct kexec_buf kbuf = { .image = image, .buf_align = PAGE_SIZE,
+				  .buf_min = 0, .buf_max = ULONG_MAX,
+				  .top_down = true };
+	bool first_kexec_load = kexec_buffer_load_addr == 0;
+	int ret;
+
+	if (!kexec_can_hand_over_buffer())
+		return;
+
+	if (!first_kexec_load)
+		kfree(kexec_buffer);
+
+	/* Create a relatively big buffer, for testing. */
+	kexec_buffer_size = kbuf.bufsz = kbuf.memsz = 5 * PAGE_SIZE;
+	kexec_buffer = kbuf.buffer = kzalloc(kexec_buffer_size, GFP_KERNEL);
+	if (!kexec_buffer) {
+		pr_err("Not enough memory for the kexec measurement buffer.\n");
+		return;
+	}
+
+	/* Add some content for demonstration purposes. */
+	strcpy(kexec_buffer, "Buffer contents at kexec load time.");
+
+	/* Ask not to checksum the segment, we may have to update it later. */
+	ret = kexec_add_handover_buffer(&kbuf, false);
+	if (ret) {
+		pr_err("Error passing over kexec measurement buffer.\n");
+		return;
+	}
+	kexec_buffer_load_addr = kbuf.mem;
+
+	if (first_kexec_load)
+		register_reboot_notifier(&update_buffer_nb);
+
+	pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
+		 kexec_buffer_load_addr);
+}
+#endif /* CONFIG_KEXEC_FILE */
+
 struct ima_template_desc *ima_template_desc_current(void)
 {
 	if (!ima_template)
-- 
1.9.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.

[Andrew Morton]

On Sat, 13 Aug 2016 00:18:23 -0300 Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> wrote:

> kexec_update_segment allows a given segment in kexec_image to have
> its contents updated. This is useful if the current kernel wants to
> send information to the next kernel that is up-to-date at the time of
> reboot.
> 
> ...
>
> @@ -721,6 +721,105 @@ static struct page *kimage_alloc_page(struct kimage *image,
>  	return page;
>  }
>  
> +/**
> + * kexec_update_segment - update the contents of a kimage segment
> + * @buffer:	New contents of the segment.
> + * @bufsz:	@buffer size.
> + * @load_addr:	Segment's physical address in the next kernel.
> + * @memsz:	Segment size.
> + *
> + * This function assumes kexec_mutex is held.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int kexec_update_segment(const char *buffer, unsigned long bufsz,
> +			 unsigned long load_addr, unsigned long memsz)
> +{
> +	int i;
> +	unsigned long entry;
> +	unsigned long *ptr = NULL;
> +	void *dest = NULL;
> +
> +	if (kexec_image == NULL) {
> +		pr_err("Can't update segment: no kexec image loaded.\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * kexec_add_buffer rounds up segment sizes to PAGE_SIZE, so
> +	 * we have to do it here as well.
> +	 */
> +	memsz = ALIGN(memsz, PAGE_SIZE);
> +
> +	for (i = 0; i < kexec_image->nr_segments; i++)
> +		/* We only support updating whole segments. */
> +		if (load_addr == kexec_image->segment[i].mem &&
> +		    memsz == kexec_image->segment[i].memsz) {
> +			if (kexec_image->segment[i].do_checksum) {
> +				pr_err("Trying to update non-modifiable segment.\n");
> +				return -EINVAL;
> +			}
> +
> +			break;
> +		}
> +	if (i == kexec_image->nr_segments) {
> +		pr_err("Couldn't find segment to update: 0x%lx, size 0x%lx\n",
> +		       load_addr, memsz);
> +		return -EINVAL;
> +	}
> +
> +	for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
> +	     entry = *ptr++) {
> +		void *addr = (void *) (entry & PAGE_MASK);
> +
> +		switch (entry & IND_FLAGS) {
> +		case IND_DESTINATION:
> +			dest = addr;
> +			break;
> +		case IND_INDIRECTION:
> +			ptr = __va(addr);
> +			break;
> +		case IND_SOURCE:
> +			/* Shouldn't happen, but verify just to be safe. */
> +			if (dest == NULL) {
> +				pr_err("Invalid kexec entries list.");
> +				return -EINVAL;
> +			}
> +
> +			if (dest == (void *) load_addr) {
> +				struct page *page;
> +				char *ptr;
> +				size_t uchunk, mchunk;
> +
> +				page = kmap_to_page(addr);
> +
> +				ptr = kmap(page);

kmap_atomic() could be used here, and it is appreciably faster.


> +				ptr += load_addr & ~PAGE_MASK;
> +				mchunk = min_t(size_t, memsz,
> +					       PAGE_SIZE - (load_addr & ~PAGE_MASK));
> +				uchunk = min(bufsz, mchunk);
> +				memcpy(ptr, buffer, uchunk);
> +
> +				kunmap(page);
> +
> +				bufsz -= uchunk;
> +				load_addr += mchunk;
> +				buffer += mchunk;
> +				memsz -= mchunk;
> +			}
> +			dest += PAGE_SIZE;
> +		}
> +
> +		/* Shouldn't happen, but verify just to be safe. */
> +		if (ptr == NULL) {
> +			pr_err("Invalid kexec entries list.");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +

Re: [PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.

[Andrew Morton]

On Sat, 13 Aug 2016 00:18:23 -0300 Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> wrote:

> kexec_update_segment allows a given segment in kexec_image to have
> its contents updated. This is useful if the current kernel wants to
> send information to the next kernel that is up-to-date at the time of
> reboot.
> 
> ...
>
> @@ -721,6 +721,105 @@ static struct page *kimage_alloc_page(struct kimage *image,
>  	return page;
>  }
>  
> +/**
> + * kexec_update_segment - update the contents of a kimage segment
> + * @buffer:	New contents of the segment.
> + * @bufsz:	@buffer size.
> + * @load_addr:	Segment's physical address in the next kernel.
> + * @memsz:	Segment size.
> + *
> + * This function assumes kexec_mutex is held.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int kexec_update_segment(const char *buffer, unsigned long bufsz,
> +			 unsigned long load_addr, unsigned long memsz)
> +{
> +	int i;
> +	unsigned long entry;
> +	unsigned long *ptr = NULL;
> +	void *dest = NULL;
> +
> +	if (kexec_image == NULL) {
> +		pr_err("Can't update segment: no kexec image loaded.\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * kexec_add_buffer rounds up segment sizes to PAGE_SIZE, so
> +	 * we have to do it here as well.
> +	 */
> +	memsz = ALIGN(memsz, PAGE_SIZE);
> +
> +	for (i = 0; i < kexec_image->nr_segments; i++)
> +		/* We only support updating whole segments. */
> +		if (load_addr == kexec_image->segment[i].mem &&
> +		    memsz == kexec_image->segment[i].memsz) {
> +			if (kexec_image->segment[i].do_checksum) {
> +				pr_err("Trying to update non-modifiable segment.\n");
> +				return -EINVAL;
> +			}
> +
> +			break;
> +		}
> +	if (i == kexec_image->nr_segments) {
> +		pr_err("Couldn't find segment to update: 0x%lx, size 0x%lx\n",
> +		       load_addr, memsz);
> +		return -EINVAL;
> +	}
> +
> +	for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
> +	     entry = *ptr++) {
> +		void *addr = (void *) (entry & PAGE_MASK);
> +
> +		switch (entry & IND_FLAGS) {
> +		case IND_DESTINATION:
> +			dest = addr;
> +			break;
> +		case IND_INDIRECTION:
> +			ptr = __va(addr);
> +			break;
> +		case IND_SOURCE:
> +			/* Shouldn't happen, but verify just to be safe. */
> +			if (dest == NULL) {
> +				pr_err("Invalid kexec entries list.");
> +				return -EINVAL;
> +			}
> +
> +			if (dest == (void *) load_addr) {
> +				struct page *page;
> +				char *ptr;
> +				size_t uchunk, mchunk;
> +
> +				page = kmap_to_page(addr);
> +
> +				ptr = kmap(page);

kmap_atomic() could be used here, and it is appreciably faster.


> +				ptr += load_addr & ~PAGE_MASK;
> +				mchunk = min_t(size_t, memsz,
> +					       PAGE_SIZE - (load_addr & ~PAGE_MASK));
> +				uchunk = min(bufsz, mchunk);
> +				memcpy(ptr, buffer, uchunk);
> +
> +				kunmap(page);
> +
> +				bufsz -= uchunk;
> +				load_addr += mchunk;
> +				buffer += mchunk;
> +				memsz -= mchunk;
> +			}
> +			dest += PAGE_SIZE;
> +		}
> +
> +		/* Shouldn't happen, but verify just to be safe. */
> +		if (ptr == NULL) {
> +			pr_err("Invalid kexec entries list.");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.

[Thiago Jung Bauermann]

Hello Andrew,

Thank you for your review!

Am Montag, 15 August 2016, 15:27:56 schrieb Andrew Morton:
> On Sat, 13 Aug 2016 00:18:23 -0300 Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> wrote:
> > +/**
> > + * kexec_update_segment - update the contents of a kimage segment
> > + * @buffer:	New contents of the segment.
> > + * @bufsz:	@buffer size.
> > + * @load_addr:	Segment's physical address in the next kernel.
> > + * @memsz:	Segment size.
> > + *
> > + * This function assumes kexec_mutex is held.
> > + *
> > + * Return: 0 on success, negative errno on error.
> > + */
> > +int kexec_update_segment(const char *buffer, unsigned long bufsz,
> > +			 unsigned long load_addr, unsigned long memsz)
> > +{
> > +	int i;
> > +	unsigned long entry;
> > +	unsigned long *ptr = NULL;
> > +	void *dest = NULL;
> > +
> > +	if (kexec_image == NULL) {
> > +		pr_err("Can't update segment: no kexec image loaded.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/*
> > +	 * kexec_add_buffer rounds up segment sizes to PAGE_SIZE, so
> > +	 * we have to do it here as well.
> > +	 */
> > +	memsz = ALIGN(memsz, PAGE_SIZE);
> > +
> > +	for (i = 0; i < kexec_image->nr_segments; i++)
> > +		/* We only support updating whole segments. */
> > +		if (load_addr == kexec_image->segment[i].mem &&
> > +		    memsz == kexec_image->segment[i].memsz) {
> > +			if (kexec_image->segment[i].do_checksum) {
> > +				pr_err("Trying to update non-modifiable segment.\n");
> > +				return -EINVAL;
> > +			}
> > +
> > +			break;
> > +		}
> > +	if (i == kexec_image->nr_segments) {
> > +		pr_err("Couldn't find segment to update: 0x%lx, size 0x%lx\n",
> > +		       load_addr, memsz);
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
> > +	     entry = *ptr++) {
> > +		void *addr = (void *) (entry & PAGE_MASK);
> > +
> > +		switch (entry & IND_FLAGS) {
> > +		case IND_DESTINATION:
> > +			dest = addr;
> > +			break;
> > +		case IND_INDIRECTION:
> > +			ptr = __va(addr);
> > +			break;
> > +		case IND_SOURCE:
> > +			/* Shouldn't happen, but verify just to be safe. */
> > +			if (dest == NULL) {
> > +				pr_err("Invalid kexec entries list.");
> > +				return -EINVAL;
> > +			}
> > +
> > +			if (dest == (void *) load_addr) {
> > +				struct page *page;
> > +				char *ptr;
> > +				size_t uchunk, mchunk;
> > +
> > +				page = kmap_to_page(addr);
> > +
> > +				ptr = kmap(page);
> 
> kmap_atomic() could be used here, and it is appreciably faster.

Good idea. The patch below implements your suggestion.

This has a consequence for patch 5/6 in this series, because it makes
this code be used in the path of the kexec_file_load and
kexec_load syscalls.

In the latter case, there's a call to copy_from_user and thus kmap_atomic 
can't be used. I can change the patch to use kmap_atomic if
state->from_kernel is true and kmap otherwise, but perhaps this is one more 
hint that patch 5/6 is not a very good idea after all.

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


Subject: [PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.

kexec_update_segment allows a given segment in kexec_image to have
its contents updated. This is useful if the current kernel wants to
send information to the next kernel that is up-to-date at the time of
reboot.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 include/linux/kexec.h |  2 ++
 kernel/kexec_core.c   | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 37eea32fdff1..14dda81e3e01 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -259,6 +259,8 @@ extern int kexec_purgatory_get_set_symbol(struct kimage *image,
 					  unsigned int size, bool get_value);
 extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
 					     const char *name);
+int kexec_update_segment(const char *buffer, unsigned long bufsz,
+			 unsigned long load_addr, unsigned long memsz);
 extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 561675589511..9782b292714e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -721,6 +721,105 @@ static struct page *kimage_alloc_page(struct kimage *image,
 	return page;
 }
 
+/**
+ * kexec_update_segment - update the contents of a kimage segment
+ * @buffer:	New contents of the segment.
+ * @bufsz:	@buffer size.
+ * @load_addr:	Segment's physical address in the next kernel.
+ * @memsz:	Segment size.
+ *
+ * This function assumes kexec_mutex is held.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int kexec_update_segment(const char *buffer, unsigned long bufsz,
+			 unsigned long load_addr, unsigned long memsz)
+{
+	int i;
+	unsigned long entry;
+	unsigned long *ptr = NULL;
+	void *dest = NULL;
+
+	if (kexec_image == NULL) {
+		pr_err("Can't update segment: no kexec image loaded.\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * kexec_add_buffer rounds up segment sizes to PAGE_SIZE, so
+	 * we have to do it here as well.
+	 */
+	memsz = ALIGN(memsz, PAGE_SIZE);
+
+	for (i = 0; i < kexec_image->nr_segments; i++)
+		/* We only support updating whole segments. */
+		if (load_addr == kexec_image->segment[i].mem &&
+		    memsz == kexec_image->segment[i].memsz) {
+			if (kexec_image->segment[i].do_checksum) {
+				pr_err("Trying to update non-modifiable segment.\n");
+				return -EINVAL;
+			}
+
+			break;
+		}
+	if (i == kexec_image->nr_segments) {
+		pr_err("Couldn't find segment to update: 0x%lx, size 0x%lx\n",
+		       load_addr, memsz);
+		return -EINVAL;
+	}
+
+	for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
+	     entry = *ptr++) {
+		void *addr = (void *) (entry & PAGE_MASK);
+
+		switch (entry & IND_FLAGS) {
+		case IND_DESTINATION:
+			dest = addr;
+			break;
+		case IND_INDIRECTION:
+			ptr = __va(addr);
+			break;
+		case IND_SOURCE:
+			/* Shouldn't happen, but verify just to be safe. */
+			if (dest == NULL) {
+				pr_err("Invalid kexec entries list.");
+				return -EINVAL;
+			}
+
+			if (dest == (void *) load_addr) {
+				struct page *page;
+				char *ptr;
+				size_t uchunk, mchunk;
+
+				page = kmap_to_page(addr);
+
+				ptr = kmap_atomic(page);
+				ptr += load_addr & ~PAGE_MASK;
+				mchunk = min_t(size_t, memsz,
+					       PAGE_SIZE - (load_addr & ~PAGE_MASK));
+				uchunk = min(bufsz, mchunk);
+				memcpy(ptr, buffer, uchunk);
+
+				kunmap_atomic(ptr);
+
+				bufsz -= uchunk;
+				load_addr += mchunk;
+				buffer += mchunk;
+				memsz -= mchunk;
+			}
+			dest += PAGE_SIZE;
+		}
+
+		/* Shouldn't happen, but verify just to be safe. */
+		if (ptr == NULL) {
+			pr_err("Invalid kexec entries list.");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int kimage_load_normal_segment(struct kimage *image,
 					 struct kexec_segment *segment)
 {
-- 
1.9.1

Re: [PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.

[Thiago Jung Bauermann]

Hello Andrew,

Thank you for your review!

Am Montag, 15 August 2016, 15:27:56 schrieb Andrew Morton:
> On Sat, 13 Aug 2016 00:18:23 -0300 Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> wrote:
> > +/**
> > + * kexec_update_segment - update the contents of a kimage segment
> > + * @buffer:	New contents of the segment.
> > + * @bufsz:	@buffer size.
> > + * @load_addr:	Segment's physical address in the next kernel.
> > + * @memsz:	Segment size.
> > + *
> > + * This function assumes kexec_mutex is held.
> > + *
> > + * Return: 0 on success, negative errno on error.
> > + */
> > +int kexec_update_segment(const char *buffer, unsigned long bufsz,
> > +			 unsigned long load_addr, unsigned long memsz)
> > +{
> > +	int i;
> > +	unsigned long entry;
> > +	unsigned long *ptr = NULL;
> > +	void *dest = NULL;
> > +
> > +	if (kexec_image == NULL) {
> > +		pr_err("Can't update segment: no kexec image loaded.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/*
> > +	 * kexec_add_buffer rounds up segment sizes to PAGE_SIZE, so
> > +	 * we have to do it here as well.
> > +	 */
> > +	memsz = ALIGN(memsz, PAGE_SIZE);
> > +
> > +	for (i = 0; i < kexec_image->nr_segments; i++)
> > +		/* We only support updating whole segments. */
> > +		if (load_addr == kexec_image->segment[i].mem &&
> > +		    memsz == kexec_image->segment[i].memsz) {
> > +			if (kexec_image->segment[i].do_checksum) {
> > +				pr_err("Trying to update non-modifiable segment.\n");
> > +				return -EINVAL;
> > +			}
> > +
> > +			break;
> > +		}
> > +	if (i == kexec_image->nr_segments) {
> > +		pr_err("Couldn't find segment to update: 0x%lx, size 0x%lx\n",
> > +		       load_addr, memsz);
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
> > +	     entry = *ptr++) {
> > +		void *addr = (void *) (entry & PAGE_MASK);
> > +
> > +		switch (entry & IND_FLAGS) {
> > +		case IND_DESTINATION:
> > +			dest = addr;
> > +			break;
> > +		case IND_INDIRECTION:
> > +			ptr = __va(addr);
> > +			break;
> > +		case IND_SOURCE:
> > +			/* Shouldn't happen, but verify just to be safe. */
> > +			if (dest == NULL) {
> > +				pr_err("Invalid kexec entries list.");
> > +				return -EINVAL;
> > +			}
> > +
> > +			if (dest == (void *) load_addr) {
> > +				struct page *page;
> > +				char *ptr;
> > +				size_t uchunk, mchunk;
> > +
> > +				page = kmap_to_page(addr);
> > +
> > +				ptr = kmap(page);
> 
> kmap_atomic() could be used here, and it is appreciably faster.

Good idea. The patch below implements your suggestion.

This has a consequence for patch 5/6 in this series, because it makes
this code be used in the path of the kexec_file_load and
kexec_load syscalls.

In the latter case, there's a call to copy_from_user and thus kmap_atomic 
can't be used. I can change the patch to use kmap_atomic if
state->from_kernel is true and kmap otherwise, but perhaps this is one more 
hint that patch 5/6 is not a very good idea after all.

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


Subject: [PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.

kexec_update_segment allows a given segment in kexec_image to have
its contents updated. This is useful if the current kernel wants to
send information to the next kernel that is up-to-date at the time of
reboot.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 include/linux/kexec.h |  2 ++
 kernel/kexec_core.c   | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 37eea32fdff1..14dda81e3e01 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -259,6 +259,8 @@ extern int kexec_purgatory_get_set_symbol(struct kimage *image,
 					  unsigned int size, bool get_value);
 extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
 					     const char *name);
+int kexec_update_segment(const char *buffer, unsigned long bufsz,
+			 unsigned long load_addr, unsigned long memsz);
 extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 561675589511..9782b292714e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -721,6 +721,105 @@ static struct page *kimage_alloc_page(struct kimage *image,
 	return page;
 }
 
+/**
+ * kexec_update_segment - update the contents of a kimage segment
+ * @buffer:	New contents of the segment.
+ * @bufsz:	@buffer size.
+ * @load_addr:	Segment's physical address in the next kernel.
+ * @memsz:	Segment size.
+ *
+ * This function assumes kexec_mutex is held.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int kexec_update_segment(const char *buffer, unsigned long bufsz,
+			 unsigned long load_addr, unsigned long memsz)
+{
+	int i;
+	unsigned long entry;
+	unsigned long *ptr = NULL;
+	void *dest = NULL;
+
+	if (kexec_image == NULL) {
+		pr_err("Can't update segment: no kexec image loaded.\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * kexec_add_buffer rounds up segment sizes to PAGE_SIZE, so
+	 * we have to do it here as well.
+	 */
+	memsz = ALIGN(memsz, PAGE_SIZE);
+
+	for (i = 0; i < kexec_image->nr_segments; i++)
+		/* We only support updating whole segments. */
+		if (load_addr == kexec_image->segment[i].mem &&
+		    memsz == kexec_image->segment[i].memsz) {
+			if (kexec_image->segment[i].do_checksum) {
+				pr_err("Trying to update non-modifiable segment.\n");
+				return -EINVAL;
+			}
+
+			break;
+		}
+	if (i == kexec_image->nr_segments) {
+		pr_err("Couldn't find segment to update: 0x%lx, size 0x%lx\n",
+		       load_addr, memsz);
+		return -EINVAL;
+	}
+
+	for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
+	     entry = *ptr++) {
+		void *addr = (void *) (entry & PAGE_MASK);
+
+		switch (entry & IND_FLAGS) {
+		case IND_DESTINATION:
+			dest = addr;
+			break;
+		case IND_INDIRECTION:
+			ptr = __va(addr);
+			break;
+		case IND_SOURCE:
+			/* Shouldn't happen, but verify just to be safe. */
+			if (dest == NULL) {
+				pr_err("Invalid kexec entries list.");
+				return -EINVAL;
+			}
+
+			if (dest == (void *) load_addr) {
+				struct page *page;
+				char *ptr;
+				size_t uchunk, mchunk;
+
+				page = kmap_to_page(addr);
+
+				ptr = kmap_atomic(page);
+				ptr += load_addr & ~PAGE_MASK;
+				mchunk = min_t(size_t, memsz,
+					       PAGE_SIZE - (load_addr & ~PAGE_MASK));
+				uchunk = min(bufsz, mchunk);
+				memcpy(ptr, buffer, uchunk);
+
+				kunmap_atomic(ptr);
+
+				bufsz -= uchunk;
+				load_addr += mchunk;
+				buffer += mchunk;
+				memsz -= mchunk;
+			}
+			dest += PAGE_SIZE;
+		}
+
+		/* Shouldn't happen, but verify just to be safe. */
+		if (ptr == NULL) {
+			pr_err("Invalid kexec entries list.");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int kimage_load_normal_segment(struct kimage *image,
 					 struct kexec_segment *segment)
 {
-- 
1.9.1



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 0/6] kexec_file: Add buffer hand-over for the next kernel

[Dave Young]

On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> Hello,
> 
> This patch series implements a mechanism which allows the kernel to pass
> on a buffer to the kernel that will be kexec'd. This buffer is passed
> as a segment which is added to the kimage when it is being prepared
> by kexec_file_load.
> 
> How the second kernel is informed of this buffer is architecture-specific.
> On powerpc, this is done via the device tree, by checking
> the properties /chosen/linux,kexec-handover-buffer-start and
> /chosen/linux,kexec-handover-buffer-end, which is analogous to how the
> kernel finds the initrd.
> 
> This is needed because the Integrity Measurement Architecture subsystem
> needs to preserve its measurement list accross the kexec reboot. The
> following patch series for the IMA subsystem uses this feature for that
> purpose:
> 
> https://lists.infradead.org/pipermail/kexec/2016-August/016745.html
> 
> This is so that IMA can implement trusted boot support on the OpenPower
> platform, because on such systems an intermediary Linux instance running
> as part of the firmware is used to boot the target operating system via
> kexec. Using this mechanism, IMA on this intermediary instance can
> hand over to the target OS the measurements of the components that were
> used to boot it.
> 
> Because there could be additional measurement events between the
> kexec_file_load call and the actual reboot, IMA needs a way to update the
> buffer with those additional events before rebooting. One can minimize
> the interval between the kexec_file_load and the reboot syscalls, but as
> small as it can be, there is always the possibility that the measurement
> list will be out of date at the time of reboot.
> 
> To address this issue, this patch series also introduces
> kexec_update_segment, which allows a reboot notifier to change the
> contents of the image segment during the reboot process.
> 
> Patch 5 makes kimage_load_normal_segment and kexec_update_segment share
> code. It's not much code that they can share though, so I'm not sure if
> the result is actually better.
> 
> The last patch is not intended to be merged, it just demonstrates how
> this feature can be used.
> 
> This series applies on top of v5 of the "kexec_file_load implementation
> for PowerPC" patch series (which applies on top of v4.8-rc1):
> 
> https://lists.infradead.org/pipermail/kexec/2016-August/016843.html

I'm trying to review your patches, but seems I can not apply them
cleanly to mainline kernel or v4.8-rc1

Apply the kexec_file_load series failed as below on v4.8-rc1:

Applying: kexec_file: Allow arch-specific memory walking for
kexec_add_buffer
error: patch failed: include/linux/kexec.h:149
error: include/linux/kexec.h: patch does not apply
Patch failed at 0001 kexec_file: Allow arch-specific memory walking for
kexec_add_buffer
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

What is the order of your patch series of the three patchset?

[PATCH v2 0/2] extend kexec_file_load system call
[PATCH v5 00/13] kexec_file_load implementation for PowerPC
[PATCH v2 0/6] kexec_file: Add buffer hand-over for the next kernel

Do they depend on other patches?

Thanks
Dave

Re: [PATCH v2 0/6] kexec_file: Add buffer hand-over for the next kernel

[Dave Young]

On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> Hello,
> 
> This patch series implements a mechanism which allows the kernel to pass
> on a buffer to the kernel that will be kexec'd. This buffer is passed
> as a segment which is added to the kimage when it is being prepared
> by kexec_file_load.
> 
> How the second kernel is informed of this buffer is architecture-specific.
> On powerpc, this is done via the device tree, by checking
> the properties /chosen/linux,kexec-handover-buffer-start and
> /chosen/linux,kexec-handover-buffer-end, which is analogous to how the
> kernel finds the initrd.
> 
> This is needed because the Integrity Measurement Architecture subsystem
> needs to preserve its measurement list accross the kexec reboot. The
> following patch series for the IMA subsystem uses this feature for that
> purpose:
> 
> https://lists.infradead.org/pipermail/kexec/2016-August/016745.html
> 
> This is so that IMA can implement trusted boot support on the OpenPower
> platform, because on such systems an intermediary Linux instance running
> as part of the firmware is used to boot the target operating system via
> kexec. Using this mechanism, IMA on this intermediary instance can
> hand over to the target OS the measurements of the components that were
> used to boot it.
> 
> Because there could be additional measurement events between the
> kexec_file_load call and the actual reboot, IMA needs a way to update the
> buffer with those additional events before rebooting. One can minimize
> the interval between the kexec_file_load and the reboot syscalls, but as
> small as it can be, there is always the possibility that the measurement
> list will be out of date at the time of reboot.
> 
> To address this issue, this patch series also introduces
> kexec_update_segment, which allows a reboot notifier to change the
> contents of the image segment during the reboot process.
> 
> Patch 5 makes kimage_load_normal_segment and kexec_update_segment share
> code. It's not much code that they can share though, so I'm not sure if
> the result is actually better.
> 
> The last patch is not intended to be merged, it just demonstrates how
> this feature can be used.
> 
> This series applies on top of v5 of the "kexec_file_load implementation
> for PowerPC" patch series (which applies on top of v4.8-rc1):
> 
> https://lists.infradead.org/pipermail/kexec/2016-August/016843.html

I'm trying to review your patches, but seems I can not apply them
cleanly to mainline kernel or v4.8-rc1

Apply the kexec_file_load series failed as below on v4.8-rc1:

Applying: kexec_file: Allow arch-specific memory walking for
kexec_add_buffer
error: patch failed: include/linux/kexec.h:149
error: include/linux/kexec.h: patch does not apply
Patch failed at 0001 kexec_file: Allow arch-specific memory walking for
kexec_add_buffer
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

What is the order of your patch series of the three patchset?

[PATCH v2 0/2] extend kexec_file_load system call
[PATCH v5 00/13] kexec_file_load implementation for PowerPC
[PATCH v2 0/6] kexec_file: Add buffer hand-over for the next kernel

Do they depend on other patches?

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 0/6] kexec_file: Add buffer hand-over for the next kernel

[Thiago Jung Bauermann]

Hello Dave,

Am Mittwoch, 17 August 2016, 10:52:26 schrieb Dave Young:
> On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> > This series applies on top of v5 of the "kexec_file_load implementation
> > for PowerPC" patch series (which applies on top of v4.8-rc1):
> > 
> > https://lists.infradead.org/pipermail/kexec/2016-August/016843.html
> 
> I'm trying to review your patches, but seems I can not apply them
> cleanly to mainline kernel or v4.8-rc1

Strange, I just did a test using the patches I received via the kexec 
mailing list, and git am applied them cleanly on v4.8-rc1.

> Apply the kexec_file_load series failed as below on v4.8-rc1:
> 
> Applying: kexec_file: Allow arch-specific memory walking for
> kexec_add_buffer
> error: patch failed: include/linux/kexec.h:149
> error: include/linux/kexec.h: patch does not apply
> Patch failed at 0001 kexec_file: Allow arch-specific memory walking for
> kexec_add_buffer
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> What is the order of your patch series of the three patchset?
> 
> [PATCH v2 0/2] extend kexec_file_load system call
> [PATCH v5 00/13] kexec_file_load implementation for PowerPC
> [PATCH v2 0/6] kexec_file: Add buffer hand-over for the next kernel

Yes, that is correct.

> Do they depend on other patches?

No, they apply directly on v4.8-rc1.

I just published a branch with the patches, if you want you can use that 
instead. The branch is called kexec-patches and is at the repo 
git@github.com:bauermann/linux

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

Re: [PATCH v2 0/6] kexec_file: Add buffer hand-over for the next kernel

[Thiago Jung Bauermann]

Hello Dave,

Am Mittwoch, 17 August 2016, 10:52:26 schrieb Dave Young:
> On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> > This series applies on top of v5 of the "kexec_file_load implementation
> > for PowerPC" patch series (which applies on top of v4.8-rc1):
> > 
> > https://lists.infradead.org/pipermail/kexec/2016-August/016843.html
> 
> I'm trying to review your patches, but seems I can not apply them
> cleanly to mainline kernel or v4.8-rc1

Strange, I just did a test using the patches I received via the kexec 
mailing list, and git am applied them cleanly on v4.8-rc1.

> Apply the kexec_file_load series failed as below on v4.8-rc1:
> 
> Applying: kexec_file: Allow arch-specific memory walking for
> kexec_add_buffer
> error: patch failed: include/linux/kexec.h:149
> error: include/linux/kexec.h: patch does not apply
> Patch failed at 0001 kexec_file: Allow arch-specific memory walking for
> kexec_add_buffer
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> What is the order of your patch series of the three patchset?
> 
> [PATCH v2 0/2] extend kexec_file_load system call
> [PATCH v5 00/13] kexec_file_load implementation for PowerPC
> [PATCH v2 0/6] kexec_file: Add buffer hand-over for the next kernel

Yes, that is correct.

> Do they depend on other patches?

No, they apply directly on v4.8-rc1.

I just published a branch with the patches, if you want you can use that 
instead. The branch is called kexec-patches and is at the repo 
git@github.com:bauermann/linux

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


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Dave Young]

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

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

[Dave Young]

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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Thiago Jung Bauermann]

Hello Dave,

Thanks for your review!

[ Trimming down Cc: list a little to try to clear the "too many recipients"   
  mailing list restriction. ]

Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> 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?

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?

> 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.

-- 
[]'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.

Adjust places using struct kexec_buf to set skip_checksum.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/kexec_elf_64.c |  5 +++--
 arch/x86/kernel/crash.c            |  3 ++-
 arch/x86/kernel/kexec-bzimage64.c  |  2 +-
 include/linux/kexec.h              | 23 ++++++++++++++---------
 kernel/kexec_file.c                | 15 +++++++--------
 5 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c
index 22afc7b5ee73..d009f5363968 100644
--- a/arch/powerpc/kernel/kexec_elf_64.c
+++ b/arch/powerpc/kernel/kexec_elf_64.c
@@ -107,7 +107,7 @@ static int elf_exec_load(struct kimage *image, struct elfhdr *ehdr,
 	int ret;
 	size_t i;
 	struct kexec_buf kbuf = { .image = image, .buf_max = ppc64_rma_size,
-				  .top_down = false };
+				  .top_down = false, .skip_checksum = false };
 
 	/* Read in the PT_LOAD segments. */
 	for (i = 0; i < ehdr->e_phnum; i++) {
@@ -162,7 +162,8 @@ void *elf64_load(struct kimage *image, char *kernel_buf,
 	struct elf_info elf_info;
 	struct fdt_reserve_entry *rsvmap;
 	struct kexec_buf kbuf = { .image = image, .buf_min = 0,
-				  .buf_max = ppc64_rma_size };
+				  .buf_max = ppc64_rma_size,
+				  .skip_checksum = false };
 
 	ret = build_elf_exec_info(kernel_buf, kernel_len, &ehdr, &elf_info);
 	if (ret)
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 38a1cdf6aa05..7b8f62c86651 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -617,7 +617,8 @@ int crash_load_segments(struct kimage *image)
 {
 	int ret;
 	struct kexec_buf kbuf = { .image = image, .buf_min = 0,
-				  .buf_max = ULONG_MAX, .top_down = false };
+				  .buf_max = ULONG_MAX, .top_down = false,
+				  .skip_checksum = false };
 
 	/*
 	 * Determine and load a segment for backup area. First 640K RAM
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 4b3a75329fb6..449f433cd225 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -341,7 +341,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
 	unsigned int setup_hdr_offset = offsetof(struct boot_params, hdr);
 	unsigned int efi_map_offset, efi_map_sz, efi_setup_data_offset;
 	struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX,
-				  .top_down = true };
+				  .top_down = true, .skip_checksum = false };
 
 	header = (struct setup_header *)(kernel + setup_hdr_offset);
 	setup_sects = header->setup_sects;
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

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

[Thiago Jung Bauermann]

Hello Dave,

Thanks for your review!

[ Trimming down Cc: list a little to try to clear the "too many recipients"   
  mailing list restriction. ]

Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> 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?

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?

> 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.

-- 
[]'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.

Adjust places using struct kexec_buf to set skip_checksum.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/kexec_elf_64.c |  5 +++--
 arch/x86/kernel/crash.c            |  3 ++-
 arch/x86/kernel/kexec-bzimage64.c  |  2 +-
 include/linux/kexec.h              | 23 ++++++++++++++---------
 kernel/kexec_file.c                | 15 +++++++--------
 5 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c
index 22afc7b5ee73..d009f5363968 100644
--- a/arch/powerpc/kernel/kexec_elf_64.c
+++ b/arch/powerpc/kernel/kexec_elf_64.c
@@ -107,7 +107,7 @@ static int elf_exec_load(struct kimage *image, struct elfhdr *ehdr,
 	int ret;
 	size_t i;
 	struct kexec_buf kbuf = { .image = image, .buf_max = ppc64_rma_size,
-				  .top_down = false };
+				  .top_down = false, .skip_checksum = false };
 
 	/* Read in the PT_LOAD segments. */
 	for (i = 0; i < ehdr->e_phnum; i++) {
@@ -162,7 +162,8 @@ void *elf64_load(struct kimage *image, char *kernel_buf,
 	struct elf_info elf_info;
 	struct fdt_reserve_entry *rsvmap;
 	struct kexec_buf kbuf = { .image = image, .buf_min = 0,
-				  .buf_max = ppc64_rma_size };
+				  .buf_max = ppc64_rma_size,
+				  .skip_checksum = false };
 
 	ret = build_elf_exec_info(kernel_buf, kernel_len, &ehdr, &elf_info);
 	if (ret)
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 38a1cdf6aa05..7b8f62c86651 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -617,7 +617,8 @@ int crash_load_segments(struct kimage *image)
 {
 	int ret;
 	struct kexec_buf kbuf = { .image = image, .buf_min = 0,
-				  .buf_max = ULONG_MAX, .top_down = false };
+				  .buf_max = ULONG_MAX, .top_down = false,
+				  .skip_checksum = false };
 
 	/*
 	 * Determine and load a segment for backup area. First 640K RAM
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 4b3a75329fb6..449f433cd225 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -341,7 +341,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
 	unsigned int setup_hdr_offset = offsetof(struct boot_params, hdr);
 	unsigned int efi_map_offset, efi_map_sz, efi_setup_data_offset;
 	struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX,
-				  .top_down = true };
+				  .top_down = true, .skip_checksum = false };
 
 	header = (struct setup_header *)(kernel + setup_hdr_offset);
 	setup_sects = header->setup_sects;
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



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Dave Young]

On 08/18/16 at 06:09pm, Thiago Jung Bauermann wrote:
> Hello Dave,
> 
> Thanks for your review!
> 
> [ Trimming down Cc: list a little to try to clear the "too many recipients"   
>   mailing list restriction. ]

I also got "too many recipients".. Thanks for the trimming.

> 
> Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> > 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?
> 
> 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?
> 
> > 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.
> 
> -- 
> []'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.
> 
> Adjust places using struct kexec_buf to set skip_checksum.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/kexec_elf_64.c |  5 +++--
>  arch/x86/kernel/crash.c            |  3 ++-
>  arch/x86/kernel/kexec-bzimage64.c  |  2 +-
>  include/linux/kexec.h              | 23 ++++++++++++++---------
>  kernel/kexec_file.c                | 15 +++++++--------
>  5 files changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c
> index 22afc7b5ee73..d009f5363968 100644
> --- a/arch/powerpc/kernel/kexec_elf_64.c
> +++ b/arch/powerpc/kernel/kexec_elf_64.c
> @@ -107,7 +107,7 @@ static int elf_exec_load(struct kimage *image, struct elfhdr *ehdr,
>  	int ret;
>  	size_t i;
>  	struct kexec_buf kbuf = { .image = image, .buf_max = ppc64_rma_size,
> -				  .top_down = false };
> +				  .top_down = false, .skip_checksum = false };

No need to set it as false because it will be initialized to 0 by
default?

>  
>  	/* Read in the PT_LOAD segments. */
>  	for (i = 0; i < ehdr->e_phnum; i++) {
> @@ -162,7 +162,8 @@ void *elf64_load(struct kimage *image, char *kernel_buf,
>  	struct elf_info elf_info;
>  	struct fdt_reserve_entry *rsvmap;
>  	struct kexec_buf kbuf = { .image = image, .buf_min = 0,
> -				  .buf_max = ppc64_rma_size };
> +				  .buf_max = ppc64_rma_size,
> +				  .skip_checksum = false };
>  
>  	ret = build_elf_exec_info(kernel_buf, kernel_len, &ehdr, &elf_info);
>  	if (ret)
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 38a1cdf6aa05..7b8f62c86651 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -617,7 +617,8 @@ int crash_load_segments(struct kimage *image)
>  {
>  	int ret;
>  	struct kexec_buf kbuf = { .image = image, .buf_min = 0,
> -				  .buf_max = ULONG_MAX, .top_down = false };
> +				  .buf_max = ULONG_MAX, .top_down = false,
> +				  .skip_checksum = false };
>  
>  	/*
>  	 * Determine and load a segment for backup area. First 640K RAM
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 4b3a75329fb6..449f433cd225 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -341,7 +341,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
>  	unsigned int setup_hdr_offset = offsetof(struct boot_params, hdr);
>  	unsigned int efi_map_offset, efi_map_sz, efi_setup_data_offset;
>  	struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX,
> -				  .top_down = true };
> +				  .top_down = true, .skip_checksum = false };
>  
>  	header = (struct setup_header *)(kernel + setup_hdr_offset);
>  	setup_sects = header->setup_sects;
> 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
> 
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

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

[Dave Young]

On 08/18/16 at 06:09pm, Thiago Jung Bauermann wrote:
> Hello Dave,
> 
> Thanks for your review!
> 
> [ Trimming down Cc: list a little to try to clear the "too many recipients"   
>   mailing list restriction. ]

I also got "too many recipients".. Thanks for the trimming.

> 
> Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> > 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?
> 
> 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?
> 
> > 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.
> 
> -- 
> []'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.
> 
> Adjust places using struct kexec_buf to set skip_checksum.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/kexec_elf_64.c |  5 +++--
>  arch/x86/kernel/crash.c            |  3 ++-
>  arch/x86/kernel/kexec-bzimage64.c  |  2 +-
>  include/linux/kexec.h              | 23 ++++++++++++++---------
>  kernel/kexec_file.c                | 15 +++++++--------
>  5 files changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c
> index 22afc7b5ee73..d009f5363968 100644
> --- a/arch/powerpc/kernel/kexec_elf_64.c
> +++ b/arch/powerpc/kernel/kexec_elf_64.c
> @@ -107,7 +107,7 @@ static int elf_exec_load(struct kimage *image, struct elfhdr *ehdr,
>  	int ret;
>  	size_t i;
>  	struct kexec_buf kbuf = { .image = image, .buf_max = ppc64_rma_size,
> -				  .top_down = false };
> +				  .top_down = false, .skip_checksum = false };

No need to set it as false because it will be initialized to 0 by
default?

>  
>  	/* Read in the PT_LOAD segments. */
>  	for (i = 0; i < ehdr->e_phnum; i++) {
> @@ -162,7 +162,8 @@ void *elf64_load(struct kimage *image, char *kernel_buf,
>  	struct elf_info elf_info;
>  	struct fdt_reserve_entry *rsvmap;
>  	struct kexec_buf kbuf = { .image = image, .buf_min = 0,
> -				  .buf_max = ppc64_rma_size };
> +				  .buf_max = ppc64_rma_size,
> +				  .skip_checksum = false };
>  
>  	ret = build_elf_exec_info(kernel_buf, kernel_len, &ehdr, &elf_info);
>  	if (ret)
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 38a1cdf6aa05..7b8f62c86651 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -617,7 +617,8 @@ int crash_load_segments(struct kimage *image)
>  {
>  	int ret;
>  	struct kexec_buf kbuf = { .image = image, .buf_min = 0,
> -				  .buf_max = ULONG_MAX, .top_down = false };
> +				  .buf_max = ULONG_MAX, .top_down = false,
> +				  .skip_checksum = false };
>  
>  	/*
>  	 * Determine and load a segment for backup area. First 640K RAM
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 4b3a75329fb6..449f433cd225 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -341,7 +341,7 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
>  	unsigned int setup_hdr_offset = offsetof(struct boot_params, hdr);
>  	unsigned int efi_map_offset, efi_map_sz, efi_setup_data_offset;
>  	struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX,
> -				  .top_down = true };
> +				  .top_down = true, .skip_checksum = false };
>  
>  	header = (struct setup_header *)(kernel + setup_hdr_offset);
>  	setup_sects = header->setup_sects;
> 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
> 
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 2/6] powerpc: kexec_file: Add buffer hand-over support for the next kernel

[Dave Young]

On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> The buffer hand-over mechanism allows the currently running kernel to pass
> data to kernel that will be kexec'd via a kexec segment. The second kernel
> can check whether the previous kernel sent data and retrieve it.
> 
> This is the architecture-specific part.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/kexec.h       |  12 +++-
>  arch/powerpc/kernel/kexec_elf_64.c     |   2 +-
>  arch/powerpc/kernel/machine_kexec_64.c | 114 +++++++++++++++++++++++++++++++--
>  3 files changed, 120 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index 31bc64e07c8f..b20738df26f8 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -92,12 +92,20 @@ static inline bool kdump_in_progress(void)
>  }
>  
>  #ifdef CONFIG_KEXEC_FILE
> +#define ARCH_HAS_KIMAGE_ARCH
> +
> +struct kimage_arch {
> +	phys_addr_t handover_buffer_addr;
> +	unsigned long handover_buffer_size;
> +};
> +
>  int setup_purgatory(struct kimage *image, const void *slave_code,
>  		    const void *fdt, unsigned long kernel_load_addr,
>  		    unsigned long fdt_load_addr, unsigned long stack_top,
>  		    int debug);
> -int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
> -		  unsigned long initrd_len, const char *cmdline);
> +int setup_new_fdt(const struct kimage *image, void *fdt,
> +		  unsigned long initrd_load_addr, unsigned long initrd_len,
> +		  const char *cmdline);
>  bool find_debug_console(const void *fdt, int chosen_node);
>  int merge_partial_dtb(void *to, const void *from);
>  #endif /* CONFIG_KEXEC_FILE */
> diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c
> index 1b902ad66e2a..22afc7b5ee73 100644
> --- a/arch/powerpc/kernel/kexec_elf_64.c
> +++ b/arch/powerpc/kernel/kexec_elf_64.c
> @@ -219,7 +219,7 @@ void *elf64_load(struct kimage *image, char *kernel_buf,
>  		}
>  	}
>  
> -	ret = setup_new_fdt(fdt, initrd_load_addr, initrd_len, cmdline);
> +	ret = setup_new_fdt(image, fdt, initrd_load_addr, initrd_len, cmdline);
>  	if (ret)
>  		goto out;
>  
> diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
> index a484a6346146..190c652e49b7 100644
> --- a/arch/powerpc/kernel/machine_kexec_64.c
> +++ b/arch/powerpc/kernel/machine_kexec_64.c
> @@ -490,6 +490,60 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
>  	return image->fops->cleanup(image->image_loader_data);
>  }
>  
> +bool kexec_can_hand_over_buffer(void)
> +{
> +	return true;
> +}
> +
> +int arch_kexec_add_handover_buffer(struct kimage *image,
> +				   unsigned long load_addr, unsigned long size)
> +{
> +	image->arch.handover_buffer_addr = load_addr;
> +	image->arch.handover_buffer_size = size;
> +
> +	return 0;
> +}
> +
> +int kexec_get_handover_buffer(void **addr, unsigned long *size)
> +{
> +	int ret;
> +	u64 start_addr, end_addr;
> +
> +	ret = of_property_read_u64(of_chosen,
> +				   "linux,kexec-handover-buffer-start",
> +				   &start_addr);
> +	if (ret == -EINVAL)
> +		return -ENOENT;
> +	else if (ret)
> +		return -EINVAL;
> +
> +	ret = of_property_read_u64(of_chosen, "linux,kexec-handover-buffer-end",
> +				   &end_addr);
> +	if (ret == -EINVAL)
> +		return -ENOENT;
> +	else if (ret)
> +		return -EINVAL;
> +
> +	*addr =  __va(start_addr);
> +	/* -end is the first address after the buffer. */
> +	*size = end_addr - start_addr;
> +
> +	return 0;
> +}

This depends on dtb, so if IMA want to extend it to arches like x86 in
the future you will have to think about other way to pass it.

How about think about a general way now?

> +
> +int kexec_free_handover_buffer(void)
> +{
> +	int ret;
> +	void *addr;
> +	unsigned long size;
> +
> +	ret = kexec_get_handover_buffer(&addr, &size);
> +	if (ret)
> +		return ret;
> +
> +	return memblock_free((phys_addr_t) addr, size);
> +}
> +
>  /**
>   * arch_kexec_walk_mem() - call func(data) for each unreserved memory block
>   * @kbuf:	Context info for the search. Also passed to @func.
> @@ -687,9 +741,52 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
>  	return 0;
>  }
>  
> -/*
> - * setup_new_fdt() - modify /chosen and memory reservation for the next kernel
> - * @fdt:
> +/**
> + * setup_handover_buffer() - add properties and reservation for the handover buffer
> + * @image:		kexec image being loaded.
> + * @fdt:		Flattened device tree for the next kernel.
> + * @chosen_node:	Offset to the chosen node.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +static int setup_handover_buffer(const struct kimage *image, void *fdt,
> +				 int chosen_node)
> +{
> +	int ret;
> +
> +	if (image->arch.handover_buffer_addr == 0)
> +		return 0;
> +
> +	ret = fdt_setprop_u64(fdt, chosen_node,
> +			      "linux,kexec-handover-buffer-start",
> +			      image->arch.handover_buffer_addr);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	/* -end is the first address after the buffer. */
> +	ret = fdt_setprop_u64(fdt, chosen_node,
> +			      "linux,kexec-handover-buffer-end",
> +			      image->arch.handover_buffer_addr +
> +			      image->arch.handover_buffer_size);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	ret = fdt_add_mem_rsv(fdt, image->arch.handover_buffer_addr,
> +			      image->arch.handover_buffer_size);
> +	if (ret)
> +		return -EINVAL;
> +
> +	pr_debug("kexec handover buffer at 0x%llx, size = 0x%lx\n",
> +		 image->arch.handover_buffer_addr,
> +		 image->arch.handover_buffer_size);
> +
> +	return 0;
> +}
> +
> +/**
> + * setup_new_fdt() - modify /chosen and memory reservations for the next kernel
> + * @image:		kexec image being loaded.
> + * @fdt:		Flattened device tree for the next kernel.
>   * @initrd_load_addr:	Address where the next initrd will be loaded.
>   * @initrd_len:		Size of the next initrd, or 0 if there will be none.
>   * @cmdline:		Command line for the next kernel, or NULL if there will
> @@ -697,8 +794,9 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
>   *
>   * Return: 0 on success, or negative errno on error.
>   */
> -int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
> -		  unsigned long initrd_len, const char *cmdline)
> +int setup_new_fdt(const struct kimage *image, void *fdt,
> +		  unsigned long initrd_load_addr, unsigned long initrd_len,
> +		  const char *cmdline)
>  {
>  	uint64_t oldfdt_addr;
>  	int i, ret, chosen_node;
> @@ -847,6 +945,12 @@ int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
>  		}
>  	}
>  
> +	ret = setup_handover_buffer(image, fdt, chosen_node);
> +	if (ret) {
> +		pr_err("Error setting up the new device tree.\n");
> +		return ret;
> +	}
> +
>  	ret = fdt_setprop(fdt, chosen_node, "linux,booted-from-kexec", NULL, 0);
>  	if (ret) {
>  		pr_err("Error setting up the new device tree.\n");
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 2/6] powerpc: kexec_file: Add buffer hand-over support for the next kernel

[Dave Young]

On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> The buffer hand-over mechanism allows the currently running kernel to pass
> data to kernel that will be kexec'd via a kexec segment. The second kernel
> can check whether the previous kernel sent data and retrieve it.
> 
> This is the architecture-specific part.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/kexec.h       |  12 +++-
>  arch/powerpc/kernel/kexec_elf_64.c     |   2 +-
>  arch/powerpc/kernel/machine_kexec_64.c | 114 +++++++++++++++++++++++++++++++--
>  3 files changed, 120 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index 31bc64e07c8f..b20738df26f8 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -92,12 +92,20 @@ static inline bool kdump_in_progress(void)
>  }
>  
>  #ifdef CONFIG_KEXEC_FILE
> +#define ARCH_HAS_KIMAGE_ARCH
> +
> +struct kimage_arch {
> +	phys_addr_t handover_buffer_addr;
> +	unsigned long handover_buffer_size;
> +};
> +
>  int setup_purgatory(struct kimage *image, const void *slave_code,
>  		    const void *fdt, unsigned long kernel_load_addr,
>  		    unsigned long fdt_load_addr, unsigned long stack_top,
>  		    int debug);
> -int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
> -		  unsigned long initrd_len, const char *cmdline);
> +int setup_new_fdt(const struct kimage *image, void *fdt,
> +		  unsigned long initrd_load_addr, unsigned long initrd_len,
> +		  const char *cmdline);
>  bool find_debug_console(const void *fdt, int chosen_node);
>  int merge_partial_dtb(void *to, const void *from);
>  #endif /* CONFIG_KEXEC_FILE */
> diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c
> index 1b902ad66e2a..22afc7b5ee73 100644
> --- a/arch/powerpc/kernel/kexec_elf_64.c
> +++ b/arch/powerpc/kernel/kexec_elf_64.c
> @@ -219,7 +219,7 @@ void *elf64_load(struct kimage *image, char *kernel_buf,
>  		}
>  	}
>  
> -	ret = setup_new_fdt(fdt, initrd_load_addr, initrd_len, cmdline);
> +	ret = setup_new_fdt(image, fdt, initrd_load_addr, initrd_len, cmdline);
>  	if (ret)
>  		goto out;
>  
> diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
> index a484a6346146..190c652e49b7 100644
> --- a/arch/powerpc/kernel/machine_kexec_64.c
> +++ b/arch/powerpc/kernel/machine_kexec_64.c
> @@ -490,6 +490,60 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
>  	return image->fops->cleanup(image->image_loader_data);
>  }
>  
> +bool kexec_can_hand_over_buffer(void)
> +{
> +	return true;
> +}
> +
> +int arch_kexec_add_handover_buffer(struct kimage *image,
> +				   unsigned long load_addr, unsigned long size)
> +{
> +	image->arch.handover_buffer_addr = load_addr;
> +	image->arch.handover_buffer_size = size;
> +
> +	return 0;
> +}
> +
> +int kexec_get_handover_buffer(void **addr, unsigned long *size)
> +{
> +	int ret;
> +	u64 start_addr, end_addr;
> +
> +	ret = of_property_read_u64(of_chosen,
> +				   "linux,kexec-handover-buffer-start",
> +				   &start_addr);
> +	if (ret == -EINVAL)
> +		return -ENOENT;
> +	else if (ret)
> +		return -EINVAL;
> +
> +	ret = of_property_read_u64(of_chosen, "linux,kexec-handover-buffer-end",
> +				   &end_addr);
> +	if (ret == -EINVAL)
> +		return -ENOENT;
> +	else if (ret)
> +		return -EINVAL;
> +
> +	*addr =  __va(start_addr);
> +	/* -end is the first address after the buffer. */
> +	*size = end_addr - start_addr;
> +
> +	return 0;
> +}

This depends on dtb, so if IMA want to extend it to arches like x86 in
the future you will have to think about other way to pass it.

How about think about a general way now?

> +
> +int kexec_free_handover_buffer(void)
> +{
> +	int ret;
> +	void *addr;
> +	unsigned long size;
> +
> +	ret = kexec_get_handover_buffer(&addr, &size);
> +	if (ret)
> +		return ret;
> +
> +	return memblock_free((phys_addr_t) addr, size);
> +}
> +
>  /**
>   * arch_kexec_walk_mem() - call func(data) for each unreserved memory block
>   * @kbuf:	Context info for the search. Also passed to @func.
> @@ -687,9 +741,52 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
>  	return 0;
>  }
>  
> -/*
> - * setup_new_fdt() - modify /chosen and memory reservation for the next kernel
> - * @fdt:
> +/**
> + * setup_handover_buffer() - add properties and reservation for the handover buffer
> + * @image:		kexec image being loaded.
> + * @fdt:		Flattened device tree for the next kernel.
> + * @chosen_node:	Offset to the chosen node.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +static int setup_handover_buffer(const struct kimage *image, void *fdt,
> +				 int chosen_node)
> +{
> +	int ret;
> +
> +	if (image->arch.handover_buffer_addr == 0)
> +		return 0;
> +
> +	ret = fdt_setprop_u64(fdt, chosen_node,
> +			      "linux,kexec-handover-buffer-start",
> +			      image->arch.handover_buffer_addr);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	/* -end is the first address after the buffer. */
> +	ret = fdt_setprop_u64(fdt, chosen_node,
> +			      "linux,kexec-handover-buffer-end",
> +			      image->arch.handover_buffer_addr +
> +			      image->arch.handover_buffer_size);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	ret = fdt_add_mem_rsv(fdt, image->arch.handover_buffer_addr,
> +			      image->arch.handover_buffer_size);
> +	if (ret)
> +		return -EINVAL;
> +
> +	pr_debug("kexec handover buffer at 0x%llx, size = 0x%lx\n",
> +		 image->arch.handover_buffer_addr,
> +		 image->arch.handover_buffer_size);
> +
> +	return 0;
> +}
> +
> +/**
> + * setup_new_fdt() - modify /chosen and memory reservations for the next kernel
> + * @image:		kexec image being loaded.
> + * @fdt:		Flattened device tree for the next kernel.
>   * @initrd_load_addr:	Address where the next initrd will be loaded.
>   * @initrd_len:		Size of the next initrd, or 0 if there will be none.
>   * @cmdline:		Command line for the next kernel, or NULL if there will
> @@ -697,8 +794,9 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
>   *
>   * Return: 0 on success, or negative errno on error.
>   */
> -int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
> -		  unsigned long initrd_len, const char *cmdline)
> +int setup_new_fdt(const struct kimage *image, void *fdt,
> +		  unsigned long initrd_load_addr, unsigned long initrd_len,
> +		  const char *cmdline)
>  {
>  	uint64_t oldfdt_addr;
>  	int i, ret, chosen_node;
> @@ -847,6 +945,12 @@ int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
>  		}
>  	}
>  
> +	ret = setup_handover_buffer(image, fdt, chosen_node);
> +	if (ret) {
> +		pr_err("Error setting up the new device tree.\n");
> +		return ret;
> +	}
> +
>  	ret = fdt_setprop(fdt, chosen_node, "linux,booted-from-kexec", NULL, 0);
>  	if (ret) {
>  		pr_err("Error setting up the new device tree.\n");
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Thiago Jung Bauermann]

Am Montag, 22 August 2016, 11:17:45 schrieb Dave Young:
> On 08/18/16 at 06:09pm, Thiago Jung Bauermann wrote:
> > Hello Dave,
> > 
> > Thanks for your review!
> > 
> > [ Trimming down Cc: list a little to try to clear the "too many
> > recipients"> 
> >   mailing list restriction. ]
> 
> I also got "too many recipients".. Thanks for the trimming.

Didn't work though. What is the maximum number of recipients?

> > Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> > > 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?
> > 
> > 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?> 
> > > 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.

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

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

[Thiago Jung Bauermann]

Am Montag, 22 August 2016, 11:17:45 schrieb Dave Young:
> On 08/18/16 at 06:09pm, Thiago Jung Bauermann wrote:
> > Hello Dave,
> > 
> > Thanks for your review!
> > 
> > [ Trimming down Cc: list a little to try to clear the "too many
> > recipients"> 
> >   mailing list restriction. ]
> 
> I also got "too many recipients".. Thanks for the trimming.

Didn't work though. What is the maximum number of recipients?

> > Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> > > 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?
> > 
> > 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?> 
> > > 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.

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


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Dave Young]

On 08/22/16 at 12:25am, Thiago Jung Bauermann wrote:
> Am Montag, 22 August 2016, 11:17:45 schrieb Dave Young:
> > On 08/18/16 at 06:09pm, Thiago Jung Bauermann wrote:
> > > Hello Dave,
> > > 
> > > Thanks for your review!
> > > 
> > > [ Trimming down Cc: list a little to try to clear the "too many
> > > recipients"> 
> > >   mailing list restriction. ]
> > 
> > I also got "too many recipients".. Thanks for the trimming.
> 
> Didn't work though. What is the maximum number of recipients?

I have no idea as well..

> 
> > > Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> > > > 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?
> > > 
> > > 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?> 
> > > > 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.

Thanks
Dave

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

[Dave Young]

On 08/22/16 at 12:25am, Thiago Jung Bauermann wrote:
> Am Montag, 22 August 2016, 11:17:45 schrieb Dave Young:
> > On 08/18/16 at 06:09pm, Thiago Jung Bauermann wrote:
> > > Hello Dave,
> > > 
> > > Thanks for your review!
> > > 
> > > [ Trimming down Cc: list a little to try to clear the "too many
> > > recipients"> 
> > >   mailing list restriction. ]
> > 
> > I also got "too many recipients".. Thanks for the trimming.
> 
> Didn't work though. What is the maximum number of recipients?

I have no idea as well..

> 
> > > Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> > > > 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?
> > > 
> > > 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?> 
> > > > 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.

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 2/6] powerpc: kexec_file: Add buffer hand-over support for the next kernel

[Thiago Jung Bauermann]

Am Montag, 22 August 2016, 11:21:35 schrieb Dave Young:
> On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> > diff --git a/arch/powerpc/kernel/machine_kexec_64.c
> > b/arch/powerpc/kernel/machine_kexec_64.c index
> > a484a6346146..190c652e49b7 100644
> > --- a/arch/powerpc/kernel/machine_kexec_64.c
> > +++ b/arch/powerpc/kernel/machine_kexec_64.c
> > @@ -490,6 +490,60 @@ int arch_kimage_file_post_load_cleanup(struct
> > kimage *image)> 
> >  	return image->fops->cleanup(image->image_loader_data);
> >  
> >  }
> > 
> > +bool kexec_can_hand_over_buffer(void)
> > +{
> > +	return true;
> > +}
> > +
> > +int arch_kexec_add_handover_buffer(struct kimage *image,
> > +				   unsigned long load_addr, unsigned long 
size)
> > +{
> > +	image->arch.handover_buffer_addr = load_addr;
> > +	image->arch.handover_buffer_size = size;
> > +
> > +	return 0;
> > +}
> > +
> > +int kexec_get_handover_buffer(void **addr, unsigned long *size)
> > +{
> > +	int ret;
> > +	u64 start_addr, end_addr;
> > +
> > +	ret = of_property_read_u64(of_chosen,
> > +				   "linux,kexec-handover-buffer-start",
> > +				   &start_addr);
> > +	if (ret == -EINVAL)
> > +		return -ENOENT;
> > +	else if (ret)
> > +		return -EINVAL;
> > +
> > +	ret = of_property_read_u64(of_chosen,
> > "linux,kexec-handover-buffer-end", +				   
&end_addr);
> > +	if (ret == -EINVAL)
> > +		return -ENOENT;
> > +	else if (ret)
> > +		return -EINVAL;
> > +
> > +	*addr =  __va(start_addr);
> > +	/* -end is the first address after the buffer. */
> > +	*size = end_addr - start_addr;
> > +
> > +	return 0;
> > +}
> 
> This depends on dtb, so if IMA want to extend it to arches like x86 in
> the future you will have to think about other way to pass it.
> 
> How about think about a general way now?

The only general way I can think of is by adding a kernel command line 
parameter which the first kernel would pass to the second kernel, but IMHO 
that is ugly, because such parameter wouldn't be useful to a user, and it 
would also be something that, from the perspective of the user, would 
magically appear in the kernel command line of the second kernel...

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

Re: [PATCH v2 2/6] powerpc: kexec_file: Add buffer hand-over support for the next kernel

[Thiago Jung Bauermann]

Am Montag, 22 August 2016, 11:21:35 schrieb Dave Young:
> On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> > diff --git a/arch/powerpc/kernel/machine_kexec_64.c
> > b/arch/powerpc/kernel/machine_kexec_64.c index
> > a484a6346146..190c652e49b7 100644
> > --- a/arch/powerpc/kernel/machine_kexec_64.c
> > +++ b/arch/powerpc/kernel/machine_kexec_64.c
> > @@ -490,6 +490,60 @@ int arch_kimage_file_post_load_cleanup(struct
> > kimage *image)> 
> >  	return image->fops->cleanup(image->image_loader_data);
> >  
> >  }
> > 
> > +bool kexec_can_hand_over_buffer(void)
> > +{
> > +	return true;
> > +}
> > +
> > +int arch_kexec_add_handover_buffer(struct kimage *image,
> > +				   unsigned long load_addr, unsigned long 
size)
> > +{
> > +	image->arch.handover_buffer_addr = load_addr;
> > +	image->arch.handover_buffer_size = size;
> > +
> > +	return 0;
> > +}
> > +
> > +int kexec_get_handover_buffer(void **addr, unsigned long *size)
> > +{
> > +	int ret;
> > +	u64 start_addr, end_addr;
> > +
> > +	ret = of_property_read_u64(of_chosen,
> > +				   "linux,kexec-handover-buffer-start",
> > +				   &start_addr);
> > +	if (ret == -EINVAL)
> > +		return -ENOENT;
> > +	else if (ret)
> > +		return -EINVAL;
> > +
> > +	ret = of_property_read_u64(of_chosen,
> > "linux,kexec-handover-buffer-end", +				   
&end_addr);
> > +	if (ret == -EINVAL)
> > +		return -ENOENT;
> > +	else if (ret)
> > +		return -EINVAL;
> > +
> > +	*addr =  __va(start_addr);
> > +	/* -end is the first address after the buffer. */
> > +	*size = end_addr - start_addr;
> > +
> > +	return 0;
> > +}
> 
> This depends on dtb, so if IMA want to extend it to arches like x86 in
> the future you will have to think about other way to pass it.
> 
> How about think about a general way now?

The only general way I can think of is by adding a kernel command line 
parameter which the first kernel would pass to the second kernel, but IMHO 
that is ugly, because such parameter wouldn't be useful to a user, and it 
would also be something that, from the perspective of the user, would 
magically appear in the kernel command line of the second kernel...

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


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Thiago Jung Bauermann]

Am Montag, 22 August 2016, 11:36:43 schrieb Dave Young:
> On 08/22/16 at 12:25am, Thiago Jung Bauermann wrote:
> > Am Montag, 22 August 2016, 11:17:45 schrieb Dave Young:
> > > On 08/18/16 at 06:09pm, Thiago Jung Bauermann wrote:
> > > > Hello Dave,
> > > > 
> > > > Thanks for your review!
> > > > 
> > > > [ Trimming down Cc: list a little to try to clear the "too many
> > > > recipients">
> > > > 
> > > >   mailing list restriction. ]
> > > 
> > > I also got "too many recipients".. Thanks for the trimming.
> > 
> > Didn't work though. What is the maximum number of recipients?
> 
> I have no idea as well..
> 
> > > > Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> > > > > 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?
> > > > 
> > > > 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?>
> > > > 
> > > > > 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.

Ah, you are right! I'll provide an updated patch then. Thanks for your 
suggestion.

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

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

[Thiago Jung Bauermann]

Am Montag, 22 August 2016, 11:36:43 schrieb Dave Young:
> On 08/22/16 at 12:25am, Thiago Jung Bauermann wrote:
> > Am Montag, 22 August 2016, 11:17:45 schrieb Dave Young:
> > > On 08/18/16 at 06:09pm, Thiago Jung Bauermann wrote:
> > > > Hello Dave,
> > > > 
> > > > Thanks for your review!
> > > > 
> > > > [ Trimming down Cc: list a little to try to clear the "too many
> > > > recipients">
> > > > 
> > > >   mailing list restriction. ]
> > > 
> > > I also got "too many recipients".. Thanks for the trimming.
> > 
> > Didn't work though. What is the maximum number of recipients?
> 
> I have no idea as well..
> 
> > > > Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> > > > > 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?
> > > > 
> > > > 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?>
> > > > 
> > > > > 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.

Ah, you are right! I'll provide an updated patch then. Thanks for your 
suggestion.

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


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 2/6] powerpc: kexec_file: Add buffer hand-over support for the next kernel

[Dave Young]

On 08/22/16 at 12:38am, Thiago Jung Bauermann wrote:
> Am Montag, 22 August 2016, 11:21:35 schrieb Dave Young:
> > On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> > > diff --git a/arch/powerpc/kernel/machine_kexec_64.c
> > > b/arch/powerpc/kernel/machine_kexec_64.c index
> > > a484a6346146..190c652e49b7 100644
> > > --- a/arch/powerpc/kernel/machine_kexec_64.c
> > > +++ b/arch/powerpc/kernel/machine_kexec_64.c
> > > @@ -490,6 +490,60 @@ int arch_kimage_file_post_load_cleanup(struct
> > > kimage *image)> 
> > >  	return image->fops->cleanup(image->image_loader_data);
> > >  
> > >  }
> > > 
> > > +bool kexec_can_hand_over_buffer(void)
> > > +{
> > > +	return true;
> > > +}
> > > +
> > > +int arch_kexec_add_handover_buffer(struct kimage *image,
> > > +				   unsigned long load_addr, unsigned long 
> size)
> > > +{
> > > +	image->arch.handover_buffer_addr = load_addr;
> > > +	image->arch.handover_buffer_size = size;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +int kexec_get_handover_buffer(void **addr, unsigned long *size)
> > > +{
> > > +	int ret;
> > > +	u64 start_addr, end_addr;
> > > +
> > > +	ret = of_property_read_u64(of_chosen,
> > > +				   "linux,kexec-handover-buffer-start",
> > > +				   &start_addr);
> > > +	if (ret == -EINVAL)
> > > +		return -ENOENT;
> > > +	else if (ret)
> > > +		return -EINVAL;
> > > +
> > > +	ret = of_property_read_u64(of_chosen,
> > > "linux,kexec-handover-buffer-end", +				   
> &end_addr);
> > > +	if (ret == -EINVAL)
> > > +		return -ENOENT;
> > > +	else if (ret)
> > > +		return -EINVAL;
> > > +
> > > +	*addr =  __va(start_addr);
> > > +	/* -end is the first address after the buffer. */
> > > +	*size = end_addr - start_addr;
> > > +
> > > +	return 0;
> > > +}
> > 
> > This depends on dtb, so if IMA want to extend it to arches like x86 in
> > the future you will have to think about other way to pass it.
> > 
> > How about think about a general way now?
> 
> The only general way I can think of is by adding a kernel command line 
> parameter which the first kernel would pass to the second kernel, but IMHO 
> that is ugly, because such parameter wouldn't be useful to a user, and it 
> would also be something that, from the perspective of the user, would 
> magically appear in the kernel command line of the second kernel...

Sorry I just brought up the question, actually I have no idea either.
Maybe we have to do this with arch specific ways..

Thanks
Dave

Re: [PATCH v2 2/6] powerpc: kexec_file: Add buffer hand-over support for the next kernel

[Dave Young]

On 08/22/16 at 12:38am, Thiago Jung Bauermann wrote:
> Am Montag, 22 August 2016, 11:21:35 schrieb Dave Young:
> > On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> > > diff --git a/arch/powerpc/kernel/machine_kexec_64.c
> > > b/arch/powerpc/kernel/machine_kexec_64.c index
> > > a484a6346146..190c652e49b7 100644
> > > --- a/arch/powerpc/kernel/machine_kexec_64.c
> > > +++ b/arch/powerpc/kernel/machine_kexec_64.c
> > > @@ -490,6 +490,60 @@ int arch_kimage_file_post_load_cleanup(struct
> > > kimage *image)> 
> > >  	return image->fops->cleanup(image->image_loader_data);
> > >  
> > >  }
> > > 
> > > +bool kexec_can_hand_over_buffer(void)
> > > +{
> > > +	return true;
> > > +}
> > > +
> > > +int arch_kexec_add_handover_buffer(struct kimage *image,
> > > +				   unsigned long load_addr, unsigned long 
> size)
> > > +{
> > > +	image->arch.handover_buffer_addr = load_addr;
> > > +	image->arch.handover_buffer_size = size;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +int kexec_get_handover_buffer(void **addr, unsigned long *size)
> > > +{
> > > +	int ret;
> > > +	u64 start_addr, end_addr;
> > > +
> > > +	ret = of_property_read_u64(of_chosen,
> > > +				   "linux,kexec-handover-buffer-start",
> > > +				   &start_addr);
> > > +	if (ret == -EINVAL)
> > > +		return -ENOENT;
> > > +	else if (ret)
> > > +		return -EINVAL;
> > > +
> > > +	ret = of_property_read_u64(of_chosen,
> > > "linux,kexec-handover-buffer-end", +				   
> &end_addr);
> > > +	if (ret == -EINVAL)
> > > +		return -ENOENT;
> > > +	else if (ret)
> > > +		return -EINVAL;
> > > +
> > > +	*addr =  __va(start_addr);
> > > +	/* -end is the first address after the buffer. */
> > > +	*size = end_addr - start_addr;
> > > +
> > > +	return 0;
> > > +}
> > 
> > This depends on dtb, so if IMA want to extend it to arches like x86 in
> > the future you will have to think about other way to pass it.
> > 
> > How about think about a general way now?
> 
> The only general way I can think of is by adding a kernel command line 
> parameter which the first kernel would pass to the second kernel, but IMHO 
> that is ugly, because such parameter wouldn't be useful to a user, and it 
> would also be something that, from the perspective of the user, would 
> magically appear in the kernel command line of the second kernel...

Sorry I just brought up the question, actually I have no idea either.
Maybe we have to do this with arch specific ways..

Thanks
Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

[Thiago Jung Bauermann]

Am Montag, 22 August 2016, 11:36:43 schrieb Dave Young:
> On 08/22/16 at 12:25am, Thiago Jung Bauermann wrote:
> > Am Montag, 22 August 2016, 11:17:45 schrieb Dave Young:
> > > On 08/18/16 at 06:09pm, Thiago Jung Bauermann wrote:
> > > > Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> > > > > 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?
> > > > 
> > > > 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?>
> > > > 
> > > > > 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 <bauerman@linux.vnet.ibm.com>
---
 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

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

[Thiago Jung Bauermann]

Am Montag, 22 August 2016, 11:36:43 schrieb Dave Young:
> On 08/22/16 at 12:25am, Thiago Jung Bauermann wrote:
> > Am Montag, 22 August 2016, 11:17:45 schrieb Dave Young:
> > > On 08/18/16 at 06:09pm, Thiago Jung Bauermann wrote:
> > > > Am Donnerstag, 18 August 2016, 17:03:30 schrieb Dave Young:
> > > > > 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?
> > > > 
> > > > 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?>
> > > > 
> > > > > 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 <bauerman@linux.vnet.ibm.com>
---
 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



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 2/6] powerpc: kexec_file: Add buffer hand-over support for the next kernel

[Thiago Jung Bauermann]

Am Montag, 22 August 2016, 15:22:00 schrieb Dave Young:
> On 08/22/16 at 12:38am, Thiago Jung Bauermann wrote:
> > Am Montag, 22 August 2016, 11:21:35 schrieb Dave Young:
> > > On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> > > > diff --git a/arch/powerpc/kernel/machine_kexec_64.c
> > > > b/arch/powerpc/kernel/machine_kexec_64.c index
> > > > a484a6346146..190c652e49b7 100644
> > > > --- a/arch/powerpc/kernel/machine_kexec_64.c
> > > > +++ b/arch/powerpc/kernel/machine_kexec_64.c
> > > > @@ -490,6 +490,60 @@ int arch_kimage_file_post_load_cleanup(struct
> > > > kimage *image)>
> > > > 
> > > >  	return image->fops->cleanup(image->image_loader_data);
> > > >  
> > > >  }
> > > > 
> > > > +bool kexec_can_hand_over_buffer(void)
> > > > +{
> > > > +	return true;
> > > > +}
> > > > +
> > > > +int arch_kexec_add_handover_buffer(struct kimage *image,
> > > > +				   unsigned long load_addr, unsigned 
long
> > 
> > size)
> > 
> > > > +{
> > > > +	image->arch.handover_buffer_addr = load_addr;
> > > > +	image->arch.handover_buffer_size = size;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +int kexec_get_handover_buffer(void **addr, unsigned long *size)
> > > > +{
> > > > +	int ret;
> > > > +	u64 start_addr, end_addr;
> > > > +
> > > > +	ret = of_property_read_u64(of_chosen,
> > > > +				   "linux,kexec-handover-buffer-
start",
> > > > +				   &start_addr);
> > > > +	if (ret == -EINVAL)
> > > > +		return -ENOENT;
> > > > +	else if (ret)
> > > > +		return -EINVAL;
> > > > +
> > > > +	ret = of_property_read_u64(of_chosen,
> > > > "linux,kexec-handover-buffer-end", +
> > 
> > &end_addr);
> > 
> > > > +	if (ret == -EINVAL)
> > > > +		return -ENOENT;
> > > > +	else if (ret)
> > > > +		return -EINVAL;
> > > > +
> > > > +	*addr =  __va(start_addr);
> > > > +	/* -end is the first address after the buffer. */
> > > > +	*size = end_addr - start_addr;
> > > > +
> > > > +	return 0;
> > > > +}
> > > 
> > > This depends on dtb, so if IMA want to extend it to arches like x86 in
> > > the future you will have to think about other way to pass it.
> > > 
> > > How about think about a general way now?
> > 
> > The only general way I can think of is by adding a kernel command line
> > parameter which the first kernel would pass to the second kernel, but
> > IMHO that is ugly, because such parameter wouldn't be useful to a user,
> > and it would also be something that, from the perspective of the user,
> > would magically appear in the kernel command line of the second
> > kernel...
> Sorry I just brought up the question, actually I have no idea either.
> Maybe we have to do this with arch specific ways..

Actually, I don't think it's possible to avoid arch-specific code because 
the first kernel has to put the buffer memory region in a reserved memory 
map, and that is arch-specific.

On powerpc, this is done by adding it to the device tree memory reservation 
map. On x86, I believe this would be done added to the e820 map.

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

Re: [PATCH v2 2/6] powerpc: kexec_file: Add buffer hand-over support for the next kernel

[Thiago Jung Bauermann]

Am Montag, 22 August 2016, 15:22:00 schrieb Dave Young:
> On 08/22/16 at 12:38am, Thiago Jung Bauermann wrote:
> > Am Montag, 22 August 2016, 11:21:35 schrieb Dave Young:
> > > On 08/13/16 at 12:18am, Thiago Jung Bauermann wrote:
> > > > diff --git a/arch/powerpc/kernel/machine_kexec_64.c
> > > > b/arch/powerpc/kernel/machine_kexec_64.c index
> > > > a484a6346146..190c652e49b7 100644
> > > > --- a/arch/powerpc/kernel/machine_kexec_64.c
> > > > +++ b/arch/powerpc/kernel/machine_kexec_64.c
> > > > @@ -490,6 +490,60 @@ int arch_kimage_file_post_load_cleanup(struct
> > > > kimage *image)>
> > > > 
> > > >  	return image->fops->cleanup(image->image_loader_data);
> > > >  
> > > >  }
> > > > 
> > > > +bool kexec_can_hand_over_buffer(void)
> > > > +{
> > > > +	return true;
> > > > +}
> > > > +
> > > > +int arch_kexec_add_handover_buffer(struct kimage *image,
> > > > +				   unsigned long load_addr, unsigned 
long
> > 
> > size)
> > 
> > > > +{
> > > > +	image->arch.handover_buffer_addr = load_addr;
> > > > +	image->arch.handover_buffer_size = size;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +int kexec_get_handover_buffer(void **addr, unsigned long *size)
> > > > +{
> > > > +	int ret;
> > > > +	u64 start_addr, end_addr;
> > > > +
> > > > +	ret = of_property_read_u64(of_chosen,
> > > > +				   "linux,kexec-handover-buffer-
start",
> > > > +				   &start_addr);
> > > > +	if (ret == -EINVAL)
> > > > +		return -ENOENT;
> > > > +	else if (ret)
> > > > +		return -EINVAL;
> > > > +
> > > > +	ret = of_property_read_u64(of_chosen,
> > > > "linux,kexec-handover-buffer-end", +
> > 
> > &end_addr);
> > 
> > > > +	if (ret == -EINVAL)
> > > > +		return -ENOENT;
> > > > +	else if (ret)
> > > > +		return -EINVAL;
> > > > +
> > > > +	*addr =  __va(start_addr);
> > > > +	/* -end is the first address after the buffer. */
> > > > +	*size = end_addr - start_addr;
> > > > +
> > > > +	return 0;
> > > > +}
> > > 
> > > This depends on dtb, so if IMA want to extend it to arches like x86 in
> > > the future you will have to think about other way to pass it.
> > > 
> > > How about think about a general way now?
> > 
> > The only general way I can think of is by adding a kernel command line
> > parameter which the first kernel would pass to the second kernel, but
> > IMHO that is ugly, because such parameter wouldn't be useful to a user,
> > and it would also be something that, from the perspective of the user,
> > would magically appear in the kernel command line of the second
> > kernel...
> Sorry I just brought up the question, actually I have no idea either.
> Maybe we have to do this with arch specific ways..

Actually, I don't think it's possible to avoid arch-specific code because 
the first kernel has to put the buffer memory region in a reserved memory 
map, and that is arch-specific.

On powerpc, this is done by adding it to the device tree memory reservation 
map. On x86, I believe this would be done added to the e820 map.

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


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec