[RFC PATCH v1 1/1] Add support for arm64 to carry ima measurement log in kexec_file_load

2019-09-13 Thread Prakhar Srivastava 
During kexec_file_load, carrying forward the ima measurement log allows
a verifying party to get the entire runtime event log since the last
full reboot since that is when PCRs were last reset.

Signed-off-by: Prakhar Srivastava 
---
 arch/arm64/Kconfig |   7 +
 arch/arm64/include/asm/ima.h   |  29 
 arch/arm64/include/asm/kexec.h |   5 +
 arch/arm64/kernel/Makefile |   3 +-
 arch/arm64/kernel/ima_kexec.c  | 213 +
 arch/arm64/kernel/machine_kexec_file.c |   6 +
 6 files changed, 262 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/include/asm/ima.h
 create mode 100644 arch/arm64/kernel/ima_kexec.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 3adcec05b1f6..f39b12dbf9e8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -976,6 +976,13 @@ config KEXEC_VERIFY_SIG
  verification for the corresponding kernel image type being
  loaded in order for this to work.
 
+config HAVE_IMA_KEXEC
+   bool "Carry over IMA measurement log during kexec_file_load() syscall"
+   depends on KEXEC_FILE
+   help
+ Select this option to carry over IMA measurement log during
+ kexec_file_load.
+
 config KEXEC_IMAGE_VERIFY_SIG
bool "Enable Image signature verification support"
default y
diff --git a/arch/arm64/include/asm/ima.h b/arch/arm64/include/asm/ima.h
new file mode 100644
index ..e23cee84729f
--- /dev/null
+++ b/arch/arm64/include/asm/ima.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ARM64_IMA_H
+#define _ASM_ARM64_IMA_H
+
+struct kimage;
+
+int ima_get_kexec_buffer(void **addr, size_t *size);
+int ima_free_kexec_buffer(void);
+
+#ifdef CONFIG_IMA
+void remove_ima_buffer(void *fdt, int chosen_node);
+#else
+static inline void remove_ima_buffer(void *fdt, int chosen_node) {}
+#endif
+
+#ifdef CONFIG_IMA_KEXEC
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+ size_t size);
+
+int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node);
+#else
+static inline int setup_ima_buffer(const struct kimage *image, void *fdt,
+  int chosen_node)
+{
+   remove_ima_buffer(fdt, chosen_node);
+   return 0;
+}
+#endif /* CONFIG_IMA_KEXEC */
+#endif /* _ASM_ARM64_IMA_H */
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 12a561a54128..e8d2412066e7 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -96,6 +96,11 @@ static inline void crash_post_resume(void) {}
 struct kimage_arch {
void *dtb;
unsigned long dtb_mem;
+
+#ifdef CONFIG_IMA_KEXEC
+   phys_addr_t ima_buffer_addr;
+   size_t ima_buffer_size;
+#endif
 };
 
 extern const struct kexec_file_ops kexec_image_ops;
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 478491f07b4f..580238f2e9a7 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -55,7 +55,8 @@ obj-$(CONFIG_RANDOMIZE_BASE)  += kaslr.o
 obj-$(CONFIG_HIBERNATION)  += hibernate.o hibernate-asm.o
 obj-$(CONFIG_KEXEC_CORE)   += machine_kexec.o relocate_kernel.o
\
   cpu-reset.o
-obj-$(CONFIG_KEXEC_FILE)   += machine_kexec_file.o kexec_image.o
+obj-$(CONFIG_KEXEC_FILE)   += machine_kexec_file.o kexec_image.o   
\
+  ima_kexec.o
 obj-$(CONFIG_ARM64_RELOC_TEST) += arm64-reloc-test.o
 arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o
 obj-$(CONFIG_CRASH_DUMP)   += crash_dump.o
