Re: [PATCH v2 2/4] powerpc: expose secure variables to userspace via sysfs
On Wed, Aug 21, 2019 at 11:08:21AM -0400, Nayna Jain wrote:
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-secvar
> @@ -0,0 +1,27 @@
> +What:/sys/firmware/secvar
> +Date:August 2019
> +Contact: Nayna Jain
> +Description:
> + This directory exposes interfaces for interacting with
> + the secure variables managed by OPAL firmware.
> +
> + This is only for the powerpc/powernv platform.
> +
> + Directory:
> + vars: This directory lists all the variables that
> + are supported by the OPAL. The variables are
> + represented in the form of directories with
> + their variable names. The variable name is
> + unique and is in ASCII representation. The data
> + and size can be determined by reading their
> + respective attribute files.
> +
> + Each variable directory has the following files:
> + name: An ASCII representation of the variable name
> + data: A read-only file containing the value of the
> + variable
> + size: An integer representation of the size of the
> + content of the variable. In other works, it
> + represents the size of the data
> + update: A write-only file that is used to submit the new
> + value for the variable.
Can you break this out into one-entry-per-file like most other entries
are defined? That makes it easier for tools to parse (specifically the
tool in the tree right now...)
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 42109682b727..b4bdf77837b2 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -925,6 +925,15 @@ config PPC_SECURE_BOOT
> allows user to enable OS Secure Boot on PowerPC systems that
> have firmware secure boot support.
>
> +config SECVAR_SYSFS
> +tristate "Enable sysfs interface for POWER secure variables"
> +depends on PPC_SECURE_BOOT
No depends on SYSFS?
> +help
> + POWER secure variables are managed and controlled by firmware.
> + These variables are exposed to userspace via sysfs to enable
> + read/write operations on these variables. Say Y if you have
> + secure boot enabled and want to expose variables to userspace.
Mix of tabs and spaces :(
> +
> endmenu
>
> config ISA_DMA_API
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 9041563f1c74..4ea7b738c3a3 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -158,6 +158,7 @@ obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o
> epapr_hcalls.o
> obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o
>
> obj-$(CONFIG_PPC_SECURE_BOOT)+= secboot.o ima_arch.o secvar-ops.o
> +obj-$(CONFIG_SECVAR_SYSFS) += secvar-sysfs.o
No tab?
>
> # Disable GCOV, KCOV & sanitizers in odd or sensitive code
> GCOV_PROFILE_prom_init.o := n
> diff --git a/arch/powerpc/kernel/secvar-sysfs.c
> b/arch/powerpc/kernel/secvar-sysfs.c
> new file mode 100644
> index ..e46986bb29a0
> --- /dev/null
> +++ b/arch/powerpc/kernel/secvar-sysfs.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 IBM Corporation
> + *
> + * This code exposes secure variables to user via sysfs
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +//Approximating it for now, it is bound to change.
" " before "A" here please.
> +#define VARIABLE_MAX_SIZE 32000
> +
> +static struct kobject *powerpc_kobj;
> +static struct secvar_operations *secvarops;
> +struct kset *secvar_kset;
> +
> +static ssize_t name_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "%s", kobj->name);
> +}
Why do you need this entry as it is the directory name? Userspace
already "knows" it if they can open this file.
> +
> +static ssize_t size_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + unsigned long dsize;
> + int rc;
> +
> + rc = secvarops->get_variable(kobj->name, strlen(kobj->name) + 1, NULL,
> + );
> + if (rc) {
> + pr_err("Error retrieving variable size %d\n", rc);
> + return rc;
> + }
> +
> + rc = sprintf(buf, "%ld", dsize);
> +
> + return rc;
> +}
> +
> +static ssize_t data_read(struct file *filep, struct kobject *kobj,
> + struct bin_attribute *attr, char *buf, loff_t off,
> + size_t count)
> +{
> + unsigned long dsize;
> + int rc;
>
Re: [PATCH v2 3/4] x86/efi: move common keyring handler functions to new file
On Wed, Aug 21, 2019 at 11:08:22AM -0400, Nayna Jain wrote: > This patch moves the common code to keyring_handler.c That says _what_ you are doing, but not _why_ you are doing it. We have no idea :(
Re: [PATCH v2 4/4] powerpc: load firmware trusted keys into kernel keyring
On Wed, Aug 21, 2019 at 11:08:23AM -0400, Nayna Jain wrote: > The keys used to verify the Host OS kernel are managed by OPAL as secure > variables. This patch loads the verification keys into the .platform > keyring and revocation keys into .blacklist keyring. This enables > verification and loading of the kernels signed by the boot time keys which > are trusted by firmware. > > Signed-off-by: Nayna Jain > --- > security/integrity/Kconfig| 9 ++ > security/integrity/Makefile | 3 + > .../integrity/platform_certs/load_powerpc.c | 94 +++ > 3 files changed, 106 insertions(+) > create mode 100644 security/integrity/platform_certs/load_powerpc.c > > diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig > index 0bae6adb63a9..2b4109c157e2 100644 > --- a/security/integrity/Kconfig > +++ b/security/integrity/Kconfig > @@ -72,6 +72,15 @@ config LOAD_IPL_KEYS > depends on S390 > def_bool y > > +config LOAD_PPC_KEYS > + bool "Enable loading of platform and revocation keys for POWER" > + depends on INTEGRITY_PLATFORM_KEYRING > + depends on PPC_SECURE_BOOT > + def_bool y def_bool y only for things that the system will not boot if it is not enabled because you added a new feature. Otherwise just do not set the default. > + help > + Enable loading of db keys to the .platform keyring and dbx keys to > + the .blacklist keyring for powerpc based platforms. > + > config INTEGRITY_AUDIT > bool "Enables integrity auditing support " > depends on AUDIT > diff --git a/security/integrity/Makefile b/security/integrity/Makefile > index 525bf1d6e0db..9eeb6b053de3 100644 > --- a/security/integrity/Makefile > +++ b/security/integrity/Makefile > @@ -14,6 +14,9 @@ integrity-$(CONFIG_LOAD_UEFI_KEYS) += > platform_certs/efi_parser.o \ > platform_certs/load_uefi.o \ > platform_certs/keyring_handler.o > integrity-$(CONFIG_LOAD_IPL_KEYS) += platform_certs/load_ipl_s390.o > +integrity-$(CONFIG_LOAD_PPC_KEYS) += platform_certs/efi_parser.o \ > + platform_certs/load_powerpc.o \ > + platform_certs/keyring_handler.o > $(obj)/load_uefi.o: KBUILD_CFLAGS += -fshort-wchar > subdir-$(CONFIG_IMA) += ima > diff --git a/security/integrity/platform_certs/load_powerpc.c > b/security/integrity/platform_certs/load_powerpc.c > new file mode 100644 > index ..f4d869171062 > --- /dev/null > +++ b/security/integrity/platform_certs/load_powerpc.c > @@ -0,0 +1,94 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2019 IBM Corporation > + * Author: Nayna Jain > + * > + * load_powernv.c That's not the name of this file :( And the perfect example of why you NEVER have the name of the file in the file itself, as it's not needed and easy to get wrong :) thanks, greg k-h
Re: [PATCH v2 2/4] powerpc: expose secure variables to userspace via sysfs
On Wed, 2019-08-21 at 11:08 -0400, Nayna Jain wrote: > PowerNV secure variables, which store the keys used for OS kernel > verification, are managed by the firmware. These secure variables need to > be accessed by the userspace for addition/deletion of the certificates. > > This patch adds the sysfs interface to expose secure variables for PowerNV > secureboot. The users shall use this interface for manipulating > the keys stored in the secure variables. > > Signed-off-by: Nayna Jain > --- > Documentation/ABI/testing/sysfs-secvar | 27 > arch/powerpc/Kconfig | 9 ++ > arch/powerpc/kernel/Makefile | 1 + > arch/powerpc/kernel/secvar-sysfs.c | 210 + > 4 files changed, 247 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-secvar > create mode 100644 arch/powerpc/kernel/secvar-sysfs.c > > diff --git a/Documentation/ABI/testing/sysfs-secvar > b/Documentation/ABI/testing/sysfs-secvar > new file mode 100644 > index ..68f0e03d873d > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-secvar > @@ -0,0 +1,27 @@ > +What:/sys/firmware/secvar > +Date:August 2019 > +Contact: Nayna Jain > +Description: > + This directory exposes interfaces for interacting with > + the secure variables managed by OPAL firmware. > + > + This is only for the powerpc/powernv platform. > + > + Directory: > + vars: This directory lists all the variables that > + are supported by the OPAL. The variables are > + represented in the form of directories with > + their variable names. The variable name is > + unique and is in ASCII representation. The data > + and size can be determined by reading their > + respective attribute files. > + > + Each variable directory has the following files: > + name: An ASCII representation of the variable name > + data: A read-only file containing the value of the > + variable > + size: An integer representation of the size of the > + content of the variable. In other works, it > + represents the size of the data > + update: A write-only file that is used to submit the new > + value for the variable. > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index 42109682b727..b4bdf77837b2 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -925,6 +925,15 @@ config PPC_SECURE_BOOT > allows user to enable OS Secure Boot on PowerPC systems that > have firmware secure boot support. > > +config SECVAR_SYSFS > +tristate "Enable sysfs interface for POWER secure variables" > +depends on PPC_SECURE_BOOT > +help > + POWER secure variables are managed and controlled by firmware. > + These variables are exposed to userspace via sysfs to enable > + read/write operations on these variables. Say Y if you have > + secure boot enabled and want to expose variables to userspace. > + > endmenu > > config ISA_DMA_API > diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile > index 9041563f1c74..4ea7b738c3a3 100644 > --- a/arch/powerpc/kernel/Makefile > +++ b/arch/powerpc/kernel/Makefile > @@ -158,6 +158,7 @@ obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o > epapr_hcalls.o > obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o > > obj-$(CONFIG_PPC_SECURE_BOOT)+= secboot.o ima_arch.o secvar-ops.o > +obj-$(CONFIG_SECVAR_SYSFS) += secvar-sysfs.o > > # Disable GCOV, KCOV & sanitizers in odd or sensitive code > GCOV_PROFILE_prom_init.o := n > diff --git a/arch/powerpc/kernel/secvar-sysfs.c > b/arch/powerpc/kernel/secvar-sysfs.c > new file mode 100644 > index ..e46986bb29a0 > --- /dev/null > +++ b/arch/powerpc/kernel/secvar-sysfs.c > @@ -0,0 +1,210 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2019 IBM Corporation > + * > + * This code exposes secure variables to user via sysfs > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +//Approximating it for now, it is bound to change. > +#define VARIABLE_MAX_SIZE 32000 this needs to be communicated from the secvar backend, maybe via a field in the ops structure? > + > +static struct kobject *powerpc_kobj; Call it secvar_kobj or something. > +static struct secvar_operations *secvarops; Ah, the old I-can't-believe-it's-not-global trick. > +struct kset *secvar_kset; shouldn't that be static too? > + > +static ssize_t name_show(struct kobject *kobj, struct kobj_attribute *attr, > +
Re: [PATCH v2 1/4] powerpc/powernv: Add OPAL API interface to access secure variable
On Wed, 2019-08-21 at 11:08 -0400, Nayna Jain wrote:
> The X.509 certificates trusted by the platform and required to secure boot
> the OS kernel are wrapped in secure variables, which are controlled by
> OPAL.
>
> This patch adds firmware/kernel interface to read and write OPAL secure
> variables based on the unique key.
>
> This support can be enabled using CONFIG_OPAL_SECVAR.
>
> Signed-off-by: Claudio Carvalho
> Signed-off-by: Nayna Jain
> ---
> arch/powerpc/include/asm/opal-api.h | 5 +-
> arch/powerpc/include/asm/opal.h | 6 ++
> arch/powerpc/include/asm/secvar.h| 55 ++
> arch/powerpc/kernel/Makefile | 2 +-
> arch/powerpc/kernel/secvar-ops.c | 25 +
> arch/powerpc/platforms/powernv/Kconfig | 6 ++
> arch/powerpc/platforms/powernv/Makefile | 1 +
> arch/powerpc/platforms/powernv/opal-call.c | 3 +
> arch/powerpc/platforms/powernv/opal-secvar.c | 102 +++
> arch/powerpc/platforms/powernv/opal.c| 5 +
> 10 files changed, 208 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/include/asm/secvar.h
> create mode 100644 arch/powerpc/kernel/secvar-ops.c
> create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c
>
> diff --git a/arch/powerpc/include/asm/opal-api.h
> b/arch/powerpc/include/asm/opal-api.h
> index 383242eb0dea..b238b4f26c5b 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -208,7 +208,10 @@
> #define OPAL_HANDLE_HMI2 166
> #define OPAL_NX_COPROC_INIT 167
> #define OPAL_XIVE_GET_VP_STATE 170
> -#define OPAL_LAST170
> +#define OPAL_SECVAR_GET 173
> +#define OPAL_SECVAR_GET_NEXT174
> +#define OPAL_SECVAR_ENQUEUE_UPDATE 175
> +#define OPAL_LAST 175
>
> #define QUIESCE_HOLD 1 /* Spin all calls at entry */
> #define QUIESCE_REJECT 2 /* Fail all calls with
> OPAL_BUSY */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 57bd029c715e..247adec2375f 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -388,6 +388,12 @@ void opal_powercap_init(void);
> void opal_psr_init(void);
> void opal_sensor_groups_init(void);
Put these with the rest of the OPAL API wrapper prototypes
(i.e. just after opal_nx_coproc_init()) rather than with the
internal functions.
> > +extern int opal_secvar_get(uint64_t k_key, uint64_t k_key_len,
> +uint64_t k_data, uint64_t k_data_size);
> +extern int opal_secvar_get_next(uint64_t k_key, uint64_t k_key_len,
> + uint64_t k_key_size);
> +extern int opal_secvar_enqueue_update(uint64_t k_key, uint64_t k_key_len,
> + uint64_t k_data, uint64_t k_data_size);
Everything in asm/opal.h is intended for consumption by the kernel, so
use a useful kernel type (or annotation) rather than blank uint64_t for
the parameters that are actually pointers. You should also ditch the k_
prefix since it doesn't make much sense having it inside the kernel.
As a general comment, don't use extern on function prototypes. They're
extern by default and, more importantly, it's contrary to the normal
kernel style.
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_POWERPC_OPAL_H */
> diff --git a/arch/powerpc/include/asm/secvar.h
> b/arch/powerpc/include/asm/secvar.h
> new file mode 100644
> index ..645654456265
> --- /dev/null
> +++ b/arch/powerpc/include/asm/secvar.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * PowerPC secure variable operations.
> + *
> + * Copyright (C) 2019 IBM Corporation
> + * Author: Nayna Jain
> + *
> + */
> +#ifndef SECVAR_OPS_H
> +#define SECVAR_OPS_H
> +
> +#include
> +#include
missing space
> +
> +struct secvar_operations {
> + int (*get_variable)(const char *key, unsigned long key_len, u8 *data,
> + unsigned long *data_size);
> + int (*get_next_variable)(const char *key, unsigned long *key_len,
> + unsigned long keysize);
> + int (*set_variable)(const char *key, unsigned long key_len, u8 *data,
> + unsigned long data_size);
> +};
Calling them requires writing code like:
secvar_ops->get_variable(blah);
Why not shorten it to:
secvar_ops->get(blah);
> +#ifdef CONFIG_PPC_SECURE_BOOT
> +
> +extern void set_secvar_ops(struct secvar_operations *ops);
> +extern struct secvar_operations *get_secvar_ops(void);
> +
> +#else
> +
> +static inline void set_secvar_ops(struct secvar_operations *ops)
> +{
> +}
> +
> +static inline struct secvar_operations *get_secvar_ops(void)
> +{
> + return NULL;
> +}
> +
> +#endif
> +#ifdef CONFIG_OPAL_SECVAR
>
Re: [PATCH v2 1/4] powerpc/powernv: Add OPAL API interface to access secure variable
On Thu, Aug 22, 2019 at 3:02 PM Oliver O'Halloran wrote: > > On Wed, 2019-08-21 at 11:08 -0400, Nayna Jain wrote: > > diff --git a/arch/powerpc/platforms/powernv/opal.c > > b/arch/powerpc/platforms/powernv/opal.c > > index aba443be7daa..ffe6f1cf0830 100644 > > --- a/arch/powerpc/platforms/powernv/opal.c > > +++ b/arch/powerpc/platforms/powernv/opal.c > > @@ -32,6 +32,8 @@ > > #include > > #include > > #include > > +#include > > +#include > > > > #include "powernv.h" > > > > @@ -988,6 +990,9 @@ static int __init opal_init(void) > > /* Initialise OPAL Power control interface */ > > opal_power_control_init(); > > > > + if (is_powerpc_secvar_supported()) > > + secvar_init(); > > + > > The usual pattern here is to have the init function check for support > internally. > > Also, is_powerpc_secvar_supported() doesn't appear to be defined > anywhere. Is that supposed to be is_opal_secvar_supported()? Or is this > series supposed to be applied on top of another series? To answer my own question, yes it depends on the series at [1] which adds IMA support. Turns out actually reading the cover letter helps, who knew. That said, I'm still not entirely sure about this. The implementation of is_powerpc_secvar_supported() in [2] parses the DT and seems to assume the DT bindings that OPAL produces. Are those common with the DT bindings produced by OF when running on pseries? [1] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=125961 [2] http://patchwork.ozlabs.org/patch/1149257/ > > > return 0; > > } > > machine_subsys_initcall(powernv, opal_init); >
Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
On Tue, Aug 20, 2019 at 03:28:25PM +0300, Ard Biesheuvel wrote:
> On Tue, 20 Aug 2019 at 14:56, Russell King - ARM Linux admin
> wrote:
> >
> > On Fri, Aug 02, 2019 at 05:38:54AM +, Chester Lin wrote:
> > > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > > index f3ce34113f89..909b11ba48d8 100644
> > > --- a/arch/arm/mm/mmu.c
> > > +++ b/arch/arm/mm/mmu.c
> > > @@ -1184,6 +1184,9 @@ void __init adjust_lowmem_bounds(void)
> > > phys_addr_t block_start = reg->base;
> > > phys_addr_t block_end = reg->base + reg->size;
> > >
> > > + if (memblock_is_nomap(reg))
> > > + continue;
> > > +
> > > if (reg->base < vmalloc_limit) {
> > > if (block_end > lowmem_limit)
> > > /*
> >
> > I think this hunk is sane - if the memory is marked nomap, then it isn't
> > available for the kernel's use, so as far as calculating where the
> > lowmem/highmem boundary is, it effectively doesn't exist and should be
> > skipped.
> >
>
> I agree.
>
> Chester, could you explain what you need beyond this change (and my
> EFI stub change involving TEXT_OFFSET) to make things work on the
> RPi2?
>
Hi Ard,
In fact I am working with Guillaume to try booting zImage kernel and openSUSE
from grub2.04 + arm32-efistub so that's why we get this issue on RPi2, which is
one of the test machines we have. However we want a better solution for all
cases but not just RPi2 since we don't want to affect other platforms as well.
Regards,
Chester
Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
On Wed, 21 Aug 2019 at 10:11, Mike Rapoport wrote:
>
> On Wed, Aug 21, 2019 at 09:35:16AM +0300, Ard Biesheuvel wrote:
> > On Wed, 21 Aug 2019 at 09:11, Chester Lin wrote:
> > >
> > > On Tue, Aug 20, 2019 at 03:28:25PM +0300, Ard Biesheuvel wrote:
> > > > On Tue, 20 Aug 2019 at 14:56, Russell King - ARM Linux admin
> > > > wrote:
> > > > >
> > > > > On Fri, Aug 02, 2019 at 05:38:54AM +, Chester Lin wrote:
> > > > > > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > > > > > index f3ce34113f89..909b11ba48d8 100644
> > > > > > --- a/arch/arm/mm/mmu.c
> > > > > > +++ b/arch/arm/mm/mmu.c
> > > > > > @@ -1184,6 +1184,9 @@ void __init adjust_lowmem_bounds(void)
> > > > > > phys_addr_t block_start = reg->base;
> > > > > > phys_addr_t block_end = reg->base + reg->size;
> > > > > >
> > > > > > + if (memblock_is_nomap(reg))
> > > > > > + continue;
> > > > > > +
> > > > > > if (reg->base < vmalloc_limit) {
> > > > > > if (block_end > lowmem_limit)
> > > > > > /*
> > > > >
> > > > > I think this hunk is sane - if the memory is marked nomap, then it
> > > > > isn't
> > > > > available for the kernel's use, so as far as calculating where the
> > > > > lowmem/highmem boundary is, it effectively doesn't exist and should be
> > > > > skipped.
> > > > >
> > > >
> > > > I agree.
> > > >
> > > > Chester, could you explain what you need beyond this change (and my
> > > > EFI stub change involving TEXT_OFFSET) to make things work on the
> > > > RPi2?
> > > >
> > >
> > > Hi Ard,
> > >
> > > In fact I am working with Guillaume to try booting zImage kernel and
> > > openSUSE
> > > from grub2.04 + arm32-efistub so that's why we get this issue on RPi2,
> > > which is
> > > one of the test machines we have. However we want a better solution for
> > > all
> > > cases but not just RPi2 since we don't want to affect other platforms as
> > > well.
> > >
> >
> > Thanks Chester, but that doesn't answer my question.
> >
> > Your fix is a single patch that changes various things that are only
> > vaguely related. We have already identified that we need to take
> > TEXT_OFFSET (minus some space used by the swapper page tables) into
> > account into the EFI stub if we want to ensure compatibility with many
> > different platforms, and as it turns out, this applies not only to
> > RPi2 but to other platforms as well, most notably the ones that
> > require a TEXT_OFFSET of 0x208000, since they also have reserved
> > regions at the base of RAM.
> >
> > My question was what else we need beyond:
> > - the EFI stub TEXT_OFFSET fix [0]
> > - the change to disregard NOMAP memblocks in adjust_lowmem_bounds()
> > - what else???
>
> I think the only missing part here is to ensure that non-reserved memory in
> bank 0 starts from a PMD-aligned address. I believe this could be done if
> EFI stub, but I'm not really familiar with it so this just a semi-educated
> guess :)
>
Given that it is the ARM arch code that imposes this requirement, how
about adding something like this to adjust_lowmem_bounds():
if (memblock_start_of_DRAM() % PMD_SIZE)
memblock_mark_nomap(memblock_start_of_DRAM(),
PMD_SIZE - (memblock_start_of_DRAM() % PMD_SIZE));
(and introduce the nomap check into the loop)
Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
On Wed, Aug 21, 2019 at 09:35:16AM +0300, Ard Biesheuvel wrote:
> On Wed, 21 Aug 2019 at 09:11, Chester Lin wrote:
> >
> > On Tue, Aug 20, 2019 at 03:28:25PM +0300, Ard Biesheuvel wrote:
> > > On Tue, 20 Aug 2019 at 14:56, Russell King - ARM Linux admin
> > > wrote:
> > > >
> > > > On Fri, Aug 02, 2019 at 05:38:54AM +, Chester Lin wrote:
> > > > > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > > > > index f3ce34113f89..909b11ba48d8 100644
> > > > > --- a/arch/arm/mm/mmu.c
> > > > > +++ b/arch/arm/mm/mmu.c
> > > > > @@ -1184,6 +1184,9 @@ void __init adjust_lowmem_bounds(void)
> > > > > phys_addr_t block_start = reg->base;
> > > > > phys_addr_t block_end = reg->base + reg->size;
> > > > >
> > > > > + if (memblock_is_nomap(reg))
> > > > > + continue;
> > > > > +
> > > > > if (reg->base < vmalloc_limit) {
> > > > > if (block_end > lowmem_limit)
> > > > > /*
> > > >
> > > > I think this hunk is sane - if the memory is marked nomap, then it isn't
> > > > available for the kernel's use, so as far as calculating where the
> > > > lowmem/highmem boundary is, it effectively doesn't exist and should be
> > > > skipped.
> > > >
> > >
> > > I agree.
> > >
> > > Chester, could you explain what you need beyond this change (and my
> > > EFI stub change involving TEXT_OFFSET) to make things work on the
> > > RPi2?
> > >
> >
> > Hi Ard,
> >
> > In fact I am working with Guillaume to try booting zImage kernel and
> > openSUSE
> > from grub2.04 + arm32-efistub so that's why we get this issue on RPi2,
> > which is
> > one of the test machines we have. However we want a better solution for all
> > cases but not just RPi2 since we don't want to affect other platforms as
> > well.
> >
>
> Thanks Chester, but that doesn't answer my question.
>
> Your fix is a single patch that changes various things that are only
> vaguely related. We have already identified that we need to take
> TEXT_OFFSET (minus some space used by the swapper page tables) into
> account into the EFI stub if we want to ensure compatibility with many
> different platforms, and as it turns out, this applies not only to
> RPi2 but to other platforms as well, most notably the ones that
> require a TEXT_OFFSET of 0x208000, since they also have reserved
> regions at the base of RAM.
>
> My question was what else we need beyond:
> - the EFI stub TEXT_OFFSET fix [0]
> - the change to disregard NOMAP memblocks in adjust_lowmem_bounds()
> - what else???
I think the only missing part here is to ensure that non-reserved memory in
bank 0 starts from a PMD-aligned address. I believe this could be done if
EFI stub, but I'm not really familiar with it so this just a semi-educated
guess :)
> [0]
> https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/commit/?h=next=0eb7bad595e52666b642a02862ad996a0f9bfcc0
--
Sincerely yours,
Mike.
Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
On Wed, Aug 21, 2019 at 10:11:01AM +0300, Mike Rapoport wrote:
> On Wed, Aug 21, 2019 at 09:35:16AM +0300, Ard Biesheuvel wrote:
> > On Wed, 21 Aug 2019 at 09:11, Chester Lin wrote:
> > >
> > > On Tue, Aug 20, 2019 at 03:28:25PM +0300, Ard Biesheuvel wrote:
> > > > On Tue, 20 Aug 2019 at 14:56, Russell King - ARM Linux admin
> > > > wrote:
> > > > >
> > > > > On Fri, Aug 02, 2019 at 05:38:54AM +, Chester Lin wrote:
> > > > > > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > > > > > index f3ce34113f89..909b11ba48d8 100644
> > > > > > --- a/arch/arm/mm/mmu.c
> > > > > > +++ b/arch/arm/mm/mmu.c
> > > > > > @@ -1184,6 +1184,9 @@ void __init adjust_lowmem_bounds(void)
> > > > > > phys_addr_t block_start = reg->base;
> > > > > > phys_addr_t block_end = reg->base + reg->size;
> > > > > >
> > > > > > + if (memblock_is_nomap(reg))
> > > > > > + continue;
> > > > > > +
> > > > > > if (reg->base < vmalloc_limit) {
> > > > > > if (block_end > lowmem_limit)
> > > > > > /*
> > > > >
> > > > > I think this hunk is sane - if the memory is marked nomap, then it
> > > > > isn't
> > > > > available for the kernel's use, so as far as calculating where the
> > > > > lowmem/highmem boundary is, it effectively doesn't exist and should be
> > > > > skipped.
> > > > >
> > > >
> > > > I agree.
> > > >
> > > > Chester, could you explain what you need beyond this change (and my
> > > > EFI stub change involving TEXT_OFFSET) to make things work on the
> > > > RPi2?
> > > >
> > >
> > > Hi Ard,
> > >
> > > In fact I am working with Guillaume to try booting zImage kernel and
> > > openSUSE
> > > from grub2.04 + arm32-efistub so that's why we get this issue on RPi2,
> > > which is
> > > one of the test machines we have. However we want a better solution for
> > > all
> > > cases but not just RPi2 since we don't want to affect other platforms as
> > > well.
> > >
> >
> > Thanks Chester, but that doesn't answer my question.
> >
> > Your fix is a single patch that changes various things that are only
> > vaguely related. We have already identified that we need to take
> > TEXT_OFFSET (minus some space used by the swapper page tables) into
> > account into the EFI stub if we want to ensure compatibility with many
> > different platforms, and as it turns out, this applies not only to
> > RPi2 but to other platforms as well, most notably the ones that
> > require a TEXT_OFFSET of 0x208000, since they also have reserved
> > regions at the base of RAM.
> >
> > My question was what else we need beyond:
> > - the EFI stub TEXT_OFFSET fix [0]
> > - the change to disregard NOMAP memblocks in adjust_lowmem_bounds()
> > - what else???
>
> I think the only missing part here is to ensure that non-reserved memory in
> bank 0 starts from a PMD-aligned address. I believe this could be done if
> EFI stub, but I'm not really familiar with it so this just a semi-educated
> guess :)
>
> > [0]
> > https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/commit/?h=next=0eb7bad595e52666b642a02862ad996a0f9bfcc0
>
Hi Ard and Mike,
Sorry for my misunderstanding and I agree with Mike. We could still meet the
memblock_limit issue if there's a non-reserved memory in bank0 starts from an
unaligned address.
Regards,
Chester
Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
On Wed, 21 Aug 2019 at 09:11, Chester Lin wrote:
>
> On Tue, Aug 20, 2019 at 03:28:25PM +0300, Ard Biesheuvel wrote:
> > On Tue, 20 Aug 2019 at 14:56, Russell King - ARM Linux admin
> > wrote:
> > >
> > > On Fri, Aug 02, 2019 at 05:38:54AM +, Chester Lin wrote:
> > > > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > > > index f3ce34113f89..909b11ba48d8 100644
> > > > --- a/arch/arm/mm/mmu.c
> > > > +++ b/arch/arm/mm/mmu.c
> > > > @@ -1184,6 +1184,9 @@ void __init adjust_lowmem_bounds(void)
> > > > phys_addr_t block_start = reg->base;
> > > > phys_addr_t block_end = reg->base + reg->size;
> > > >
> > > > + if (memblock_is_nomap(reg))
> > > > + continue;
> > > > +
> > > > if (reg->base < vmalloc_limit) {
> > > > if (block_end > lowmem_limit)
> > > > /*
> > >
> > > I think this hunk is sane - if the memory is marked nomap, then it isn't
> > > available for the kernel's use, so as far as calculating where the
> > > lowmem/highmem boundary is, it effectively doesn't exist and should be
> > > skipped.
> > >
> >
> > I agree.
> >
> > Chester, could you explain what you need beyond this change (and my
> > EFI stub change involving TEXT_OFFSET) to make things work on the
> > RPi2?
> >
>
> Hi Ard,
>
> In fact I am working with Guillaume to try booting zImage kernel and openSUSE
> from grub2.04 + arm32-efistub so that's why we get this issue on RPi2, which
> is
> one of the test machines we have. However we want a better solution for all
> cases but not just RPi2 since we don't want to affect other platforms as well.
>
Thanks Chester, but that doesn't answer my question.
Your fix is a single patch that changes various things that are only
vaguely related. We have already identified that we need to take
TEXT_OFFSET (minus some space used by the swapper page tables) into
account into the EFI stub if we want to ensure compatibility with many
different platforms, and as it turns out, this applies not only to
RPi2 but to other platforms as well, most notably the ones that
require a TEXT_OFFSET of 0x208000, since they also have reserved
regions at the base of RAM.
My question was what else we need beyond:
- the EFI stub TEXT_OFFSET fix [0]
- the change to disregard NOMAP memblocks in adjust_lowmem_bounds()
- what else???
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/commit/?h=next=0eb7bad595e52666b642a02862ad996a0f9bfcc0
Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
On Wed, 21 Aug 2019 at 11:29, Mike Rapoport wrote:
>
> On Wed, Aug 21, 2019 at 10:29:37AM +0300, Ard Biesheuvel wrote:
> > On Wed, 21 Aug 2019 at 10:11, Mike Rapoport wrote:
> > >
...
> > > I think the only missing part here is to ensure that non-reserved memory
> > > in
> > > bank 0 starts from a PMD-aligned address. I believe this could be done if
> > > EFI stub, but I'm not really familiar with it so this just a semi-educated
> > > guess :)
> > >
> >
> > Given that it is the ARM arch code that imposes this requirement, how
> > about adding something like this to adjust_lowmem_bounds():
> >
> > if (memblock_start_of_DRAM() % PMD_SIZE)
> > memblock_mark_nomap(memblock_start_of_DRAM(),
> > PMD_SIZE - (memblock_start_of_DRAM() % PMD_SIZE));
>
> memblock_start_of_DRAM() won't work here, as it returns the actual start of
> the DRAM including NOMAP regions. Moreover, as we cannot mark a region
> NOMAP inside for_each_memblock() this should be done beforehand.
>
> I think something like this could work:
>
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 2f0f07e..f2b635b 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1178,6 +1178,19 @@ void __init adjust_lowmem_bounds(void)
> */
> vmalloc_limit = (u64)(uintptr_t)vmalloc_min - PAGE_OFFSET +
> PHYS_OFFSET;
>
> + /*
> +* The first usable region must be PMD aligned. Mark its start
> +* as MEMBLOCK_NOMAP if it isn't
> +*/
> + for_each_memblock(memory, reg) {
> + if (!memblock_is_nomap(reg) && (reg->base % PMD_SIZE)) {
> + phys_addr_t size = PMD_SIZE - (reg->base % PMD_SIZE);
> +
> + memblock_mark_nomap(reg->base, size);
> + break;
We should break on the first !NOMAP memblock, even if it is already
PMD aligned, but beyond that, this looks ok to me.
Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
On Wed, Aug 21, 2019 at 10:29:37AM +0300, Ard Biesheuvel wrote:
> On Wed, 21 Aug 2019 at 10:11, Mike Rapoport wrote:
> >
> > On Wed, Aug 21, 2019 at 09:35:16AM +0300, Ard Biesheuvel wrote:
> > > On Wed, 21 Aug 2019 at 09:11, Chester Lin wrote:
> > > >
> > > > On Tue, Aug 20, 2019 at 03:28:25PM +0300, Ard Biesheuvel wrote:
> > > > > On Tue, 20 Aug 2019 at 14:56, Russell King - ARM Linux admin
> > > > > wrote:
> > > > > >
> > > > > > On Fri, Aug 02, 2019 at 05:38:54AM +, Chester Lin wrote:
> > > > > > > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > > > > > > index f3ce34113f89..909b11ba48d8 100644
> > > > > > > --- a/arch/arm/mm/mmu.c
> > > > > > > +++ b/arch/arm/mm/mmu.c
> > > > > > > @@ -1184,6 +1184,9 @@ void __init adjust_lowmem_bounds(void)
> > > > > > > phys_addr_t block_start = reg->base;
> > > > > > > phys_addr_t block_end = reg->base + reg->size;
> > > > > > >
> > > > > > > + if (memblock_is_nomap(reg))
> > > > > > > + continue;
> > > > > > > +
> > > > > > > if (reg->base < vmalloc_limit) {
> > > > > > > if (block_end > lowmem_limit)
> > > > > > > /*
> > > > > >
> > > > > > I think this hunk is sane - if the memory is marked nomap, then it
> > > > > > isn't
> > > > > > available for the kernel's use, so as far as calculating where the
> > > > > > lowmem/highmem boundary is, it effectively doesn't exist and should
> > > > > > be
> > > > > > skipped.
> > > > > >
> > > > >
> > > > > I agree.
> > > > >
> > > > > Chester, could you explain what you need beyond this change (and my
> > > > > EFI stub change involving TEXT_OFFSET) to make things work on the
> > > > > RPi2?
> > > > >
> > > >
> > > > Hi Ard,
> > > >
> > > > In fact I am working with Guillaume to try booting zImage kernel and
> > > > openSUSE
> > > > from grub2.04 + arm32-efistub so that's why we get this issue on RPi2,
> > > > which is
> > > > one of the test machines we have. However we want a better solution for
> > > > all
> > > > cases but not just RPi2 since we don't want to affect other platforms
> > > > as well.
> > > >
> > >
> > > Thanks Chester, but that doesn't answer my question.
> > >
> > > Your fix is a single patch that changes various things that are only
> > > vaguely related. We have already identified that we need to take
> > > TEXT_OFFSET (minus some space used by the swapper page tables) into
> > > account into the EFI stub if we want to ensure compatibility with many
> > > different platforms, and as it turns out, this applies not only to
> > > RPi2 but to other platforms as well, most notably the ones that
> > > require a TEXT_OFFSET of 0x208000, since they also have reserved
> > > regions at the base of RAM.
> > >
> > > My question was what else we need beyond:
> > > - the EFI stub TEXT_OFFSET fix [0]
> > > - the change to disregard NOMAP memblocks in adjust_lowmem_bounds()
> > > - what else???
> >
> > I think the only missing part here is to ensure that non-reserved memory in
> > bank 0 starts from a PMD-aligned address. I believe this could be done if
> > EFI stub, but I'm not really familiar with it so this just a semi-educated
> > guess :)
> >
>
> Given that it is the ARM arch code that imposes this requirement, how
> about adding something like this to adjust_lowmem_bounds():
>
> if (memblock_start_of_DRAM() % PMD_SIZE)
> memblock_mark_nomap(memblock_start_of_DRAM(),
> PMD_SIZE - (memblock_start_of_DRAM() % PMD_SIZE));
memblock_start_of_DRAM() won't work here, as it returns the actual start of
the DRAM including NOMAP regions. Moreover, as we cannot mark a region
NOMAP inside for_each_memblock() this should be done beforehand.
I think something like this could work:
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 2f0f07e..f2b635b 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1178,6 +1178,19 @@ void __init adjust_lowmem_bounds(void)
*/
vmalloc_limit = (u64)(uintptr_t)vmalloc_min - PAGE_OFFSET + PHYS_OFFSET;
+ /*
+* The first usable region must be PMD aligned. Mark its start
+* as MEMBLOCK_NOMAP if it isn't
+*/
+ for_each_memblock(memory, reg) {
+ if (!memblock_is_nomap(reg) && (reg->base % PMD_SIZE)) {
+ phys_addr_t size = PMD_SIZE - (reg->base % PMD_SIZE);
+
+ memblock_mark_nomap(reg->base, size);
+ break;
+ }
+ }
+
for_each_memblock(memory, reg) {
phys_addr_t block_start = reg->base;
phys_addr_t block_end = reg->base + reg->size;
> (and introduce the nomap check into the loop)
--
Sincerely yours,
Mike.
[PATCH v2 1/4] powerpc/powernv: Add OPAL API interface to access secure variable
The X.509 certificates trusted by the platform and required to secure boot
the OS kernel are wrapped in secure variables, which are controlled by
OPAL.
This patch adds firmware/kernel interface to read and write OPAL secure
variables based on the unique key.
This support can be enabled using CONFIG_OPAL_SECVAR.
Signed-off-by: Claudio Carvalho
Signed-off-by: Nayna Jain
---
arch/powerpc/include/asm/opal-api.h | 5 +-
arch/powerpc/include/asm/opal.h | 6 ++
arch/powerpc/include/asm/secvar.h| 55 ++
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/secvar-ops.c | 25 +
arch/powerpc/platforms/powernv/Kconfig | 6 ++
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-call.c | 3 +
arch/powerpc/platforms/powernv/opal-secvar.c | 102 +++
arch/powerpc/platforms/powernv/opal.c| 5 +
10 files changed, 208 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/include/asm/secvar.h
create mode 100644 arch/powerpc/kernel/secvar-ops.c
create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c
diff --git a/arch/powerpc/include/asm/opal-api.h
b/arch/powerpc/include/asm/opal-api.h
index 383242eb0dea..b238b4f26c5b 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -208,7 +208,10 @@
#define OPAL_HANDLE_HMI2 166
#defineOPAL_NX_COPROC_INIT 167
#define OPAL_XIVE_GET_VP_STATE 170
-#define OPAL_LAST 170
+#define OPAL_SECVAR_GET 173
+#define OPAL_SECVAR_GET_NEXT174
+#define OPAL_SECVAR_ENQUEUE_UPDATE 175
+#define OPAL_LAST 175
#define QUIESCE_HOLD 1 /* Spin all calls at entry */
#define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 57bd029c715e..247adec2375f 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -388,6 +388,12 @@ void opal_powercap_init(void);
void opal_psr_init(void);
void opal_sensor_groups_init(void);
+extern int opal_secvar_get(uint64_t k_key, uint64_t k_key_len,
+ uint64_t k_data, uint64_t k_data_size);
+extern int opal_secvar_get_next(uint64_t k_key, uint64_t k_key_len,
+ uint64_t k_key_size);
+extern int opal_secvar_enqueue_update(uint64_t k_key, uint64_t k_key_len,
+ uint64_t k_data, uint64_t k_data_size);
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_OPAL_H */
diff --git a/arch/powerpc/include/asm/secvar.h
b/arch/powerpc/include/asm/secvar.h
new file mode 100644
index ..645654456265
--- /dev/null
+++ b/arch/powerpc/include/asm/secvar.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * PowerPC secure variable operations.
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain
+ *
+ */
+#ifndef SECVAR_OPS_H
+#define SECVAR_OPS_H
+
+#include
+#include
+
+struct secvar_operations {
+ int (*get_variable)(const char *key, unsigned long key_len, u8 *data,
+ unsigned long *data_size);
+ int (*get_next_variable)(const char *key, unsigned long *key_len,
+unsigned long keysize);
+ int (*set_variable)(const char *key, unsigned long key_len, u8 *data,
+ unsigned long data_size);
+};
+
+#ifdef CONFIG_PPC_SECURE_BOOT
+
+extern void set_secvar_ops(struct secvar_operations *ops);
+extern struct secvar_operations *get_secvar_ops(void);
+
+#else
+
+static inline void set_secvar_ops(struct secvar_operations *ops)
+{
+}
+
+static inline struct secvar_operations *get_secvar_ops(void)
+{
+ return NULL;
+}
+
+#endif
+
+#ifdef CONFIG_OPAL_SECVAR
+
+extern int secvar_init(void);
+
+#else
+
+static inline int secvar_init(void)
+{
+ return -EINVAL;
+}
+
+#endif
+
+#endif
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 520b1c814197..9041563f1c74 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -157,7 +157,7 @@ endif
obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o epapr_hcalls.o
obj-$(CONFIG_KVM_GUEST)+= kvm.o kvm_emul.o
-obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o
+obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o secvar-ops.o
# Disable GCOV, KCOV & sanitizers in odd or sensitive code
GCOV_PROFILE_prom_init.o := n
diff --git a/arch/powerpc/kernel/secvar-ops.c b/arch/powerpc/kernel/secvar-ops.c
new file mode 100644
index ..198222499848
--- /dev/null
+++ b/arch/powerpc/kernel/secvar-ops.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna
[PATCH v2 0/4] powerpc: expose secure variables to the kernel and userspace
In order to verify the OS kernel on PowerNV systems, secure boot requires X.509 certificates trusted by the platform. These are stored in secure variables controlled by OPAL, called OPAL secure variables. In order to enable users to manage the keys, the secure variables need to be exposed to userspace. OPAL provides the runtime services for the kernel to be able to access the secure variables[1]. This patchset defines the kernel interface for the OPAL APIs. These APIs are used by the hooks, which load these variables to the keyring and expose them to the userspace for reading/writing. The previous version[2] of the patchset added support only for the sysfs interface. This patch adds two more patches that involves loading of the firmware trusted keys to the kernel keyring. This patchset is dependent on the base CONFIG PPC_SECURE_BOOT added by ima arch specific patches for POWER[3] Overall, this patchset adds the following support: * expose secure variables to the kernel via OPAL Runtime API interface * expose secure variables to the userspace via kernel sysfs interface * load kernel verification and revocation keys to .platform and .blacklist keyring respectively. The secure variables can be read/written using simple linux utilities cat/hexdump. For example: Path to the secure variables is: /sys/firmware/secvar/vars Each secure variable is listed as directory. $ ls -l total 0 drwxr-xr-x. 2 root root 0 Aug 20 21:20 db drwxr-xr-x. 2 root root 0 Aug 20 21:20 KEK drwxr-xr-x. 2 root root 0 Aug 20 21:20 PK The attributes of each of the secure variables are(for example: PK): [PK]$ ls -l total 0 -r--r--r--. 1 root root 32000 Aug 21 08:28 data -r--r--r--. 1 root root 65536 Aug 21 08:28 name -r--r--r--. 1 root root 65536 Aug 21 08:28 size --w---. 1 root root 32000 Aug 21 08:28 update The "data" is used to read the existing variable value using hexdump. The data is stored in ESL format. The "update" is used to write a new value using cat. The update is to be submitted as AUTH file. [1] Depends on skiboot OPAL API changes which removes metadata from the API. The new version with the changes are going to be posted soon. [2] https://lkml.org/lkml/2019/6/13/1644 [3] https://lkml.org/lkml/2019/8/19/402 Changelog: v2: * removes complete efi-sms from the sysfs implementation and is simplified * includes Greg's and Oliver's feedbacks: * adds sysfs documentation * moves sysfs code to arch/powerpc * other code related feedbacks. * adds two new patches to load keys to .platform and .blacklist keyring. These patches are added to this series as they are also dependent on OPAL APIs. Nayna Jain (4): powerpc/powernv: Add OPAL API interface to access secure variable powerpc: expose secure variables to userspace via sysfs x86/efi: move common keyring handler functions to new file powerpc: load firmware trusted keys into kernel keyring Documentation/ABI/testing/sysfs-secvar| 27 +++ arch/powerpc/Kconfig | 9 + arch/powerpc/include/asm/opal-api.h | 5 +- arch/powerpc/include/asm/opal.h | 6 + arch/powerpc/include/asm/secvar.h | 55 + arch/powerpc/kernel/Makefile | 3 +- arch/powerpc/kernel/secvar-ops.c | 25 +++ arch/powerpc/kernel/secvar-sysfs.c| 210 ++ arch/powerpc/platforms/powernv/Kconfig| 6 + arch/powerpc/platforms/powernv/Makefile | 1 + arch/powerpc/platforms/powernv/opal-call.c| 3 + arch/powerpc/platforms/powernv/opal-secvar.c | 102 + arch/powerpc/platforms/powernv/opal.c | 5 + security/integrity/Kconfig| 9 + security/integrity/Makefile | 6 +- .../platform_certs/keyring_handler.c | 80 +++ .../platform_certs/keyring_handler.h | 35 +++ .../integrity/platform_certs/load_powerpc.c | 94 security/integrity/platform_certs/load_uefi.c | 67 +- 19 files changed, 679 insertions(+), 69 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-secvar create mode 100644 arch/powerpc/include/asm/secvar.h create mode 100644 arch/powerpc/kernel/secvar-ops.c create mode 100644 arch/powerpc/kernel/secvar-sysfs.c create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c create mode 100644 security/integrity/platform_certs/keyring_handler.c create mode 100644 security/integrity/platform_certs/keyring_handler.h create mode 100644 security/integrity/platform_certs/load_powerpc.c -- 2.20.1
[PATCH v2 2/4] powerpc: expose secure variables to userspace via sysfs
PowerNV secure variables, which store the keys used for OS kernel
verification, are managed by the firmware. These secure variables need to
be accessed by the userspace for addition/deletion of the certificates.
This patch adds the sysfs interface to expose secure variables for PowerNV
secureboot. The users shall use this interface for manipulating
the keys stored in the secure variables.
Signed-off-by: Nayna Jain
---
Documentation/ABI/testing/sysfs-secvar | 27
arch/powerpc/Kconfig | 9 ++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/secvar-sysfs.c | 210 +
4 files changed, 247 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-secvar
create mode 100644 arch/powerpc/kernel/secvar-sysfs.c
diff --git a/Documentation/ABI/testing/sysfs-secvar
b/Documentation/ABI/testing/sysfs-secvar
new file mode 100644
index ..68f0e03d873d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-secvar
@@ -0,0 +1,27 @@
+What: /sys/firmware/secvar
+Date: August 2019
+Contact: Nayna Jain
+Description:
+ This directory exposes interfaces for interacting with
+ the secure variables managed by OPAL firmware.
+
+ This is only for the powerpc/powernv platform.
+
+ Directory:
+ vars: This directory lists all the variables that
+ are supported by the OPAL. The variables are
+ represented in the form of directories with
+ their variable names. The variable name is
+ unique and is in ASCII representation. The data
+ and size can be determined by reading their
+ respective attribute files.
+
+ Each variable directory has the following files:
+ name: An ASCII representation of the variable name
+ data: A read-only file containing the value of the
+ variable
+ size: An integer representation of the size of the
+ content of the variable. In other works, it
+ represents the size of the data
+ update: A write-only file that is used to submit the new
+ value for the variable.
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 42109682b727..b4bdf77837b2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -925,6 +925,15 @@ config PPC_SECURE_BOOT
allows user to enable OS Secure Boot on PowerPC systems that
have firmware secure boot support.
+config SECVAR_SYSFS
+tristate "Enable sysfs interface for POWER secure variables"
+depends on PPC_SECURE_BOOT
+help
+ POWER secure variables are managed and controlled by firmware.
+ These variables are exposed to userspace via sysfs to enable
+ read/write operations on these variables. Say Y if you have
+ secure boot enabled and want to expose variables to userspace.
+
endmenu
config ISA_DMA_API
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 9041563f1c74..4ea7b738c3a3 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -158,6 +158,7 @@ obj-$(CONFIG_EPAPR_PARAVIRT)+= epapr_paravirt.o
epapr_hcalls.o
obj-$(CONFIG_KVM_GUEST)+= kvm.o kvm_emul.o
obj-$(CONFIG_PPC_SECURE_BOOT) += secboot.o ima_arch.o secvar-ops.o
+obj-$(CONFIG_SECVAR_SYSFS) += secvar-sysfs.o
# Disable GCOV, KCOV & sanitizers in odd or sensitive code
GCOV_PROFILE_prom_init.o := n
diff --git a/arch/powerpc/kernel/secvar-sysfs.c
b/arch/powerpc/kernel/secvar-sysfs.c
new file mode 100644
index ..e46986bb29a0
--- /dev/null
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 IBM Corporation
+ *
+ * This code exposes secure variables to user via sysfs
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+//Approximating it for now, it is bound to change.
+#define VARIABLE_MAX_SIZE 32000
+
+static struct kobject *powerpc_kobj;
+static struct secvar_operations *secvarops;
+struct kset *secvar_kset;
+
+static ssize_t name_show(struct kobject *kobj, struct kobj_attribute *attr,
+char *buf)
+{
+ return sprintf(buf, "%s", kobj->name);
+}
+
+static ssize_t size_show(struct kobject *kobj, struct kobj_attribute *attr,
+char *buf)
+{
+ unsigned long dsize;
+ int rc;
+
+ rc = secvarops->get_variable(kobj->name, strlen(kobj->name) + 1, NULL,
+);
+ if (rc) {
+ pr_err("Error retrieving variable size %d\n", rc);
+
[PATCH v2 3/4] x86/efi: move common keyring handler functions to new file
This patch moves the common code to keyring_handler.c
Signed-off-by: Nayna Jain
---
security/integrity/Makefile | 3 +-
.../platform_certs/keyring_handler.c | 80 +++
.../platform_certs/keyring_handler.h | 35
security/integrity/platform_certs/load_uefi.c | 67 +---
4 files changed, 118 insertions(+), 67 deletions(-)
create mode 100644 security/integrity/platform_certs/keyring_handler.c
create mode 100644 security/integrity/platform_certs/keyring_handler.h
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 19faace69644..525bf1d6e0db 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -11,7 +11,8 @@ integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) +=
platform_certs/platform_keyring.o
integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \
- platform_certs/load_uefi.o
+ platform_certs/load_uefi.o \
+ platform_certs/keyring_handler.o
integrity-$(CONFIG_LOAD_IPL_KEYS) += platform_certs/load_ipl_s390.o
$(obj)/load_uefi.o: KBUILD_CFLAGS += -fshort-wchar
diff --git a/security/integrity/platform_certs/keyring_handler.c
b/security/integrity/platform_certs/keyring_handler.c
new file mode 100644
index ..c5ba695c10e3
--- /dev/null
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "../integrity.h"
+
+static efi_guid_t efi_cert_x509_guid __initdata = EFI_CERT_X509_GUID;
+static efi_guid_t efi_cert_x509_sha256_guid __initdata =
+ EFI_CERT_X509_SHA256_GUID;
+static efi_guid_t efi_cert_sha256_guid __initdata = EFI_CERT_SHA256_GUID;
+
+/*
+ * Blacklist a hash.
+ */
+static __init void uefi_blacklist_hash(const char *source, const void *data,
+ size_t len, const char *type,
+ size_t type_len)
+{
+ char *hash, *p;
+
+ hash = kmalloc(type_len + len * 2 + 1, GFP_KERNEL);
+ if (!hash)
+ return;
+ p = memcpy(hash, type, type_len);
+ p += type_len;
+ bin2hex(p, data, len);
+ p += len * 2;
+ *p = 0;
+
+ mark_hash_blacklisted(hash);
+ kfree(hash);
+}
+
+/*
+ * Blacklist an X509 TBS hash.
+ */
+static __init void uefi_blacklist_x509_tbs(const char *source,
+ const void *data, size_t len)
+{
+ uefi_blacklist_hash(source, data, len, "tbs:", 4);
+}
+
+/*
+ * Blacklist the hash of an executable.
+ */
+static __init void uefi_blacklist_binary(const char *source,
+const void *data, size_t len)
+{
+ uefi_blacklist_hash(source, data, len, "bin:", 4);
+}
+
+/*
+ * Return the appropriate handler for particular signature list types found in
+ * the UEFI db and MokListRT tables.
+ */
+__init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
+{
+ if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0)
+ return add_to_platform_keyring;
+ return 0;
+}
+
+/*
+ * Return the appropriate handler for particular signature list types found in
+ * the UEFI dbx and MokListXRT tables.
+ */
+__init efi_element_handler_t get_handler_for_dbx(const efi_guid_t *sig_type)
+{
+ if (efi_guidcmp(*sig_type, efi_cert_x509_sha256_guid) == 0)
+ return uefi_blacklist_x509_tbs;
+ if (efi_guidcmp(*sig_type, efi_cert_sha256_guid) == 0)
+ return uefi_blacklist_binary;
+ return 0;
+}
diff --git a/security/integrity/platform_certs/keyring_handler.h
b/security/integrity/platform_certs/keyring_handler.h
new file mode 100644
index ..829a14b95218
--- /dev/null
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain
+ */
+#ifndef PLATFORM_CERTS_INTERNAL_H
+#define PLATFORM_CERTS_INTERNAL_H
+
+#include
+
+void blacklist_hash(const char *source, const void *data,
+ size_t len, const char *type,
+ size_t type_len);
+
+/*
+ * Blacklist an X509 TBS hash.
+ */
+void blacklist_x509_tbs(const char *source, const void *data, size_t len);
+
+/*
+ * Blacklist the hash of an executable.
+ */
+void blacklist_binary(const char *source, const void *data, size_t len);
+
+/*
+ * Return the handler for particular signature list types found in the db.
+ */
+efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type);
+
+/*
+ * Return the handler for particular signature list types found in the dbx.
+ */
+efi_element_handler_t
[PATCH v2 4/4] powerpc: load firmware trusted keys into kernel keyring
The keys used to verify the Host OS kernel are managed by OPAL as secure
variables. This patch loads the verification keys into the .platform
keyring and revocation keys into .blacklist keyring. This enables
verification and loading of the kernels signed by the boot time keys which
are trusted by firmware.
Signed-off-by: Nayna Jain
---
security/integrity/Kconfig| 9 ++
security/integrity/Makefile | 3 +
.../integrity/platform_certs/load_powerpc.c | 94 +++
3 files changed, 106 insertions(+)
create mode 100644 security/integrity/platform_certs/load_powerpc.c
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 0bae6adb63a9..2b4109c157e2 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -72,6 +72,15 @@ config LOAD_IPL_KEYS
depends on S390
def_bool y
+config LOAD_PPC_KEYS
+ bool "Enable loading of platform and revocation keys for POWER"
+ depends on INTEGRITY_PLATFORM_KEYRING
+ depends on PPC_SECURE_BOOT
+ def_bool y
+ help
+ Enable loading of db keys to the .platform keyring and dbx keys to
+ the .blacklist keyring for powerpc based platforms.
+
config INTEGRITY_AUDIT
bool "Enables integrity auditing support "
depends on AUDIT
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 525bf1d6e0db..9eeb6b053de3 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -14,6 +14,9 @@ integrity-$(CONFIG_LOAD_UEFI_KEYS) +=
platform_certs/efi_parser.o \
platform_certs/load_uefi.o \
platform_certs/keyring_handler.o
integrity-$(CONFIG_LOAD_IPL_KEYS) += platform_certs/load_ipl_s390.o
+integrity-$(CONFIG_LOAD_PPC_KEYS) += platform_certs/efi_parser.o \
+platform_certs/load_powerpc.o \
+platform_certs/keyring_handler.o
$(obj)/load_uefi.o: KBUILD_CFLAGS += -fshort-wchar
subdir-$(CONFIG_IMA) += ima
diff --git a/security/integrity/platform_certs/load_powerpc.c
b/security/integrity/platform_certs/load_powerpc.c
new file mode 100644
index ..f4d869171062
--- /dev/null
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain
+ *
+ * load_powernv.c
+ * - loads keys and certs stored and controlled
+ * by the firmware.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "keyring_handler.h"
+
+static struct secvar_operations *secvarops;
+
+/*
+ * Get a certificate list blob from the named EFI variable.
+ */
+static __init void *get_cert_list(u8 *key, unsigned long keylen,
+ unsigned long *size)
+{
+ int rc;
+ void *db;
+
+ rc = secvarops->get_variable(key, keylen, NULL, size);
+ if (rc) {
+ pr_err("Couldn't get size: %d\n", rc);
+ return NULL;
+ }
+
+ db = kmalloc(*size, GFP_KERNEL);
+ if (!db)
+ return NULL;
+
+ rc = secvarops->get_variable(key, keylen, db, size);
+ if (rc) {
+ kfree(db);
+ pr_err("Error reading db var: %d\n", rc);
+ return NULL;
+ }
+
+ return db;
+}
+
+/*
+ * Load the certs contained in the UEFI databases into the platform trusted
+ * keyring and the UEFI blacklisted X.509 cert SHA256 hashes into the blacklist
+ * keyring.
+ */
+static int __init load_powerpc_certs(void)
+{
+ void *db = NULL, *dbx = NULL;
+ unsigned long dbsize = 0, dbxsize = 0;
+ int rc = 0;
+
+ secvarops = get_secvar_ops();
+ if (!secvarops)
+ return -ENOENT;
+
+ /* Get db, and dbx. They might not exist, so it isn't
+* an error if we can't get them.
+*/
+ db = get_cert_list("db", 3, );
+ if (!db) {
+ pr_err("Couldn't get db list from OPAL\n");
+ } else {
+ rc = parse_efi_signature_list("OPAL:db",
+ db, dbsize, get_handler_for_db);
+ if (rc)
+ pr_err("Couldn't parse db signatures: %d\n",
+ rc);
+ kfree(db);
+ }
+
+ dbx = get_cert_list("dbx", 3, );
+ if (!dbx) {
+ pr_info("Couldn't get dbx list from OPAL\n");
+ } else {
+ rc = parse_efi_signature_list("OPAL:dbx",
+ dbx, dbxsize,
+ get_handler_for_dbx);
+ if (rc)
+ pr_err("Couldn't parse dbx signatures: %d\n", rc);
+ kfree(dbx);
+ }
+
+ return rc;
+}
+late_initcall(load_powerpc_certs);
--
2.20.1
