From: Thiago Jung Bauermann <bauerman@linux.ibm.com>
To: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Cc: zohar@linux.ibm.com, robh@kernel.org, gregkh@linuxfoundation.org,
james.morse@arm.com, catalin.marinas@arm.com, sashal@kernel.org,
will@kernel.org, mpe@ellerman.id.au, benh@kernel.crashing.org,
paulus@samba.org, robh+dt@kernel.org, frowand.list@gmail.com,
vincenzo.frascino@arm.com, mark.rutland@arm.com,
dmitry.kasatkin@gmail.com, jmorris@namei.org, serge@hallyn.com,
pasha.tatashin@soleen.com, allison@lohutok.net,
kstewart@linuxfoundation.org, takahiro.akashi@linaro.org,
tglx@linutronix.de, masahiroy@kernel.org, bhsharma@redhat.com,
mbrugger@suse.com, hsinyi@chromium.org, tao.li@vivo.com,
christophe.leroy@c-s.fr, linux-integrity@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
prsriva@linux.microsoft.com, balajib@linux.microsoft.com
Subject: Re: [PATCH v10 5/8] powerpc: Move remove_ima_buffer() to drivers/of/kexec.c
Date: Sat, 05 Dec 2020 17:14:22 -0300 [thread overview]
Message-ID: <87sg8km2n5.fsf@manicouagan.localdomain> (raw)
In-Reply-To: <20201204195149.611-6-nramas@linux.microsoft.com>
Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> remove_ima_buffer() removes the chosen node "linux,ima-kexec-buffer"
> from the device tree and frees the memory reserved for carrying forward
> the IMA measurement logs on kexec. This function does not have
> architecture specific code, but is currently limited to powerpc.
>
> Move remove_ima_buffer() to "drivers/of/ima_kexec.c" so that it is
s/ima_kexec./kexec.c/
> accessible for other architectures as well.
>
> Co-developed-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
> Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Looks good. Just minor comments below. Nevertheless:
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> arch/powerpc/include/asm/ima.h | 8 ++------
> arch/powerpc/kexec/ima.c | 26 --------------------------
> drivers/of/kexec.c | 29 +++++++++++++++++++++++++++++
> include/linux/kexec.h | 1 +
> 4 files changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ima.h b/arch/powerpc/include/asm/ima.h
> index ead488cf3981..a2fc71bc3b23 100644
> --- a/arch/powerpc/include/asm/ima.h
> +++ b/arch/powerpc/include/asm/ima.h
> @@ -2,17 +2,13 @@
> #ifndef _ASM_POWERPC_IMA_H
> #define _ASM_POWERPC_IMA_H
>
> +#include <linux/kexec.h>
> +
> struct kimage;
When you include <linux/kexec.h>, there's no need anymore for the
`struct kimage` forward declaration so you can remove it.
>
> 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
> -
This patch introduces a slight behaviour change on powerpc when
CONFIG_IMA isn't set: since remove_ima_buffer() is now defined,
kexec_file_load() will remove the IMA kexec buffer when creating the FDT
for the next kernel. Before this patch, it will leave it there.
I think it's actually an improvement since a stale IMA kexec buffer
isn't useful and just wastes memory. This should be mentioned on the
commit message.
> #ifdef CONFIG_IMA_KEXEC
> int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
> size_t size);
> diff --git a/arch/powerpc/kexec/ima.c b/arch/powerpc/kexec/ima.c
> index 906e8212435d..68017123b07d 100644
> --- a/arch/powerpc/kexec/ima.c
> +++ b/arch/powerpc/kexec/ima.c
> @@ -61,32 +61,6 @@ int ima_free_kexec_buffer(void)
> 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)
> -{
> - int ret;
> - unsigned long addr;
> - size_t size;
> -
> - ret = get_ima_kexec_buffer(fdt, chosen_node, &addr, &size);
> - if (ret)
> - return;
> -
> - fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer");
> -
> - 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
> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> index 516b86f7113a..42d16dfff78d 100644
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
> @@ -129,3 +129,32 @@ int get_ima_kexec_buffer(void *fdt, int chosen_node,
>
> return 0;
> }
> +
> +/**
> + * 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)
> +{
> + int ret;
> + unsigned long addr;
> + size_t size;
> +
> + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> + return;
> +
> + ret = get_ima_kexec_buffer(fdt, chosen_node, &addr, &size);
> + if (ret)
> + return;
> +
> + fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer");
> +
> + ret = delete_fdt_mem_rsv(fdt, addr, size);
> + if (!ret)
> + pr_debug("Removed old IMA buffer reservation.\n");
> +}
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 10ff704ab670..52a0efff184d 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -411,6 +411,7 @@ static inline int kexec_crash_loaded(void) { return 0; }
> extern int get_root_addr_size_cells(int *addr_cells, int *size_cells);
> extern int get_ima_kexec_buffer(void *fdt, int chosen_node,
> unsigned long *addr, size_t *size);
> +extern void remove_ima_buffer(void *fdt, int chosen_node);
> extern int delete_fdt_mem_rsv(void *fdt, unsigned long start,
> unsigned long size);
> #endif /* CONFIG_OF_FLATTREE */
Same comment as before: remove the `extern` keyword.
--
Thiago Jung Bauermann
IBM Linux Technology Center
next prev parent reply other threads:[~2020-12-05 20:16 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-04 19:51 [PATCH v10 0/8] Carry forward IMA measurement log on kexec on ARM64 Lakshmi Ramasubramanian
2020-12-04 19:51 ` [PATCH v10 1/8] powerpc: fix compiler warnings and errors Lakshmi Ramasubramanian
2020-12-05 1:20 ` Thiago Jung Bauermann
2020-12-04 19:51 ` [PATCH v10 2/8] powerpc: Move delete_fdt_mem_rsv() to drivers/of/kexec.c Lakshmi Ramasubramanian
2020-12-05 2:22 ` Thiago Jung Bauermann
2020-12-07 1:50 ` Lakshmi Ramasubramanian
2020-12-11 16:37 ` Lakshmi Ramasubramanian
2020-12-11 18:19 ` Thiago Jung Bauermann
2020-12-11 19:25 ` Lakshmi Ramasubramanian
2020-12-12 1:14 ` Thiago Jung Bauermann
2020-12-04 19:51 ` [PATCH v10 3/8] powerpc: Move ima buffer functions " Lakshmi Ramasubramanian
2020-12-05 19:48 ` Thiago Jung Bauermann
2020-12-07 1:53 ` Lakshmi Ramasubramanian
2020-12-04 19:51 ` [PATCH v10 4/8] powerpc: Use ima kexec node functions Lakshmi Ramasubramanian
2020-12-05 19:51 ` Thiago Jung Bauermann
2020-12-07 1:54 ` Lakshmi Ramasubramanian
2020-12-04 19:51 ` [PATCH v10 5/8] powerpc: Move remove_ima_buffer() to drivers/of/kexec.c Lakshmi Ramasubramanian
2020-12-05 20:14 ` Thiago Jung Bauermann [this message]
2020-12-07 1:57 ` Lakshmi Ramasubramanian
2020-12-04 19:51 ` [PATCH v10 6/8] powerpc: Move ima_get_kexec_buffer() and ima_free_kexec_buffer() to ima Lakshmi Ramasubramanian
2020-12-05 21:02 ` Thiago Jung Bauermann
2020-12-07 1:58 ` Lakshmi Ramasubramanian
2020-12-04 19:51 ` [PATCH v10 7/8] powerpc: Move arch_ima_add_kexec_buffer " Lakshmi Ramasubramanian
2020-12-05 21:36 ` Thiago Jung Bauermann
2020-12-07 2:03 ` Lakshmi Ramasubramanian
2020-12-07 14:57 ` Mimi Zohar
2020-12-04 19:51 ` [PATCH v10 8/8] arm64: Add IMA log information in kimage used for kexec Lakshmi Ramasubramanian
2020-12-05 21:44 ` Thiago Jung Bauermann
2020-12-07 2:05 ` Lakshmi Ramasubramanian
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87sg8km2n5.fsf@manicouagan.localdomain \
--to=bauerman@linux.ibm.com \
--cc=allison@lohutok.net \
--cc=balajib@linux.microsoft.com \
--cc=benh@kernel.crashing.org \
--cc=bhsharma@redhat.com \
--cc=catalin.marinas@arm.com \
--cc=christophe.leroy@c-s.fr \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.kasatkin@gmail.com \
--cc=frowand.list@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=hsinyi@chromium.org \
--cc=james.morse@arm.com \
--cc=jmorris@namei.org \
--cc=kstewart@linuxfoundation.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=masahiroy@kernel.org \
--cc=mbrugger@suse.com \
--cc=mpe@ellerman.id.au \
--cc=nramas@linux.microsoft.com \
--cc=pasha.tatashin@soleen.com \
--cc=paulus@samba.org \
--cc=prsriva@linux.microsoft.com \
--cc=robh+dt@kernel.org \
--cc=robh@kernel.org \
--cc=sashal@kernel.org \
--cc=serge@hallyn.com \
--cc=takahiro.akashi@linaro.org \
--cc=tao.li@vivo.com \
--cc=tglx@linutronix.de \
--cc=vincenzo.frascino@arm.com \
--cc=will@kernel.org \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).