Re: [PATCH v4 1/5] powerpc: Refactor kexec functions to move arch independent code to IMA
On 8/27/20 6:23 PM, Thiago Jung Bauermann wrote:
Lakshmi Ramasubramanian writes:
+/**
+ * ima_get_kexec_buffer - get IMA 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 ima_get_kexec_buffer(void **addr, size_t *size)
I just noticed that this function is only called from within
ima_kexec.c, so it can be made static.
Will do.
+{
+ int ret, len;
+ unsigned long tmp_addr;
+ size_t tmp_size;
+ const void *prop;
+
+ prop = of_get_property(of_chosen, FDT_PROP_IMA_KEXEC_BUFFER, );
+ if (!prop)
+ return -ENOENT;
+
+ ret = do_get_kexec_buffer(prop, len, _addr, _size);
+ if (ret)
+ return ret;
+
+ *addr = __va(tmp_addr);
+ *size = tmp_size;
+
+ return 0;
+}
+
+/**
+ * ima_free_kexec_buffer - free memory used by the IMA buffer
+ */
+int ima_free_kexec_buffer(void)
This one can be static as well.
Will do.
-lakshmi
+{
+ int ret;
+ unsigned long addr;
+ size_t size;
+ struct property *prop;
+
+ prop = of_find_property(of_chosen, FDT_PROP_IMA_KEXEC_BUFFER, NULL);
+ if (!prop)
+ return -ENOENT;
+
+ ret = do_get_kexec_buffer(prop->value, prop->length, , );
+ if (ret)
+ return ret;
+
+ ret = of_remove_property(of_chosen, prop);
+ if (ret)
+ return ret;
+
+ return memblock_free(addr, size);
+
+}
+
#ifdef CONFIG_IMA_KEXEC
static int ima_dump_measurement_list(unsigned long *buffer_size, void
**buffer,
unsigned long segment_size)
Re: [PATCH v4 1/5] powerpc: Refactor kexec functions to move arch independent code to IMA
On 8/27/20 4:35 PM, Thiago Jung Bauermann wrote: Lakshmi Ramasubramanian writes: The functions ima_get_kexec_buffer() and ima_free_kexec_buffer() that handle carrying forward the IMA measurement logs on kexec for powerpc do not have architecture specific code, but they are currently defined for powerpc only. Move these functions to IMA subsystem so that it can be used for other architectures as well. A later patch in this series will use these functions for carrying forward the IMA measurement log for ARM64. Define FDT_PROP_IMA_KEXEC_BUFFER for the chosen node, namely "linux,ima-kexec-buffer", that is added to the DTB to hold the address and the size of the memory reserved to carry the IMA measurement log. Co-developed-by: Prakhar Srivastava Signed-off-by: Prakhar Srivastava Signed-off-by: Lakshmi Ramasubramanian This patch removes two functions from arch/powerpc/kexec/ima.c, but adds four to security/integrity/ima/ima_kexec.c. The extra ones are get_addr_size_cells() and do_get_kexec_buffer(), which are being copied from the powerpc code but can't be removed yet because they're still used there by remove_ima_buffer() and setup_ima_buffer(). On the next patch you remove the need for these functions in powerpc code and therefore delete them. This confused me at first, so I think it would be cleared if you put patch 2 first in the series and then on this patch you can simply move the four functions and delete them from arch/powerpc/kexec/ima.c. If you prefer to keep the current order, it's worth mentioning on the commit log where get_addr_size_cells() and do_get_kexec_buffer() are coming from. Regardless: Reviewed-by: Thiago Jung Bauermann Thanks for reviewing the changes Thiago. I'll update the commit log to describe the changes related to get_addr_size_cells() and do_get_kexec_buffer(). -lakshmi
Re: [PATCH v4 1/5] powerpc: Refactor kexec functions to move arch independent code to IMA
Lakshmi Ramasubramanian writes:
> +/**
> + * ima_get_kexec_buffer - get IMA 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 ima_get_kexec_buffer(void **addr, size_t *size)
I just noticed that this function is only called from within
ima_kexec.c, so it can be made static.
> +{
> + int ret, len;
> + unsigned long tmp_addr;
> + size_t tmp_size;
> + const void *prop;
> +
> + prop = of_get_property(of_chosen, FDT_PROP_IMA_KEXEC_BUFFER, );
> + if (!prop)
> + return -ENOENT;
> +
> + ret = do_get_kexec_buffer(prop, len, _addr, _size);
> + if (ret)
> + return ret;
> +
> + *addr = __va(tmp_addr);
> + *size = tmp_size;
> +
> + return 0;
> +}
> +
> +/**
> + * ima_free_kexec_buffer - free memory used by the IMA buffer
> + */
> +int ima_free_kexec_buffer(void)
This one can be static as well.
> +{
> + int ret;
> + unsigned long addr;
> + size_t size;
> + struct property *prop;
> +
> + prop = of_find_property(of_chosen, FDT_PROP_IMA_KEXEC_BUFFER, NULL);
> + if (!prop)
> + return -ENOENT;
> +
> + ret = do_get_kexec_buffer(prop->value, prop->length, , );
> + if (ret)
> + return ret;
> +
> + ret = of_remove_property(of_chosen, prop);
> + if (ret)
> + return ret;
> +
> + return memblock_free(addr, size);
> +
> +}
> +
> #ifdef CONFIG_IMA_KEXEC
> static int ima_dump_measurement_list(unsigned long *buffer_size, void
> **buffer,
>unsigned long segment_size)
--
Thiago Jung Bauermann
IBM Linux Technology Center
Re: [PATCH v4 1/5] powerpc: Refactor kexec functions to move arch independent code to IMA
Lakshmi Ramasubramanian writes: > The functions ima_get_kexec_buffer() and ima_free_kexec_buffer() that > handle carrying forward the IMA measurement logs on kexec for powerpc > do not have architecture specific code, but they are currently defined > for powerpc only. > > Move these functions to IMA subsystem so that it can be used for other > architectures as well. A later patch in this series will use these > functions for carrying forward the IMA measurement log for ARM64. > > Define FDT_PROP_IMA_KEXEC_BUFFER for the chosen node, namely > "linux,ima-kexec-buffer", that is added to the DTB to hold > the address and the size of the memory reserved to carry > the IMA measurement log. > > Co-developed-by: Prakhar Srivastava > Signed-off-by: Prakhar Srivastava > Signed-off-by: Lakshmi Ramasubramanian This patch removes two functions from arch/powerpc/kexec/ima.c, but adds four to security/integrity/ima/ima_kexec.c. The extra ones are get_addr_size_cells() and do_get_kexec_buffer(), which are being copied from the powerpc code but can't be removed yet because they're still used there by remove_ima_buffer() and setup_ima_buffer(). On the next patch you remove the need for these functions in powerpc code and therefore delete them. This confused me at first, so I think it would be cleared if you put patch 2 first in the series and then on this patch you can simply move the four functions and delete them from arch/powerpc/kexec/ima.c. If you prefer to keep the current order, it's worth mentioning on the commit log where get_addr_size_cells() and do_get_kexec_buffer() are coming from. Regardless: Reviewed-by: Thiago Jung Bauermann -- Thiago Jung Bauermann IBM Linux Technology Center
[PATCH v4 1/5] powerpc: Refactor kexec functions to move arch independent code to IMA
The functions ima_get_kexec_buffer() and ima_free_kexec_buffer() that
handle carrying forward the IMA measurement logs on kexec for powerpc
do not have architecture specific code, but they are currently defined
for powerpc only.
Move these functions to IMA subsystem so that it can be used for other
architectures as well. A later patch in this series will use these
functions for carrying forward the IMA measurement log for ARM64.
Define FDT_PROP_IMA_KEXEC_BUFFER for the chosen node, namely
"linux,ima-kexec-buffer", that is added to the DTB to hold
the address and the size of the memory reserved to carry
the IMA measurement log.
Co-developed-by: Prakhar Srivastava
Signed-off-by: Prakhar Srivastava
Signed-off-by: Lakshmi Ramasubramanian
---
arch/powerpc/include/asm/ima.h | 3 -
arch/powerpc/kexec/ima.c | 60 +---
include/linux/libfdt.h | 3 +
security/integrity/ima/ima_kexec.c | 91 ++
4 files changed, 97 insertions(+), 60 deletions(-)
diff --git a/arch/powerpc/include/asm/ima.h b/arch/powerpc/include/asm/ima.h
index ead488cf3981..bc27fd94de52 100644
--- a/arch/powerpc/include/asm/ima.h
+++ b/arch/powerpc/include/asm/ima.h
@@ -4,9 +4,6 @@
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
diff --git a/arch/powerpc/kexec/ima.c b/arch/powerpc/kexec/ima.c
index 720e50e490b6..f5112ee4bb0b 100644
--- a/arch/powerpc/kexec/ima.c
+++ b/arch/powerpc/kexec/ima.c
@@ -46,60 +46,6 @@ static int do_get_kexec_buffer(const void *prop, int len,
unsigned long *addr,
return 0;
}
-/**
- * ima_get_kexec_buffer - get IMA 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 ima_get_kexec_buffer(void **addr, size_t *size)
-{
- int ret, len;
- unsigned long tmp_addr;
- size_t tmp_size;
- const void *prop;
-
- prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", );
- if (!prop)
- return -ENOENT;
-
- ret = do_get_kexec_buffer(prop, len, _addr, _size);
- if (ret)
- return ret;
-
- *addr = __va(tmp_addr);
- *size = tmp_size;
-
- return 0;
-}
-
-/**
- * ima_free_kexec_buffer - free memory used by the IMA buffer
- */
-int ima_free_kexec_buffer(void)
-{
- int ret;
- unsigned long addr;
- size_t size;
- struct property *prop;
-
- prop = of_find_property(of_chosen, "linux,ima-kexec-buffer", NULL);
- if (!prop)
- return -ENOENT;
-
- ret = do_get_kexec_buffer(prop->value, prop->length, , );
- if (ret)
- return ret;
-
- ret = of_remove_property(of_chosen, prop);
- if (ret)
- return ret;
-
- return memblock_free(addr, size);
-
-}
-
/**
* remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
*
@@ -113,12 +59,12 @@ void remove_ima_buffer(void *fdt, int chosen_node)
size_t size;
const void *prop;
- prop = fdt_getprop(fdt, chosen_node, "linux,ima-kexec-buffer", );
+ prop = fdt_getprop(fdt, chosen_node, FDT_PROP_IMA_KEXEC_BUFFER, );
if (!prop)
return;
ret = do_get_kexec_buffer(prop, len, , );
- fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer");
+ fdt_delprop(fdt, chosen_node, FDT_PROP_IMA_KEXEC_BUFFER);
if (ret)
return;
@@ -201,7 +147,7 @@ int setup_ima_buffer(const struct kimage *image, void *fdt,
int chosen_node)
if (ret)
return ret;
- ret = fdt_setprop(fdt, chosen_node, "linux,ima-kexec-buffer", value,
+ ret = fdt_setprop(fdt, chosen_node, FDT_PROP_IMA_KEXEC_BUFFER, value,
entry_size);
if (ret < 0)
return -EINVAL;
diff --git a/include/linux/libfdt.h b/include/linux/libfdt.h
index 90ed4ebfa692..75fb40aa013b 100644
--- a/include/linux/libfdt.h
+++ b/include/linux/libfdt.h
@@ -5,4 +5,7 @@
#include
#include "../../scripts/dtc/libfdt/libfdt.h"
+/* Common device tree properties */
+#define FDT_PROP_IMA_KEXEC_BUFFER "linux,ima-kexec-buffer"
+
#endif /* _INCLUDE_LIBFDT_H_ */
diff --git a/security/integrity/ima/ima_kexec.c
b/security/integrity/ima/ima_kexec.c
index 121de3e04af2..25d79d521597 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -10,8 +10,99 @@
#include
#include
#include
+#include
+#include
+#include
#include "ima.h"
+static int get_addr_size_cells(int *addr_cells, int *size_cells)
+{
+ struct device_node *root;
+
+ root = of_find_node_by_path("/");
+ if (!root)
+ return -EINVAL;
+
+ *addr_cells =
