Re: [PATCH v12 2/4] powerpc: Move arch independent ima kexec functions to drivers/of/kexec.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 17, 2020 at 09:37:06AM -0800, Lakshmi Ramasubramanian wrote:
> The functions defined in "arch/powerpc/kexec/ima.c" handle setting up
> and freeing the resources required to carry over the IMA measurement
> list from the current kernel to the next kernel across kexec system call.
> These functions do not have architecture specific code, but are
> currently limited to powerpc.
> 
> Move setup_ima_buffer() call into of_kexec_setup_new_fdt().
> 
> Move architecture independent functions from "arch/powerpc/kexec/ima.c"
> to "drivers/of/kexec.c". Delete "arch/powerpc/kexec/ima.c" and
> "arch/powerpc/include/asm/ima.h". Remove references to the deleted files
> in powerpc and in ima.
> 
> delete_fdt_mem_rsv() defined in "arch/powerpc/kexec/file_load.c"
> is not used anymore. Remove this function.

I think this patch should come first and then patch 1. Move then rework 
the code.

> 
> Signed-off-by: Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx>
> ---
>  arch/powerpc/include/asm/ima.h     |  30 -----
>  arch/powerpc/include/asm/kexec.h   |   1 -
>  arch/powerpc/kexec/Makefile        |   7 -
>  arch/powerpc/kexec/file_load.c     |  39 ------
>  arch/powerpc/kexec/ima.c           | 182 --------------------------
>  drivers/of/kexec.c                 | 198 +++++++++++++++++++++++++++++
>  include/linux/of.h                 |  17 +++
>  security/integrity/ima/ima.h       |   4 -
>  security/integrity/ima/ima_kexec.c |   2 +
>  9 files changed, 217 insertions(+), 263 deletions(-)
>  delete mode 100644 arch/powerpc/include/asm/ima.h
>  delete mode 100644 arch/powerpc/kexec/ima.c
> 
> diff --git a/arch/powerpc/include/asm/ima.h b/arch/powerpc/include/asm/ima.h
> deleted file mode 100644
> index ead488cf3981..000000000000
> --- a/arch/powerpc/include/asm/ima.h
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef _ASM_POWERPC_IMA_H
> -#define _ASM_POWERPC_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_POWERPC_IMA_H */
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index dbf09d2f36d0..d4b7d2d6191d 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -126,7 +126,6 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
>  int setup_new_fdt(const struct kimage *image, void *fdt,
>  		  unsigned long initrd_load_addr, unsigned long initrd_len,
>  		  const char *cmdline);
> -int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size);
>  
>  #ifdef CONFIG_PPC64
>  struct kexec_buf;
> diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile
> index 4aff6846c772..b6c52608cb49 100644
> --- a/arch/powerpc/kexec/Makefile
> +++ b/arch/powerpc/kexec/Makefile
> @@ -9,13 +9,6 @@ obj-$(CONFIG_PPC32)		+= relocate_32.o
>  
>  obj-$(CONFIG_KEXEC_FILE)	+= file_load.o ranges.o file_load_$(BITS).o elf_$(BITS).o
>  
> -ifdef CONFIG_HAVE_IMA_KEXEC
> -ifdef CONFIG_IMA
> -obj-y				+= ima.o
> -endif
> -endif
> -
> -
>  # Disable GCOV, KCOV & sanitizers in odd or sensitive code
>  GCOV_PROFILE_core_$(BITS).o := n
>  KCOV_INSTRUMENT_core_$(BITS).o := n
> diff --git a/arch/powerpc/kexec/file_load.c b/arch/powerpc/kexec/file_load.c
> index 956bcb2d1ec2..f37652ccb8a1 100644
> --- a/arch/powerpc/kexec/file_load.c
> +++ b/arch/powerpc/kexec/file_load.c
> @@ -20,7 +20,6 @@
>  #include <linux/of_fdt.h>
>  #include <linux/libfdt.h>
>  #include <asm/setup.h>
> -#include <asm/ima.h>
>  
>  #define SLAVE_CODE_SIZE		256	/* First 0x100 bytes */
>  
> @@ -110,38 +109,6 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
>  	return 0;
>  }
>  
> -/**
> - * delete_fdt_mem_rsv - delete memory reservation with given address and size
> - *
> - * 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, &rsv_start, &rsv_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) {
> -				pr_err("Error deleting device tree reservation.\n");
> -				return -EINVAL;
> -			}
> -
> -			return 0;
> -		}
> -	}
> -
> -	return -ENOENT;
> -}
> -
>  /*
>   * setup_new_fdt - modify /chosen and memory reservation for the next kernel
>   * @image:		kexec image being loaded.
> @@ -163,12 +130,6 @@ int setup_new_fdt(const struct kimage *image, void *fdt,
>  	if (ret)
>  		goto err;
>  
> -	ret = setup_ima_buffer(image, fdt, fdt_path_offset(fdt, "/chosen"));
> -	if (ret) {
> -		pr_err("Error setting up the new device tree.\n");
> -		return ret;
> -	}
> -
>  	return 0;
>  
>  err:
> diff --git a/arch/powerpc/kexec/ima.c b/arch/powerpc/kexec/ima.c
> deleted file mode 100644
> index d579d3da4715..000000000000
> --- a/arch/powerpc/kexec/ima.c
> +++ /dev/null
> @@ -1,182 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> -/*
> - * Copyright (C) 2016 IBM Corporation
> - *
> - * Authors:
> - * Thiago Jung Bauermann <bauerman@xxxxxxxxxxxxxxxxxx>
> - */
> -
> -#include <linux/slab.h>
> -#include <linux/kexec.h>
> -#include <linux/of.h>
> -#include <linux/memblock.h>
> -#include <linux/libfdt.h>
> -#include <asm/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 = of_n_addr_cells(root);
> -	*size_cells = of_n_size_cells(root);
> -
> -	of_node_put(root);
> -
> -	return 0;
> -}
> -
> -static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
> -			       size_t *size)
> -{
> -	int ret, addr_cells, size_cells;
> -
> -	ret = get_addr_size_cells(&addr_cells, &size_cells);
> -	if (ret)
> -		return ret;
> -
> -	if (len < 4 * (addr_cells + size_cells))
> -		return -ENOENT;
> -
> -	*addr = of_read_number(prop, addr_cells);
> -	*size = of_read_number(prop + 4 * addr_cells, size_cells);
> -
> -	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", &len);
> -	if (!prop)
> -		return -ENOENT;
> -
> -	ret = do_get_kexec_buffer(prop, len, &tmp_addr, &tmp_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, &addr, &size);
> -	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
> - *
> - * The IMA measurement buffer is of no use to a subsequent kernel, so we always
> - * remove it from the device tree.
> - */
> -void remove_ima_buffer(void *fdt, int chosen_node)
> -{
> -	int ret, len;
> -	unsigned long addr;
> -	size_t size;
> -	const void *prop;
> -
> -	prop = fdt_getprop(fdt, chosen_node, "linux,ima-kexec-buffer", &len);
> -	if (!prop)
> -		return;
> -
> -	ret = do_get_kexec_buffer(prop, len, &addr, &size);
> -	fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer");
> -	if (ret)
> -		return;
> -
> -	ret = delete_fdt_mem_rsv(fdt, addr, size);
> -	if (!ret)
> -		pr_debug("Removed old IMA buffer reservation.\n");
> -}
> -
> -#ifdef CONFIG_IMA_KEXEC
> -/**
> - * arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA buffer
> - *
> - * Architectures should use this function to pass on the IMA buffer
> - * information to the next kernel.
> - *
> - * Return: 0 on success, negative errno on error.
> - */
> -int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
> -			      size_t size)
> -{
> -	image->arch.ima_buffer_addr = load_addr;
> -	image->arch.ima_buffer_size = size;
> -
> -	return 0;
> -}
> -
> -/**
> - * setup_ima_buffer - add IMA buffer information to the fdt
> - * @image:		kexec image being loaded.
> - * @fdt:		Flattened device tree for the next kernel.
> - * @chosen_node:	Offset to the chosen node.
> - *
> - * Return: 0 on success, or negative errno on error.
> - */
> -int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node)
> -{
> -	int ret;
> -
> -	remove_ima_buffer(fdt, chosen_node);
> -	if (!image->arch.ima_buffer_size)
> -		return 0;
> -
> -	ret = fdt_appendprop_addrrange(fdt, 0, chosen_node,
> -				       "linux,ima-kexec-buffer",
> -				       image->arch.ima_buffer_addr,
> -				       image->arch.ima_buffer_size);
> -	if (ret)
> -		return (ret == -FDT_ERR_NOSPACE ? -ENOMEM : -EINVAL);
> -
> -	ret = fdt_add_mem_rsv(fdt, image->arch.ima_buffer_addr,
> -			      image->arch.ima_buffer_size);
> -	if (ret)
> -		return -EINVAL;
> -
> -	pr_debug("IMA buffer at 0x%llx, size = 0x%zx\n",
> -		 image->arch.ima_buffer_addr, image->arch.ima_buffer_size);
> -
> -	return 0;
> -}
> -#endif /* CONFIG_IMA_KEXEC */
> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> index 66787be081fe..a6ab35f16dd2 100644
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
> @@ -10,10 +10,12 @@
>   */
>  
>  #include <linux/kernel.h>
> +#include <linux/slab.h>
>  #include <linux/kexec.h>
>  #include <linux/libfdt.h>
>  #include <linux/of.h>
>  #include <linux/of_fdt.h>
> +#include <linux/memblock.h>
>  #include <linux/random.h>
>  #include <linux/types.h>
>  
> @@ -59,6 +61,141 @@ static int fdt_find_and_del_mem_rsv(void *fdt, unsigned long start, unsigned lon
>  	return -ENOENT;
>  }
>  
> +/**
> + * get_addr_size_cells - Get address and size of root node
> + *
> + * @addr_cells: Return address of the root node
> + * @size_cells: Return size of the root node
> + *
> + * Return: 0 on success, or negative errno on error.
> + */
> +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 = of_n_addr_cells(root);
> +	*size_cells = of_n_size_cells(root);
> +
> +	of_node_put(root);
> +
> +	return 0;
> +}
> +
> +/**
> + * do_get_kexec_buffer - Get address and size of device tree property
> + *
> + * @prop: Device tree property
> + * @len: Size of @prop
> + * @addr: Return address of the node
> + * @size: Return size of the node
> + *
> + * Return: 0 on success, or negative errno on error.
> + */
> +static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
> +			       size_t *size)
> +{
> +	int ret, addr_cells, size_cells;
> +
> +	ret = get_addr_size_cells(&addr_cells, &size_cells);
> +	if (ret)
> +		return ret;
> +
> +	if (len < 4 * (addr_cells + size_cells))
> +		return -ENOENT;
> +
> +	*addr = of_read_number(prop, addr_cells);
> +	*size = of_read_number(prop + 4 * addr_cells, size_cells);
> +
> +	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", &len);
> +	if (!prop)
> +		return -ENOENT;
> +
> +	ret = do_get_kexec_buffer(prop, len, &tmp_addr, &tmp_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, &addr, &size);
> +	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
> + *
> + * @fdt: Flattened Device Tree to update
> + * @chosen_node: Offset to the chosen node in the device tree
> + *
> + * The IMA measurement buffer is of no use to a subsequent kernel, so we always
> + * remove it from the device tree.
> + */
> +void remove_ima_buffer(void *fdt, int chosen_node)

static

> +{
> +	int ret, len;
> +	unsigned long addr;
> +	size_t size;
> +	const void *prop;
> +
> +	prop = fdt_getprop(fdt, chosen_node, "linux,ima-kexec-buffer", &len);
> +	if (!prop)
> +		return;
> +
> +	ret = do_get_kexec_buffer(prop, len, &addr, &size);
> +	fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer");
> +	if (ret)
> +		return;
> +
> +	ret = fdt_find_and_del_mem_rsv(fdt, addr, size);
> +	if (!ret)
> +		pr_debug("Removed old IMA buffer reservation.\n");
> +}

All the above needs to be in an '#ifdef CONFIG_IMA_KEXEC'.

> +
>  /*
>   * of_kexec_setup_new_fdt - modify /chosen and memory reservation for the next kernel
>   *
> @@ -219,6 +356,10 @@ int of_kexec_setup_new_fdt(const struct kimage *image, void *fdt,
>  	}
>  
>  	ret = fdt_setprop(fdt, chosen_node, "linux,booted-from-kexec", NULL, 0);
> +	if (ret)
> +		goto out;
> +
> +	ret = setup_ima_buffer(image, fdt, fdt_path_offset(fdt, "/chosen"));
>  
>  out:
>  	if (ret)
> @@ -226,3 +367,60 @@ int of_kexec_setup_new_fdt(const struct kimage *image, void *fdt,
>  
>  	return 0;
>  }
> +
> +#ifdef CONFIG_IMA_KEXEC
> +/**
> + * arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA buffer
> + *
> + * @image: kimage struct to set IMA buffer data
> + * @load_addr: Starting address where IMA buffer is loaded at
> + * @size: Number of bytes in the IMA buffer
> + *
> + * Architectures should use this function to pass on the IMA buffer
> + * information to the next kernel.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
> +			      size_t size)

This should be a static inline in asm/kexec.h.

> +{
> +	image->arch.ima_buffer_addr = load_addr;
> +	image->arch.ima_buffer_size = size;
> +
> +	return 0;
> +}
> +
> +/**
> + * setup_ima_buffer - add IMA buffer information to the fdt
> + * @image:		kexec image being loaded.
> + * @fdt:		Flattened device tree for the next kernel.
> + * @chosen_node:	Offset to the chosen node.
> + *
> + * Return: 0 on success, or negative errno on error.
> + */
> +int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node)

