| Message ID | 20201204195149.611-6-nramas@linux.microsoft.com |
|---|---|
| State | New |
| Headers | show |
| Series | Carry forward IMA measurement log on kexec on ARM64 | expand |
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
On 12/5/20 12:14 PM, Thiago Jung Bauermann wrote: > > 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/ Will fix it. > >> 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. Sure - i'll remove the forward declaration. > >> >> 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. Thanks Thiago. Appreciate if you could test this change for powerpc and let me know if you see any issues. >> #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. > Will remove "extern" keyword. -lakshmi
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; 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); 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 */