diff --git a/arch/arm64/kernel/ima_kexec.c b/arch/arm64/kernel/ima_kexec.c
new file mode 100644
index ..b14326d541f3
--- /dev/null
+++ b/arch/arm64/kernel/ima_kexec.c
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Microsoft Corporation.
+ *
+ * Authors:
+ * Prakhar Srivastava 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+/**
+ * delete_fdt_mem_rsv - delete memory reservation with given address and size
+ * @fdt - pointer to the fdt.
+ * @start - start address of the memory.
+ * @size - number of cells to be deletd.
+ * 
+ * Return: 0 on success, or negative errno on error.
+ */
+int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size)
+{
+   int i, ret, num_rsvs = fdt_num_mem_rsv(fdt);
+
+   for (i = 0; i < num_rsvs; i++) {
+   uint64_t rsv_start, rsv_size;
+
+   ret = fdt_get_mem_rsv(fdt, i, _start, _size);
+   if (ret) {
+   pr_err("Malformed device tree\n");
+   return -EINVAL;
+   }
+
+   if (rsv_start == start && rsv_size == size) {
+   ret = fdt_del_mem_rsv(fdt, i);
+   if (ret) {
+

[RFC PATCH v1 0/1] Add support for arm64 to carry ima measurement log in kexec_file_load

2019-09-13 Thread Prakhar Srivastava 
Add support for arm64 to carry ima measurement log
to the next kexec'ed session triggered via kexec_file_load.
- Top of Linux 5.3-rc6

Currently during kexec the kernel file signatures are/can be validated
prior to actual load, the information(PE/ima signature) is not carried
to the next session. This lead to loss of information.

Carrying forward the ima measurement log to the next kexec'ed session 
allows a verifying party to get the entire runtime event log since the
last full reboot, since that is when PCRs were last reset.

Changelog:

v1:
  - add new fdt porperties to mark start and end for ima measurement
log.
  - use fdt_* functions to add/remove fdt properties and memory
allocations.
  - remove additional check for endian-ness as they are checked
in fdt_* functions.

v0:
  - Add support to carry ima measurement log in arm64, 
   uses same code as powerpc.

Prakhar Srivastava (1):
  Add support for arm64 to carry ima measurement log in kexec_file_load

 arch/arm64/Kconfig |   7 +
 arch/arm64/include/asm/ima.h   |  29 
 arch/arm64/include/asm/kexec.h |   5 +
 arch/arm64/kernel/Makefile |   3 +-
 arch/arm64/kernel/ima_kexec.c  | 213 +
 arch/arm64/kernel/machine_kexec_file.c |   6 +
 6 files changed, 262 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/include/asm/ima.h
 create mode 100644 arch/arm64/kernel/ima_kexec.c

-- 
2.17.1


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

[PATCH 4.19 163/190] resource: Fix find_next_iomem_res() iteration issue

2019-09-13 Thread Greg Kroah-Hartman 
[ Upstream commit 010a93bf97c72f43aac664d0a685942f83d1a103 ]

Previously find_next_iomem_res() used "*res" as both an input parameter for
the range to search and the type of resource to search for, and an output
parameter for the resource we found, which makes the interface confusing.

The current callers use find_next_iomem_res() incorrectly because they
allocate a single struct resource and use it for repeated calls to
find_next_iomem_res().  When find_next_iomem_res() returns a resource, it
overwrites the start, end, flags, and desc members of the struct.  If we
call find_next_iomem_res() again, we must update or restore these fields.
The previous code restored res.start and res.end, but not res.flags or
res.desc.

Since the callers did not restore res.flags, if they searched for flags
IORESOURCE_MEM | IORESOURCE_BUSY and found a resource with flags
IORESOURCE_MEM | IORESOURCE_BUSY | IORESOURCE_SYSRAM, the next search would
incorrectly skip resources unless they were also marked as
IORESOURCE_SYSRAM.

Fix this by restructuring the interface so it takes explicit "start, end,
flags" parameters and uses "*res" only as an output parameter.

Based on a patch by Lianbo Jiang .

 [ bp: While at it:
   - make comments kernel-doc style.
   -

Originally-by: 
http://lore.kernel.org/lkml/20180921073211.20097-2-liji...@redhat.com
Signed-off-by: Bjorn Helgaas 
Signed-off-by: Borislav Petkov 
CC: Andrew Morton 
CC: Brijesh Singh 
CC: Dan Williams 
CC: H. Peter Anvin 
CC: Lianbo Jiang 
CC: Takashi Iwai 
CC: Thomas Gleixner 
CC: Tom Lendacky 
CC: Vivek Goyal 
CC: Yaowei Bai 
CC: b...@redhat.com
CC: dan.j.willi...@intel.com
CC: dyo...@redhat.com
CC: kexec@lists.infradead.org
CC: mi...@redhat.com
CC: x86-ml 
Link: 
http://lkml.kernel.org/r/153805812916.1157.177580438135143788.st...@bhelgaas-glaptop.roam.corp.google.com
Signed-off-by: Sasha Levin 
---
 kernel/resource.c | 96 +--
 1 file changed, 42 insertions(+), 54 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 155ec873ea4d1..38b8d11c9eaf4 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -318,24 +318,27 @@ int release_resource(struct resource *old)
 
 EXPORT_SYMBOL(release_resource);
 
-/*
- * Finds the lowest iomem resource existing within [res->start..res->end].
- * The caller must specify res->start, res->end, res->flags, and optionally
- * desc.  If found, returns 0, res is overwritten, if not found, returns -1.
- * This function walks the whole tree and not just first level children until
- * and unless first_level_children_only is true.
+/**
+ * Finds the lowest iomem resource that covers part of [start..end].  The
+ * caller must specify start, end, flags, and desc (which may be
+ * IORES_DESC_NONE).
+ *
+ * If a resource is found, returns 0 and *res is overwritten with the part
+ * of the resource that's within [start..end]; if none is found, returns
+ * -1.
+ *
+ * This function walks the whole tree and not just first level children
+ * unless @first_level_children_only is true.
  */
-static int find_next_iomem_res(struct resource *res, unsigned long desc,
-  bool first_level_children_only)
+static int find_next_iomem_res(resource_size_t start, resource_size_t end,
+  unsigned long flags, unsigned long desc,
+  bool first_level_children_only,
+  struct resource *res)
 {
-   resource_size_t start, end;
struct resource *p;
bool sibling_only = false;
 
BUG_ON(!res);
-
-   start = res->start;
-   end = res->end;
BUG_ON(start >= end);
 
if (first_level_children_only)
@@ -344,7 +347,7 @@ static int find_next_iomem_res(struct resource *res, 
unsigned long desc,
read_lock(_lock);
 
for (p = iomem_resource.child; p; p = next_resource(p, sibling_only)) {
-   if ((p->flags & res->flags) != res->flags)
+   if ((p->flags & flags) != flags)
continue;
if ((desc != IORES_DESC_NONE) && (desc != p->desc))
continue;
@@ -359,32 +362,31 @@ static int find_next_iomem_res(struct resource *res, 
unsigned long desc,
read_unlock(_lock);
if (!p)
return -1;
+
/* copy data */
-   if (res->start < p->start)
-   res->start = p->start;
-   if (res->end > p->end)
-   res->end = p->end;
+   res->start = max(start, p->start);
+   res->end = min(end, p->end);
res->flags = p->flags;
res->desc = p->desc;
return 0;
 }
 
-static int __walk_iomem_res_desc(struct resource *res, unsigned long desc,
-bool first_level_children_only,
-void *arg,
+static int __walk_iomem_res_desc(resource_size_t start, resource_size_t end,
+unsigned long flags, unsigned long desc,
+

[PATCH 4.19 162/190] resource: Include resource end in walk_*() interfaces

2019-09-13 Thread Greg Kroah-Hartman 
[ Upstream commit a98959fdbda1849a01b2150bb635ed559ec06700 ]

find_next_iomem_res() finds an iomem resource that covers part of a range
described by "start, end".  All callers expect that range to be inclusive,
i.e., both start and end are included, but find_next_iomem_res() doesn't
handle the end address correctly.

If it finds an iomem resource that contains exactly the end address, it
skips it, e.g., if "start, end" is [0x0-0x1] and there happens to be an
iomem resource [mem 0x1-0x1] (the single byte at 0x1), we skip
it:

  find_next_iomem_res(...)
  {
start = 0x0;
end = 0x1;
for (p = next_resource(...)) {
  # p->start = 0x1;
  # p->end = 0x1;
  # we *should* return this resource, but this condition is false:
  if ((p->end >= start) && (p->start < end))
break;

Adjust find_next_iomem_res() so it allows a resource that includes the
single byte at the end of the range.  This is a corner case that we
probably don't see in practice.

Fixes: 58c1b5b07907 ("[PATCH] memory hotadd fixes: find_next_system_ram catch 
range fix")
Signed-off-by: Bjorn Helgaas 
Signed-off-by: Borislav Petkov 
CC: Andrew Morton 
CC: Brijesh Singh 
CC: Dan Williams 
CC: H. Peter Anvin 
CC: Lianbo Jiang 
CC: Takashi Iwai 
CC: Thomas Gleixner 
CC: Tom Lendacky 
CC: Vivek Goyal 
CC: Yaowei Bai 
CC: b...@redhat.com
CC: dan.j.willi...@intel.com
CC: dyo...@redhat.com
CC: kexec@lists.infradead.org
CC: mi...@redhat.com
CC: x86-ml 
Link: 
http://lkml.kernel.org/r/153805812254.1157.16736368485811773752.st...@bhelgaas-glaptop.roam.corp.google.com
Signed-off-by: Sasha Levin 
---
 kernel/resource.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 30e1bc68503b5..155ec873ea4d1 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -319,7 +319,7 @@ int release_resource(struct resource *old)
 EXPORT_SYMBOL(release_resource);
 
 /*
- * Finds the lowest iomem resource existing within [res->start.res->end).
+ * Finds the lowest iomem resource existing within [res->start..res->end].
  * The caller must specify res->start, res->end, res->flags, and optionally
  * desc.  If found, returns 0, res is overwritten, if not found, returns -1.
  * This function walks the whole tree and not just first level children until
@@ -352,7 +352,7 @@ static int find_next_iomem_res(struct resource *res, 
unsigned long desc,
p = NULL;
break;
}
-   if ((p->end >= start) && (p->start < end))
+   if ((p->end >= start) && (p->start <= end))
break;
}
 
-- 
2.20.1




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

Re: [PATCH] do not clean dummy variable in kexec path

2019-09-13 Thread Ard Biesheuvel 
On Tue, 13 Aug 2019 at 22:14, Matthew Garrett  wrote:
>
> On Tue, Aug 13, 2019 at 4:28 AM Laszlo Ersek  wrote:
> > (I verified yesterday, using the edk2 source code, that there is no
> > varstore reclaim after ExitBootServices(), indeed.)
>
> Some implementations do reclaim at runtime, in which case the
> create/delete dance will permit variable creation.
>
> > (a) Attempting to delete the dummy variable in efi_enter_virtual_mode().
>
> To be clear, the dummy variable should never actually come into
> existence - we explicitly attempt to create a variable that's bigger
> than the available space, so the expectation is that it will always
> fail. However, should it somehow end up being created, there's a race
> between the creation and the deletion and so there's a (small) risk
> that the variable actually ends up there. The cleanup in
> enter_virtual_mode() is just there to ensure that anything that did
> end up being created on a previous boot is deleted - the expectation
> is that it'll be a noop.
>
> > (b) The following part, in efi_query_variable_store():
> >
> > +   /*
> > +* The runtime code may now have triggered a garbage 
> > collection
> > +* run, so check the variable info again
> > +*/
> >
> > Let me start with (b). That code is essentially dead, I would say, based
> > on the information that had already been captured in the commit message
> > of [1]. Reclaim would never happen after ExitBootServices(). (I assume
> > efi_query_variable_store() is only invoked after ExitBootServices(),
> > i.e., from kernel space proper -- sorry if that's a wrong assumption.)
>
> It's dead code on Tiano, but not on at least one vendor implementation.
>
> > Considering (a): what justified the attempt to delete the dummy variable
> > in efi_enter_virtual_mode(), in commit [4]? Was that meant as a
> > fail-safe just so we don't leave a dummy variable lying around?
>
> Yes.
>
> > So even if we consider the "clean DUMMY object" hunk from [4] a
> > justified fail-safe for the normal boot path, it doesn't apply to the
> > kexec path -- the cold-booted primary kernel will have gone through
> > those motions already, will it not?
> >
> > Therefore, we should do two things:
> >
> > - Remove the cleanup from the kexec path -- the cleanup logic from [4],
> >   even if justified for the cold boot path, should have never modified
> >   the kexec path.
>
> I agree that there's no benefit in it being called in the kexec path.

Can I take that as an ack?

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