Re: [PATCH v6 23/26] powerpc/pseries: Pass PLPKS password on kexec

2023-02-10 Thread Stefan Berger




On 2/10/23 03:03, Andrew Donnellan wrote:

From: Russell Currey 

Before interacting with the PLPKS, we ask the hypervisor to generate a
password for the current boot, which is then required for most further
PLPKS operations.

If we kexec into a new kernel, the new kernel will try and fail to
generate a new password, as the password has already been set.

Pass the password through to the new kernel via the device tree, in
/chosen/ibm,plpks-pw. Check for the presence of this property before
trying to generate a new password - if it exists, use the existing
password and remove it from the device tree.

This only works with the kexec_file_load() syscall, not the older
kexec_load() syscall, however if you're using Secure Boot then you want
to be using kexec_file_load() anyway.

Signed-off-by: Russell Currey 
Signed-off-by: Andrew Donnellan 

---

v3: New patch

v4: Fix compile when CONFIG_PSERIES_PLPKS=n (snowpatch)

 Fix error handling on fdt_path_offset() call (ruscur)

v5: Fix DT property name in commit message (npiggin)

 Clear prop in FDT during init to prevent password exposure (mpe)

 Rework to remove ifdefs from C code (npiggin)

v6: Rebase on top of 7294194b47e994753a86eee8cf1c61f3f36458a3 and
 fc546faa559538fb312c77e055243ece18ab3288

 Whitespace (stefanb)

 Use more const (stefanb)

 Get rid of FDT extra space allocation for node overhead, as it
 shouldn't be necessary (ruscur)

 Note kexec_file_load() restriction in commit message
---
  arch/powerpc/include/asm/plpks.h   | 14 ++
  arch/powerpc/kernel/prom.c |  4 ++
  arch/powerpc/kexec/file_load_64.c  | 18 +---
  arch/powerpc/platforms/pseries/plpks.c | 61 ++
  4 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 757313e00521..23b77027c916 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -176,6 +176,20 @@ u64 plpks_get_signedupdatealgorithms(void);
   */
  u16 plpks_get_passwordlen(void);
  
+/**

+ * Called in early init to retrieve and clear the PLPKS password from the DT.
+ */
+void plpks_early_init_devtree(void);
+
+/**
+ * Populates the FDT with the PLPKS password to prepare for kexec.
+ */
+int plpks_populate_fdt(void *fdt);
+#else // CONFIG_PSERIES_PLPKS
+static inline bool plpks_is_available(void) { return false; }
+static inline u16 plpks_get_passwordlen(void) { BUILD_BUG(); }
+static inline void plpks_early_init_devtree(void) { }
+static inline int plpks_populate_fdt(void *fdt) { BUILD_BUG(); }
  #endif // CONFIG_PSERIES_PLPKS
  
  #endif // _ASM_POWERPC_PLPKS_H

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4f1c920aa13e..8a13b378770f 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -56,6 +56,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include 
  
@@ -893,6 +894,9 @@ void __init early_init_devtree(void *params)

powerpc_firmware_features |= FW_FEATURE_PS3_POSSIBLE;
  #endif
  
+	/* If kexec left a PLPKS password in the DT, get it and clear it */

+   plpks_early_init_devtree();
+
tm_init();
  
  	DBG(" <- early_init_devtree()\n");

diff --git a/arch/powerpc/kexec/file_load_64.c 
b/arch/powerpc/kexec/file_load_64.c
index 52085751f5f4..8a9469e1ce71 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -27,6 +27,7 @@
  #include 
  #include 
  #include 