static

> +{
> +	int ret;
> +
> +	remove_ima_buffer(fdt, chosen_node);
> +	if (!image->arch.ima_buffer_size)
> +		return 0;
> +
> +	ret = fdt_appendprop_addrrange(fdt, 0, chosen_node,
> +				       "linux,ima-kexec-buffer",
> +				       image->arch.ima_buffer_addr,
> +				       image->arch.ima_buffer_size);
> +	if (ret)
> +		return (ret == -FDT_ERR_NOSPACE ? -ENOMEM : -EINVAL);
> +
> +	ret = fdt_add_mem_rsv(fdt, image->arch.ima_buffer_addr,
> +			      image->arch.ima_buffer_size);
> +	if (ret)
> +		return -EINVAL;
> +
> +	pr_debug("IMA buffer at 0x%llx, size = 0x%zx\n",
> +		 image->arch.ima_buffer_addr, image->arch.ima_buffer_size);
> +
> +	return 0;
> +}
> +#endif /* CONFIG_IMA_KEXEC */
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 3375f5295875..7477f2266d8f 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -562,6 +562,23 @@ struct kimage;
>  int of_kexec_setup_new_fdt(const struct kimage *image, void *fdt,
>  			   unsigned long initrd_load_addr, unsigned long initrd_len,
>  			   const char *cmdline);
> +int ima_get_kexec_buffer(void **addr, size_t *size);
> +int ima_free_kexec_buffer(void);
> +void remove_ima_buffer(void *fdt, int chosen_node);
> +
> +#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);

This doesn't need to be public.

> +#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 */
>  
>  #else /* CONFIG_OF */
>  
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 6ebefec616e4..7c3947ad3773 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -24,10 +24,6 @@
>  
>  #include "../integrity.h"
>  
> -#ifdef CONFIG_HAVE_IMA_KEXEC
> -#include <asm/ima.h>
> -#endif
> -
>  enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN,
>  		     IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
>  enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 121de3e04af2..3d6a8820e2a5 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -9,7 +9,9 @@
>  
>  #include <linux/seq_file.h>
>  #include <linux/vmalloc.h>
> +#include <linux/of.h>
>  #include <linux/kexec.h>
> +#include <linux/ima.h>
>  #include "ima.h"
>  
>  #ifdef CONFIG_IMA_KEXEC
> -- 
> 2.29.2
> 



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux