On Fri Jan 20, 2023 at 5:43 PM AEST, Andrew Donnellan wrote:
> From: Russell Currey <ruscur@xxxxxxxxxx>
>
> 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/plpks-pw. Check for the presence of this property before trying
In /chosen/ibm,plpks-pw
> to generate a new password - if it exists, use the existing password and
> remove it from the device tree.
>
> Signed-off-by: Russell Currey <ruscur@xxxxxxxxxx>
> Signed-off-by: Andrew Donnellan <ajd@xxxxxxxxxxxxx>
>
> ---
>
> v3: New patch
>
> v4: Fix compile when CONFIG_PSERIES_PLPKS=n (snowpatch)
>
> Fix error handling on fdt_path_offset() call (ruscur)
> ---
> arch/powerpc/kexec/file_load_64.c | 18 ++++++++++++++++++
> arch/powerpc/platforms/pseries/plpks.c | 18 +++++++++++++++++-
> 2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index af8854f9eae3..0c9130af60cc 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -27,6 +27,7 @@
> #include <asm/kexec_ranges.h>
> #include <asm/crashdump-ppc64.h>
> #include <asm/prom.h>
> +#include <asm/plpks.h>
>
> struct umem_info {
> u64 *buf; /* data buffer for usable-memory property */
> @@ -1156,6 +1157,9 @@ int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
> {
> struct crash_mem *umem = NULL, *rmem = NULL;
> int i, nr_ranges, ret;
> +#ifdef CONFIG_PSERIES_PLPKS
> + int chosen_offset;
> +#endif
Could put this in plpks_is_available and avoid an ifdef.
>
> /*
> * Restrict memory usage for kdump kernel by setting up
> @@ -1230,6 +1234,20 @@ int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
> }
> }
>
> +#ifdef CONFIG_PSERIES_PLPKS
> + // If we have PLPKS active, we need to provide the password
> + if (plpks_is_available()) {
> + chosen_offset = fdt_path_offset(fdt, "/chosen");
> + if (chosen_offset < 0) {
> + pr_err("Can't find chosen node: %s\n",
> + fdt_strerror(chosen_offset));
> + goto out;
> + }
> + ret = fdt_setprop(fdt, chosen_offset, "ibm,plpks-pw",
> + plpks_get_password(), plpks_get_passwordlen());
> + }
> +#endif // CONFIG_PSERIES_PLPKS
I think if you define plpks_get_password and plpks_get_passwordlen as
BUILD_BUG_ON when PLPKS is not configured and plpks_is_available as
false, you could remove the ifdef entirely.
> +
> out:
> kfree(rmem);
> kfree(umem);
> diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
> index b3c7410a4f13..0350f10e1755 100644
> --- a/arch/powerpc/platforms/pseries/plpks.c
> +++ b/arch/powerpc/platforms/pseries/plpks.c
> @@ -16,6 +16,7 @@
> #include <linux/slab.h>
> #include <linux/string.h>
> #include <linux/types.h>
> +#include <linux/of.h>
> #include <asm/hvcall.h>
> #include <asm/machdep.h>
> #include <asm/plpks.h>
> @@ -126,7 +127,22 @@ static int plpks_gen_password(void)
> {
> unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
> u8 *password, consumer = PLPKS_OS_OWNER;
> - int rc;
> + struct property *prop;
> + int rc, len;
> +
> + // Before we generate the password, we may have been booted by kexec and
> + // provided with a previous password. Check for that first.
So not really generating the password then. Should it be in a different
function the caller makes first?
> + prop = of_find_property(of_chosen, "ibm,plpks-pw", &len);
> + if (prop) {
> + ospasswordlength = (u16)len;
> + ospassword = kzalloc(ospasswordlength, GFP_KERNEL);
> + if (!ospassword) {
> + of_remove_property(of_chosen, prop);
> + return -ENOMEM;
> + }
> + memcpy(ospassword, prop->value, len);
> + return of_remove_property(of_chosen, prop);
Why do you remove the property afterward?
Thanks,
Nick