+#include 
  
  struct umem_info {

u64 *buf;   /* data buffer for usable-memory property */
@@ -977,12 +978,17 @@ static unsigned int cpu_node_size(void)
   */
  unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
  {
-   unsigned int cpu_nodes, extra_size;
+   unsigned int cpu_nodes, extra_size = 0;
struct device_node *dn;
u64 usm_entries;
  
+	// Budget some space for the password blob. There's already extra space

+   // for the key name
+   if (plpks_is_available())
+   extra_size += (unsigned int)plpks_get_passwordlen();
+
if (image->type != KEXEC_TYPE_CRASH)
-   return 0;
+   return extra_size;
  
  	/*

 * For kdump kernel, account for linux,usable-memory and
@@ -992,9 +998,7 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage 
*image)
if (drmem_lmb_size()) {
usm_entries = ((memory_hotplug_max() / drmem_lmb_size()) +
   (2 * (resource_size(_res) / 
drmem_lmb_size(;
-   extra_size = (unsigned int)(usm_entries * sizeof(u64));
-   } else {
-   extra_size = 0;
+   extra_size += (unsigned int)(usm_entries * sizeof(u64));
}
  
  	/*

@@ -1233,6 +1237,10 @@ int setup_new_fdt_ppc64(const struct kimage *image, void 
*fdt,
}
}
  
+	// If we have PLPKS active, we need to provide the password to the new kernel


[PATCH v6 23/26] powerpc/pseries: Pass PLPKS password on kexec

2023-02-10 Thread Andrew Donnellan
From: Russell Currey 

Before interacting with the PLPKS, we ask the hypervisor to generate a
password for the current boot, which is then required for most further
PLPKS operations.

If we kexec into a new kernel, the new kernel will try and fail to
generate a new password, as the password has already been set.

Pass the password through to the new kernel via the device tree, in
/chosen/ibm,plpks-pw. Check for the presence of this property before
trying to generate a new password - if it exists, use the existing
password and remove it from the device tree.

This only works with the kexec_file_load() syscall, not the older
kexec_load() syscall, however if you're using Secure Boot then you want
to be using kexec_file_load() anyway.

Signed-off-by: Russell Currey 
Signed-off-by: Andrew Donnellan 

---

v3: New patch

v4: Fix compile when CONFIG_PSERIES_PLPKS=n (snowpatch)

Fix error handling on fdt_path_offset() call (ruscur)

v5: Fix DT property name in commit message (npiggin)

Clear prop in FDT during init to prevent password exposure (mpe)

Rework to remove ifdefs from C code (npiggin)

v6: Rebase on top of 7294194b47e994753a86eee8cf1c61f3f36458a3 and
fc546faa559538fb312c77e055243ece18ab3288

Whitespace (stefanb)

Use more const (stefanb)

Get rid of FDT extra space allocation for node overhead, as it
shouldn't be necessary (ruscur)

Note kexec_file_load() restriction in commit message
---
 arch/powerpc/include/asm/plpks.h   | 14 ++
 arch/powerpc/kernel/prom.c |  4 ++
 arch/powerpc/kexec/file_load_64.c  | 18 +---
 arch/powerpc/platforms/pseries/plpks.c | 61 ++
 4 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 757313e00521..23b77027c916 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -176,6 +176,20 @@ u64 plpks_get_signedupdatealgorithms(void);
  */
 u16 plpks_get_passwordlen(void);
 
+/**
+ * Called in early init to retrieve and clear the PLPKS password from the DT.
+ */
+void plpks_early_init_devtree(void);
+
+/**
+ * Populates the FDT with the PLPKS password to prepare for kexec.
+ */
+int plpks_populate_fdt(void *fdt);
+#else // CONFIG_PSERIES_PLPKS
+static inline bool plpks_is_available(void) { return false; }
+static inline u16 plpks_get_passwordlen(void) { BUILD_BUG(); }
+static inline void plpks_early_init_devtree(void) { }
+static inline int plpks_populate_fdt(void *fdt) { BUILD_BUG(); }
 #endif // CONFIG_PSERIES_PLPKS
 
 #endif // _ASM_POWERPC_PLPKS_H
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4f1c920aa13e..8a13b378770f 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -56,6 +56,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -893,6 +894,9 @@ void __init early_init_devtree(void *params)
powerpc_firmware_features |= FW_FEATURE_PS3_POSSIBLE;
 #endif
 
+   /* If kexec left a PLPKS password in the DT, get it and clear it */
+   plpks_early_init_devtree();
+
tm_init();
 
DBG(" <- early_init_devtree()\n");
diff --git a/arch/powerpc/kexec/file_load_64.c 
b/arch/powerpc/kexec/file_load_64.c
index 52085751f5f4..8a9469e1ce71 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct umem_info {
u64 *buf;   /* data buffer for usable-memory property */
@@ -977,12 +978,17 @@ static unsigned int cpu_node_size(void)
  */
 unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
 {
-   unsigned int cpu_nodes, extra_size;
+   unsigned int cpu_nodes, extra_size = 0;
struct device_node *dn;
u64 usm_entries;
 
+   // Budget some space for the password blob. There's already extra space
+   // for the key name
+   if (plpks_is_available())
+   extra_size += (unsigned int)plpks_get_passwordlen();
+
if (image->type != KEXEC_TYPE_CRASH)
-   return 0;
+   return extra_size;
 
/*
 * For kdump kernel, account for linux,usable-memory and
@@ -992,9 +998,7 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage 
*image)
if (drmem_lmb_size()) {
usm_entries = ((memory_hotplug_max() / drmem_lmb_size()) +
   (2 * (resource_size(_res) / 
drmem_lmb_size(;
-   extra_size = (unsigned int)(usm_entries * sizeof(u64));
-   } else {
-   extra_size = 0;
+   extra_size += (unsigned int)(usm_entries * sizeof(u64));
}
 
/*
@@ -1233,6 +1237,10 @@ int setup_new_fdt_ppc64(const struct kimage *image, void 
*fdt,
}
}
 
+   // If we have PLPKS active, we need to provide the password to the new 
kernel
+   if (plpks_is_available())
+   ret =