All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] powerpc: expose secure variables to the kernel and userspace
@ 2019-08-26 13:23 ` Nayna Jain
  0 siblings, 0 replies; 37+ messages in thread
From: Nayna Jain @ 2019-08-26 13:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: linux-kernel, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Ard Biesheuvel, Jeremy Kerr, Matthew Garret,
	Mimi Zohar, Greg Kroah-Hartman, Claudio Carvalho, George Wilson,
	Elaine Palmer, Eric Ricther, Oliver O'Halloran, Nayna Jain

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 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:
v3:
* includes Greg's feedbacks:
 * fixes in Patch 2/4
   * updates the Documentation.
   * fixes code feedbacks
    * adds SYSFS Kconfig dependency for SECVAR_SYSFS
    * fixes mixed tabs and spaces
    * removes "name" attribute for each of the variable name based
    directories
    * fixes using __ATTR_RO() and __BIN_ATTR_RO() and statics and const
    * fixes the racing issue by using kobj_type default groups. Also,
    fixes the kobject leakage.
    * removes extra print messages
  * updates patch description for Patch 3/4
  * removes file name from Patch 4/4 file header comment and removed
  def_bool y from the LOAD_PPC_KEYS Kconfig

* includes Oliver's feedbacks:
  * fixes Patch 1/2
   * moves OPAL API wrappers after opal_nx_proc_init(), fixed the
   naming, types and removed extern.
   * fixes spaces
   * renames get_variable() to get(), get_next_variable() to get_next()
   and set_variable() to set()
   * removed get_secvar_ops() and defined secvar_ops as global
   * fixes consts and statics
   * removes generic secvar_init() and defined platform specific
   opal_secar_init()
   * updates opal_secvar_supported() to check for secvar support even
   before checking the OPAL APIs support and also fixed the error codes.
   * addes function that converts OPAL return codes to linux errno
   * moves secvar check support in the opal_secvar_init() and defined its
   prototype in opal.h
  * fixes Patch 2/2
   * fixes static/const
   * defines macro for max name size
   * replaces OPAL error codes with linux errno and also updated error
   handling
   * moves secvar support check before creating sysfs kobjects in 
   secvar_sysfs_init()
   * fixes spaces  

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/hashes into kernel keyring

 Documentation/ABI/testing/sysfs-secvar        |  37 ++++
 arch/powerpc/Kconfig                          |  10 +
 arch/powerpc/include/asm/opal-api.h           |   5 +-
 arch/powerpc/include/asm/opal.h               |   7 +-
 arch/powerpc/include/asm/powernv.h            |   2 +
 arch/powerpc/include/asm/secvar.h             |  35 +++
 arch/powerpc/kernel/Makefile                  |   3 +-
 arch/powerpc/kernel/secvar-ops.c              |  19 ++
 arch/powerpc/kernel/secvar-sysfs.c            | 200 ++++++++++++++++++
 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  | 138 ++++++++++++
 arch/powerpc/platforms/powernv/opal.c         |   5 +
 security/integrity/Kconfig                    |   8 +
 security/integrity/Makefile                   |   6 +-
 .../platform_certs/keyring_handler.c          |  80 +++++++
 .../platform_certs/keyring_handler.h          |  32 +++
 .../integrity/platform_certs/load_powerpc.c   |  88 ++++++++
 security/integrity/platform_certs/load_uefi.c |  67 +-----
 20 files changed, 682 insertions(+), 70 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


^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH v3 0/4] powerpc: expose secure variables to the kernel and userspace
@ 2019-08-26 13:23 ` Nayna Jain
  0 siblings, 0 replies; 37+ messages in thread
From: Nayna Jain @ 2019-08-26 13:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

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 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:
v3:
* includes Greg's feedbacks:
 * fixes in Patch 2/4
   * updates the Documentation.
   * fixes code feedbacks
    * adds SYSFS Kconfig dependency for SECVAR_SYSFS
    * fixes mixed tabs and spaces
    * removes "name" attribute for each of the variable name based
    directories
    * fixes using __ATTR_RO() and __BIN_ATTR_RO() and statics and const
    * fixes the racing issue by using kobj_type default groups. Also,
    fixes the kobject leakage.
    * removes extra print messages
  * updates patch description for Patch 3/4
  * removes file name from Patch 4/4 file header comment and removed
  def_bool y from the LOAD_PPC_KEYS Kconfig

* includes Oliver's feedbacks:
  * fixes Patch 1/2
   * moves OPAL API wrappers after opal_nx_proc_init(), fixed the
   naming, types and removed extern.
   * fixes spaces
   * renames get_variable() to get(), get_next_variable() to get_next()
   and set_variable() to set()
   * removed get_secvar_ops() and defined secvar_ops as global
   * fixes consts and statics
   * removes generic secvar_init() and defined platform specific
   opal_secar_init()
   * updates opal_secvar_supported() to check for secvar support even
   before checking the OPAL APIs support and also fixed the error codes.
   * addes function that converts OPAL return codes to linux errno
   * moves secvar check support in the opal_secvar_init() and defined its
   prototype in opal.h
  * fixes Patch 2/2
   * fixes static/const
   * defines macro for max name size
   * replaces OPAL error codes with linux errno and also updated error
   handling
   * moves secvar support check before creating sysfs kobjects in 
   secvar_sysfs_init()
   * fixes spaces  

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/hashes into kernel keyring

 Documentation/ABI/testing/sysfs-secvar        |  37 ++++
 arch/powerpc/Kconfig                          |  10 +
 arch/powerpc/include/asm/opal-api.h           |   5 +-
 arch/powerpc/include/asm/opal.h               |   7 +-
 arch/powerpc/include/asm/powernv.h            |   2 +
 arch/powerpc/include/asm/secvar.h             |  35 +++
 arch/powerpc/kernel/Makefile                  |   3 +-
 arch/powerpc/kernel/secvar-ops.c              |  19 ++
 arch/powerpc/kernel/secvar-sysfs.c            | 200 ++++++++++++++++++
 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  | 138 ++++++++++++
 arch/powerpc/platforms/powernv/opal.c         |   5 +
 security/integrity/Kconfig                    |   8 +
 security/integrity/Makefile                   |   6 +-
 .../platform_certs/keyring_handler.c          |  80 +++++++
 .../platform_certs/keyring_handler.h          |  32 +++
 .../integrity/platform_certs/load_powerpc.c   |  88 ++++++++
 security/integrity/platform_certs/load_uefi.c |  67 +-----
 20 files changed, 682 insertions(+), 70 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


^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH v3 1/4] powerpc/powernv: Add OPAL API interface to access secure variable
  2019-08-26 13:23 ` Nayna Jain
@ 2019-08-26 13:23   ` Nayna Jain
  -1 siblings, 0 replies; 37+ messages in thread
From: Nayna Jain @ 2019-08-26 13:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: linux-kernel, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Ard Biesheuvel, Jeremy Kerr, Matthew Garret,
	Mimi Zohar, Greg Kroah-Hartman, Claudio Carvalho, George Wilson,
	Elaine Palmer, Eric Ricther, Oliver O'Halloran, Nayna Jain

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 <cclaudio@linux.ibm.com>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 arch/powerpc/include/asm/opal-api.h          |   5 +-
 arch/powerpc/include/asm/opal.h              |   7 +-
 arch/powerpc/include/asm/powernv.h           |   2 +
 arch/powerpc/include/asm/secvar.h            |  35 +++++
 arch/powerpc/kernel/Makefile                 |   2 +-
 arch/powerpc/kernel/secvar-ops.c             |  19 +++
 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 | 138 +++++++++++++++++++
 arch/powerpc/platforms/powernv/opal.c        |   5 +
 11 files changed, 220 insertions(+), 3 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_LAST				170
+#define OPAL_SECVAR_GET                         173
+#define OPAL_SECVAR_GET_NEXT                    174
+#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..0606b1d22db4 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -296,7 +296,11 @@ int opal_set_power_shift_ratio(u32 handle, int token, u32 psr);
 int opal_sensor_group_clear(u32 group_hndl, int token);
 int opal_sensor_group_enable(u32 group_hndl, int token, bool enable);
 int opal_nx_coproc_init(uint32_t chip_id, uint32_t ct);
-
+int opal_secvar_get(const char *key, uint64_t key_len, u8 *data,
+		    uint64_t *data_size);
+int opal_secvar_get_next(const char *key, uint64_t *key_len, uint64_t key_size);
+int opal_secvar_enqueue_update(const char *key, uint64_t key_len, u8 *data,
+			       uint64_t data_size);
 s64 opal_signal_system_reset(s32 cpu);
 s64 opal_quiesce(u64 shutdown_type, s32 cpu);
 
@@ -387,6 +391,7 @@ void opal_wake_poller(void);
 void opal_powercap_init(void);
 void opal_psr_init(void);
 void opal_sensor_groups_init(void);
+void opal_secvar_init(void);
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/arch/powerpc/include/asm/powernv.h b/arch/powerpc/include/asm/powernv.h
index e1a858718716..cff980a85dd2 100644
--- a/arch/powerpc/include/asm/powernv.h
+++ b/arch/powerpc/include/asm/powernv.h
@@ -12,10 +12,12 @@ extern void powernv_set_nmmu_ptcr(unsigned long ptcr);
 void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val);
 
 void pnv_tm_init(void);
+
 #else
 static inline void powernv_set_nmmu_ptcr(unsigned long ptcr) { }
 
 static inline void pnv_tm_init(void) { }
+
 #endif
 
 #endif /* _ASM_POWERNV_H */
diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
new file mode 100644
index 000000000000..f27655cb5db8
--- /dev/null
+++ b/arch/powerpc/include/asm/secvar.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * PowerPC secure variable operations.
+ */
+#ifndef SECVAR_OPS_H
+#define SECVAR_OPS_H
+
+#include <linux/types.h>
+#include <linux/errno.h>
+
+extern const struct secvar_operations *secvar_ops;
+
+struct secvar_operations {
+	int (*get)(const char *key, uint64_t key_len, u8 *data,
+		   uint64_t *data_size);
+	int (*get_next)(const char *key, uint64_t *key_len,
+			uint64_t keysize);
+	int (*set)(const char *key, uint64_t key_len, u8 *data,
+		   uint64_t data_size);
+};
+
+#ifdef CONFIG_PPC_SECURE_BOOT
+
+extern void set_secvar_ops(const struct secvar_operations *ops);
+
+#else
+
+static inline void set_secvar_ops(const struct secvar_operations *ops) { }
+
+#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 000000000000..e067bc0c2336
--- /dev/null
+++ b/arch/powerpc/kernel/secvar-ops.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * This file initializes secvar operations for PowerPC Secureboot
+ */
+
+#include <stddef.h>
+#include <asm/secvar.h>
+
+const struct secvar_operations *secvar_ops;
+
+void set_secvar_ops(const struct secvar_operations *ops)
+{
+	if (!ops)
+		secvar_ops = NULL;
+	secvar_ops = ops;
+}
diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
index 850eee860cf2..65b060539b5c 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -47,3 +47,9 @@ config PPC_VAS
 	  VAS adapters are found in POWER9 based systems.
 
 	  If unsure, say N.
+
+config OPAL_SECVAR
+	bool "OPAL Secure Variables"
+	depends on PPC_POWERNV
+	help
+	  This enables the kernel to access OPAL secure variables.
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index da2e99efbd04..6651c742e530 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
 obj-$(CONFIG_PPC_MEMTRACE)	+= memtrace.o
 obj-$(CONFIG_PPC_VAS)	+= vas.o vas-window.o vas-debug.o
 obj-$(CONFIG_OCXL_BASE)	+= ocxl.o
+obj-$(CONFIG_OPAL_SECVAR) += opal-secvar.o
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 29ca523c1c79..93106e867924 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -287,3 +287,6 @@ OPAL_CALL(opal_pci_set_pbcq_tunnel_bar,		OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
 OPAL_CALL(opal_sensor_read_u64,			OPAL_SENSOR_READ_U64);
 OPAL_CALL(opal_sensor_group_enable,		OPAL_SENSOR_GROUP_ENABLE);
 OPAL_CALL(opal_nx_coproc_init,			OPAL_NX_COPROC_INIT);
+OPAL_CALL(opal_secvar_get,                     OPAL_SECVAR_GET);
+OPAL_CALL(opal_secvar_get_next,                 OPAL_SECVAR_GET_NEXT);
+OPAL_CALL(opal_secvar_enqueue_update,           OPAL_SECVAR_ENQUEUE_UPDATE);
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c b/arch/powerpc/platforms/powernv/opal-secvar.c
new file mode 100644
index 000000000000..6682013fb10b
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PowerNV code for secure variables
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Claudio Carvalho <cclaudio@linux.ibm.com>
+ *         Nayna Jain <nayna@linux.ibm.com>
+ *
+ * APIs to access secure variables managed by OPAL.
+ *
+ */
+
+#define pr_fmt(fmt) "secvar: "fmt
+
+#include <linux/types.h>
+#include <asm/opal.h>
+#include <asm/secvar.h>
+#include <asm/secboot.h>
+
+static int opal_status_to_err(int rc)
+{
+	int err;
+
+	switch (rc) {
+	case OPAL_SUCCESS:
+		err = 0;
+		break;
+	case OPAL_UNSUPPORTED:
+		err = -ENXIO;
+		break;
+	case OPAL_PARAMETER:
+		err = -EINVAL;
+		break;
+	case OPAL_RESOURCE:
+		err = -ENOSPC;
+		break;
+	case OPAL_HARDWARE:
+		err = -EIO;
+		break;
+	case OPAL_NO_MEM:
+		err = -ENOMEM;
+		break;
+	case OPAL_EMPTY:
+		err = -ENOENT;
+		break;
+	case OPAL_PARTIAL:
+		err = -EFBIG;
+		break;
+	default:
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static bool is_opal_secvar_supported(void)
+{
+	static bool opal_secvar_supported;
+	static bool initialized;
+
+	if (initialized)
+		return opal_secvar_supported;
+
+	if (!is_powerpc_secvar_supported()) {
+		opal_secvar_supported = false;
+		goto out;
+	}
+
+	if (!opal_check_token(OPAL_SECVAR_GET)
+			|| !opal_check_token(OPAL_SECVAR_GET_NEXT)
+			|| !opal_check_token(OPAL_SECVAR_ENQUEUE_UPDATE)) {
+		pr_err("OPAL doesn't support secure variables\n");
+		opal_secvar_supported = false;
+	} else {
+		opal_secvar_supported = true;
+	}
+
+out:
+	initialized = true;
+
+	return opal_secvar_supported;
+}
+
+static int opal_get_variable(const char *key, uint64_t ksize,
+			     u8 *data, uint64_t *dsize)
+{
+	int rc;
+
+	if (dsize)
+		*dsize = cpu_to_be64(*dsize);
+
+	rc = opal_secvar_get(key, ksize, data, dsize);
+
+	if (dsize)
+		*dsize = be64_to_cpu(*dsize);
+
+	return opal_status_to_err(rc);
+}
+
+static int opal_get_next_variable(const char *key, uint64_t *keylen,
+				  uint64_t keysize)
+{
+	int rc;
+
+	if (keylen)
+		*keylen = cpu_to_be64(*keylen);
+
+	rc = opal_secvar_get_next(key, keylen, keysize);
+
+	if (keylen)
+		*keylen = be64_to_cpu(*keylen);
+
+	return opal_status_to_err(rc);
+}
+
+static int opal_set_variable(const char *key, uint64_t ksize, u8 *data,
+			     uint64_t dsize)
+{
+	int rc;
+
+	rc = opal_secvar_enqueue_update(key, ksize, data, dsize);
+
+	return opal_status_to_err(rc);
+}
+
+static const struct secvar_operations opal_secvar_ops = {
+	.get = opal_get_variable,
+	.get_next = opal_get_next_variable,
+	.set = opal_set_variable,
+};
+
+void opal_secvar_init(void)
+{
+	if (!is_opal_secvar_supported())
+		set_secvar_ops(NULL);
+
+	set_secvar_ops(&opal_secvar_ops);
+}
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index aba443be7daa..3226961d451a 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -32,6 +32,8 @@
 #include <asm/mce.h>
 #include <asm/imc-pmu.h>
 #include <asm/bug.h>
+#include <asm/secvar.h>
+#include <asm/secboot.h>
 
 #include "powernv.h"
 
@@ -988,6 +990,9 @@ static int __init opal_init(void)
 	/* Initialise OPAL Power control interface */
 	opal_power_control_init();
 
+	/* Initialize OPAL secure variables */
+	opal_secvar_init();
+
 	return 0;
 }
 machine_subsys_initcall(powernv, opal_init);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 1/4] powerpc/powernv: Add OPAL API interface to access secure variable
@ 2019-08-26 13:23   ` Nayna Jain
  0 siblings, 0 replies; 37+ messages in thread
From: Nayna Jain @ 2019-08-26 13:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

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 <cclaudio@linux.ibm.com>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 arch/powerpc/include/asm/opal-api.h          |   5 +-
 arch/powerpc/include/asm/opal.h              |   7 +-
 arch/powerpc/include/asm/powernv.h           |   2 +
 arch/powerpc/include/asm/secvar.h            |  35 +++++
 arch/powerpc/kernel/Makefile                 |   2 +-
 arch/powerpc/kernel/secvar-ops.c             |  19 +++
 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 | 138 +++++++++++++++++++
 arch/powerpc/platforms/powernv/opal.c        |   5 +
 11 files changed, 220 insertions(+), 3 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_LAST				170
+#define OPAL_SECVAR_GET                         173
+#define OPAL_SECVAR_GET_NEXT                    174
+#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..0606b1d22db4 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -296,7 +296,11 @@ int opal_set_power_shift_ratio(u32 handle, int token, u32 psr);
 int opal_sensor_group_clear(u32 group_hndl, int token);
 int opal_sensor_group_enable(u32 group_hndl, int token, bool enable);
 int opal_nx_coproc_init(uint32_t chip_id, uint32_t ct);
-
+int opal_secvar_get(const char *key, uint64_t key_len, u8 *data,
+		    uint64_t *data_size);
+int opal_secvar_get_next(const char *key, uint64_t *key_len, uint64_t key_size);
+int opal_secvar_enqueue_update(const char *key, uint64_t key_len, u8 *data,
+			       uint64_t data_size);
 s64 opal_signal_system_reset(s32 cpu);
 s64 opal_quiesce(u64 shutdown_type, s32 cpu);
 
@@ -387,6 +391,7 @@ void opal_wake_poller(void);
 void opal_powercap_init(void);
 void opal_psr_init(void);
 void opal_sensor_groups_init(void);
+void opal_secvar_init(void);
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/arch/powerpc/include/asm/powernv.h b/arch/powerpc/include/asm/powernv.h
index e1a858718716..cff980a85dd2 100644
--- a/arch/powerpc/include/asm/powernv.h
+++ b/arch/powerpc/include/asm/powernv.h
@@ -12,10 +12,12 @@ extern void powernv_set_nmmu_ptcr(unsigned long ptcr);
 void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val);
 
 void pnv_tm_init(void);
+
 #else
 static inline void powernv_set_nmmu_ptcr(unsigned long ptcr) { }
 
 static inline void pnv_tm_init(void) { }
+
 #endif
 
 #endif /* _ASM_POWERNV_H */
diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
new file mode 100644
index 000000000000..f27655cb5db8
--- /dev/null
+++ b/arch/powerpc/include/asm/secvar.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * PowerPC secure variable operations.
+ */
+#ifndef SECVAR_OPS_H
+#define SECVAR_OPS_H
+
+#include <linux/types.h>
+#include <linux/errno.h>
+
+extern const struct secvar_operations *secvar_ops;
+
+struct secvar_operations {
+	int (*get)(const char *key, uint64_t key_len, u8 *data,
+		   uint64_t *data_size);
+	int (*get_next)(const char *key, uint64_t *key_len,
+			uint64_t keysize);
+	int (*set)(const char *key, uint64_t key_len, u8 *data,
+		   uint64_t data_size);
+};
+
+#ifdef CONFIG_PPC_SECURE_BOOT
+
+extern void set_secvar_ops(const struct secvar_operations *ops);
+
+#else
+
+static inline void set_secvar_ops(const struct secvar_operations *ops) { }
+
+#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 000000000000..e067bc0c2336
--- /dev/null
+++ b/arch/powerpc/kernel/secvar-ops.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * This file initializes secvar operations for PowerPC Secureboot
+ */
+
+#include <stddef.h>
+#include <asm/secvar.h>
+
+const struct secvar_operations *secvar_ops;
+
+void set_secvar_ops(const struct secvar_operations *ops)
+{
+	if (!ops)
+		secvar_ops = NULL;
+	secvar_ops = ops;
+}
diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
index 850eee860cf2..65b060539b5c 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -47,3 +47,9 @@ config PPC_VAS
 	  VAS adapters are found in POWER9 based systems.
 
 	  If unsure, say N.
+
+config OPAL_SECVAR
+	bool "OPAL Secure Variables"
+	depends on PPC_POWERNV
+	help
+	  This enables the kernel to access OPAL secure variables.
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index da2e99efbd04..6651c742e530 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
 obj-$(CONFIG_PPC_MEMTRACE)	+= memtrace.o
 obj-$(CONFIG_PPC_VAS)	+= vas.o vas-window.o vas-debug.o
 obj-$(CONFIG_OCXL_BASE)	+= ocxl.o
+obj-$(CONFIG_OPAL_SECVAR) += opal-secvar.o
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 29ca523c1c79..93106e867924 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -287,3 +287,6 @@ OPAL_CALL(opal_pci_set_pbcq_tunnel_bar,		OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
 OPAL_CALL(opal_sensor_read_u64,			OPAL_SENSOR_READ_U64);
 OPAL_CALL(opal_sensor_group_enable,		OPAL_SENSOR_GROUP_ENABLE);
 OPAL_CALL(opal_nx_coproc_init,			OPAL_NX_COPROC_INIT);
+OPAL_CALL(opal_secvar_get,                     OPAL_SECVAR_GET);
+OPAL_CALL(opal_secvar_get_next,                 OPAL_SECVAR_GET_NEXT);
+OPAL_CALL(opal_secvar_enqueue_update,           OPAL_SECVAR_ENQUEUE_UPDATE);
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c b/arch/powerpc/platforms/powernv/opal-secvar.c
new file mode 100644
index 000000000000..6682013fb10b
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PowerNV code for secure variables
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Claudio Carvalho <cclaudio@linux.ibm.com>
+ *         Nayna Jain <nayna@linux.ibm.com>
+ *
+ * APIs to access secure variables managed by OPAL.
+ *
+ */
+
+#define pr_fmt(fmt) "secvar: "fmt
+
+#include <linux/types.h>
+#include <asm/opal.h>
+#include <asm/secvar.h>
+#include <asm/secboot.h>
+
+static int opal_status_to_err(int rc)
+{
+	int err;
+
+	switch (rc) {
+	case OPAL_SUCCESS:
+		err = 0;
+		break;
+	case OPAL_UNSUPPORTED:
+		err = -ENXIO;
+		break;
+	case OPAL_PARAMETER:
+		err = -EINVAL;
+		break;
+	case OPAL_RESOURCE:
+		err = -ENOSPC;
+		break;
+	case OPAL_HARDWARE:
+		err = -EIO;
+		break;
+	case OPAL_NO_MEM:
+		err = -ENOMEM;
+		break;
+	case OPAL_EMPTY:
+		err = -ENOENT;
+		break;
+	case OPAL_PARTIAL:
+		err = -EFBIG;
+		break;
+	default:
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static bool is_opal_secvar_supported(void)
+{
+	static bool opal_secvar_supported;
+	static bool initialized;
+
+	if (initialized)
+		return opal_secvar_supported;
+
+	if (!is_powerpc_secvar_supported()) {
+		opal_secvar_supported = false;
+		goto out;
+	}
+
+	if (!opal_check_token(OPAL_SECVAR_GET)
+			|| !opal_check_token(OPAL_SECVAR_GET_NEXT)
+			|| !opal_check_token(OPAL_SECVAR_ENQUEUE_UPDATE)) {
+		pr_err("OPAL doesn't support secure variables\n");
+		opal_secvar_supported = false;
+	} else {
+		opal_secvar_supported = true;
+	}
+
+out:
+	initialized = true;
+
+	return opal_secvar_supported;
+}
+
+static int opal_get_variable(const char *key, uint64_t ksize,
+			     u8 *data, uint64_t *dsize)
+{
+	int rc;
+
+	if (dsize)
+		*dsize = cpu_to_be64(*dsize);
+
+	rc = opal_secvar_get(key, ksize, data, dsize);
+
+	if (dsize)
+		*dsize = be64_to_cpu(*dsize);
+
+	return opal_status_to_err(rc);
+}
+
+static int opal_get_next_variable(const char *key, uint64_t *keylen,
+				  uint64_t keysize)
+{
+	int rc;
+
+	if (keylen)
+		*keylen = cpu_to_be64(*keylen);
+
+	rc = opal_secvar_get_next(key, keylen, keysize);
+
+	if (keylen)
+		*keylen = be64_to_cpu(*keylen);
+
+	return opal_status_to_err(rc);
+}
+
+static int opal_set_variable(const char *key, uint64_t ksize, u8 *data,
+			     uint64_t dsize)
+{
+	int rc;
+
+	rc = opal_secvar_enqueue_update(key, ksize, data, dsize);
+
+	return opal_status_to_err(rc);
+}
+
+static const struct secvar_operations opal_secvar_ops = {
+	.get = opal_get_variable,
+	.get_next = opal_get_next_variable,
+	.set = opal_set_variable,
+};
+
+void opal_secvar_init(void)
+{
+	if (!is_opal_secvar_supported())
+		set_secvar_ops(NULL);
+
+	set_secvar_ops(&opal_secvar_ops);
+}
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index aba443be7daa..3226961d451a 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -32,6 +32,8 @@
 #include <asm/mce.h>
 #include <asm/imc-pmu.h>
 #include <asm/bug.h>
+#include <asm/secvar.h>
+#include <asm/secboot.h>
 
 #include "powernv.h"
 
@@ -988,6 +990,9 @@ static int __init opal_init(void)
 	/* Initialise OPAL Power control interface */
 	opal_power_control_init();
 
+	/* Initialize OPAL secure variables */
+	opal_secvar_init();
+
 	return 0;
 }
 machine_subsys_initcall(powernv, opal_init);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
  2019-08-26 13:23 ` Nayna Jain
@ 2019-08-26 13:23   ` Nayna Jain
  -1 siblings, 0 replies; 37+ messages in thread
From: Nayna Jain @ 2019-08-26 13:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: linux-kernel, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Ard Biesheuvel, Jeremy Kerr, Matthew Garret,
	Mimi Zohar, Greg Kroah-Hartman, Claudio Carvalho, George Wilson,
	Elaine Palmer, Eric Ricther, Oliver O'Halloran, Nayna Jain

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 <nayna@linux.ibm.com>
---
 Documentation/ABI/testing/sysfs-secvar |  37 +++++
 arch/powerpc/Kconfig                   |  10 ++
 arch/powerpc/kernel/Makefile           |   1 +
 arch/powerpc/kernel/secvar-sysfs.c     | 200 +++++++++++++++++++++++++
 4 files changed, 248 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 000000000000..815bd8ec4d5e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-secvar
@@ -0,0 +1,37 @@
+What:		/sys/firmware/secvar
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	This directory is created if the POWER firmware supports OS
+		secureboot, thereby secure variables. It exposes interface
+		for reading/writing the secure variables
+
+What:		/sys/firmware/secvar/vars
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	This directory lists all the secure variables that are supported
+		by the firmware.
+
+What:		/sys/firmware/secvar/vars/<variable name>
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	Each secure variable is represented as a directory named as
+		<variable_name>. The variable name is unique and is in ASCII
+		representation. The data and size can be determined by reading
+		their respective attribute files.
+
+What:		/sys/firmware/secvar/vars/<variable_name>/size
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	An integer representation of the size of the content of the
+		variable. In other words, it represents the size of the data.
+
+What:		/sys/firmware/secvar/vars/<variable_name>/data
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	A read-only file containing the value of the variable
+
+What:		/sys/firmware/secvar/vars/<variable_name>/update
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	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..11f553e68e1f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -925,6 +925,16 @@ 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
+	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.
+
 endmenu
 
 config ISA_DMA_API
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 9041563f1c74..cbdac2e6b6f8 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 000000000000..020529a5f6fb
--- /dev/null
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 IBM Corporation <nayna@linux.ibm.com>
+ *
+ * This code exposes secure variables to user via sysfs
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/compat.h>
+#include <linux/string.h>
+#include <asm/secvar.h>
+
+/* Approximating it for now. It will be read from device tree */
+#define VARIABLE_MAX_SIZE  32000
+/* Approximate value */
+#define NAME_MAX_SIZE	   1024
+
+static struct kobject *secvar_kobj;
+static struct kset *secvar_kset;
+
+static ssize_t size_show(struct kobject *kobj, struct kobj_attribute *attr,
+			 char *buf)
+{
+	uint64_t dsize;
+	int rc;
+
+	rc = secvar_ops->get(kobj->name, strlen(kobj->name) + 1, NULL, &dsize);
+	if (rc) {
+		pr_err("Error retrieving variable size %d\n", rc);
+		return rc;
+	}
+
+	rc = sprintf(buf, "%llu\n", 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)
+{
+	uint64_t dsize;
+	int rc;
+	char *data;
+
+	rc = secvar_ops->get(kobj->name, strlen(kobj->name) + 1, NULL, &dsize);
+	if (rc) {
+		pr_err("Error getting variable size %d\n", rc);
+		return rc;
+	}
+	pr_debug("dsize is %llu\n", dsize);
+
+	data = kzalloc(dsize, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	rc = secvar_ops->get(kobj->name, strlen(kobj->name)+1, data, &dsize);
+	if (rc) {
+		pr_err("Error getting variable %d\n", rc);
+		goto data_fail;
+	}
+
+	rc = memory_read_from_buffer(buf, count, &off, data, dsize);
+
+data_fail:
+	kfree(data);
+	return rc;
+}
+
+static ssize_t update_write(struct file *filep, struct kobject *kobj,
+			    struct bin_attribute *attr, char *buf, loff_t off,
+			    size_t count)
+{
+	int rc;
+
+	pr_debug("count is %ld\n", count);
+	rc = secvar_ops->set(kobj->name, strlen(kobj->name)+1, buf, count);
+	if (rc) {
+		pr_err("Error setting the variable %s\n", kobj->name);
+		return rc;
+	}
+
+	return count;
+}
+
+static struct kobj_attribute size_attr = __ATTR_RO(size);
+
+static struct bin_attribute data_attr = __BIN_ATTR_RO(data, VARIABLE_MAX_SIZE);
+
+static struct bin_attribute update_attr = {
+	.attr = {.name = "update", .mode = 0200},
+	.size = VARIABLE_MAX_SIZE,
+	.write = update_write,
+};
+
+static struct bin_attribute  *secvar_bin_attrs[] = {
+	&data_attr,
+	&update_attr,
+	NULL,
+};
+
+static struct attribute *secvar_attrs[] = {
+	&size_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group secvar_attr_group = {
+	.attrs = secvar_attrs,
+	.bin_attrs = secvar_bin_attrs,
+};
+__ATTRIBUTE_GROUPS(secvar_attr);
+
+static struct kobj_type secvar_ktype = {
+	.sysfs_ops	= &kobj_sysfs_ops,
+	.default_groups = secvar_attr_groups,
+};
+
+static int secvar_sysfs_load(void)
+{
+	char *name;
+	uint64_t namesize = 0;
+	struct kobject *kobj;
+	int rc;
+
+	name = kzalloc(NAME_MAX_SIZE, GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+
+	do {
+		rc = secvar_ops->get_next(name, &namesize, NAME_MAX_SIZE);
+		if (rc) {
+			if (rc != -ENOENT)
+				pr_err("error getting secvar from firmware %d\n",
+					rc);
+			break;
+		}
+
+		kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
+		if (!kobj)
+			return -ENOMEM;
+
+		kobject_init(kobj, &secvar_ktype);
+
+		rc = kobject_add(kobj, &secvar_kset->kobj, "%s", name);
+		if (rc) {
+			pr_warn("kobject_add error %d for attribute: %s\n", rc,
+				name);
+			kobject_put(kobj);
+			kobj = NULL;
+		}
+
+		if (kobj)
+			kobject_uevent(kobj, KOBJ_ADD);
+
+	} while (!rc);
+
+	kfree(name);
+	return rc;
+}
+
+static int secvar_sysfs_init(void)
+{
+	if (!secvar_ops) {
+		pr_warn("secvar: failed to retrieve secvar operations.\n");
+		return -ENODEV;
+	}
+
+	secvar_kobj = kobject_create_and_add("secvar", firmware_kobj);
+	if (!secvar_kobj) {
+		pr_err("secvar: Failed to create firmware kobj\n");
+		return -ENOMEM;
+	}
+
+	secvar_kset = kset_create_and_add("vars", NULL, secvar_kobj);
+	if (!secvar_kset) {
+		pr_err("secvar: sysfs kobject registration failed.\n");
+		kobject_put(secvar_kobj);
+		return -ENOMEM;
+	}
+
+	secvar_sysfs_load();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(secvar_sysfs_init);
+
+static void secvar_sysfs_exit(void)
+{
+	kset_unregister(secvar_kset);
+	kobject_put(secvar_kobj);
+}
+EXPORT_SYMBOL_GPL(secvar_sysfs_exit);
+
+module_init(secvar_sysfs_init);
+module_exit(secvar_sysfs_exit);
+
+MODULE_AUTHOR("Nayna Jain <nayna@linux.ibm.com>");
+MODULE_DESCRIPTION("sysfs interface to POWER secure variables");
+MODULE_LICENSE("GPL");
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
@ 2019-08-26 13:23   ` Nayna Jain
  0 siblings, 0 replies; 37+ messages in thread
From: Nayna Jain @ 2019-08-26 13:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

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 <nayna@linux.ibm.com>
---
 Documentation/ABI/testing/sysfs-secvar |  37 +++++
 arch/powerpc/Kconfig                   |  10 ++
 arch/powerpc/kernel/Makefile           |   1 +
 arch/powerpc/kernel/secvar-sysfs.c     | 200 +++++++++++++++++++++++++
 4 files changed, 248 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 000000000000..815bd8ec4d5e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-secvar
@@ -0,0 +1,37 @@
+What:		/sys/firmware/secvar
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	This directory is created if the POWER firmware supports OS
+		secureboot, thereby secure variables. It exposes interface
+		for reading/writing the secure variables
+
+What:		/sys/firmware/secvar/vars
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	This directory lists all the secure variables that are supported
+		by the firmware.
+
+What:		/sys/firmware/secvar/vars/<variable name>
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	Each secure variable is represented as a directory named as
+		<variable_name>. The variable name is unique and is in ASCII
+		representation. The data and size can be determined by reading
+		their respective attribute files.
+
+What:		/sys/firmware/secvar/vars/<variable_name>/size
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	An integer representation of the size of the content of the
+		variable. In other words, it represents the size of the data.
+
+What:		/sys/firmware/secvar/vars/<variable_name>/data
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	A read-only file containing the value of the variable
+
+What:		/sys/firmware/secvar/vars/<variable_name>/update
+Date:		August 2019
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	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..11f553e68e1f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -925,6 +925,16 @@ 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
+	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.
+
 endmenu
 
 config ISA_DMA_API
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 9041563f1c74..cbdac2e6b6f8 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 000000000000..020529a5f6fb
--- /dev/null
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 IBM Corporation <nayna@linux.ibm.com>
+ *
+ * This code exposes secure variables to user via sysfs
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/compat.h>
+#include <linux/string.h>
+#include <asm/secvar.h>
+
+/* Approximating it for now. It will be read from device tree */
+#define VARIABLE_MAX_SIZE  32000
+/* Approximate value */
+#define NAME_MAX_SIZE	   1024
+
+static struct kobject *secvar_kobj;
+static struct kset *secvar_kset;
+
+static ssize_t size_show(struct kobject *kobj, struct kobj_attribute *attr,
+			 char *buf)
+{
+	uint64_t dsize;
+	int rc;
+
+	rc = secvar_ops->get(kobj->name, strlen(kobj->name) + 1, NULL, &dsize);
+	if (rc) {
+		pr_err("Error retrieving variable size %d\n", rc);
+		return rc;
+	}
+
+	rc = sprintf(buf, "%llu\n", 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)
+{
+	uint64_t dsize;
+	int rc;
+	char *data;
+
+	rc = secvar_ops->get(kobj->name, strlen(kobj->name) + 1, NULL, &dsize);
+	if (rc) {
+		pr_err("Error getting variable size %d\n", rc);
+		return rc;
+	}
+	pr_debug("dsize is %llu\n", dsize);
+
+	data = kzalloc(dsize, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	rc = secvar_ops->get(kobj->name, strlen(kobj->name)+1, data, &dsize);
+	if (rc) {
+		pr_err("Error getting variable %d\n", rc);
+		goto data_fail;
+	}
+
+	rc = memory_read_from_buffer(buf, count, &off, data, dsize);
+
+data_fail:
+	kfree(data);
+	return rc;
+}
+
+static ssize_t update_write(struct file *filep, struct kobject *kobj,
+			    struct bin_attribute *attr, char *buf, loff_t off,
+			    size_t count)
+{
+	int rc;
+
+	pr_debug("count is %ld\n", count);
+	rc = secvar_ops->set(kobj->name, strlen(kobj->name)+1, buf, count);
+	if (rc) {
+		pr_err("Error setting the variable %s\n", kobj->name);
+		return rc;
+	}
+
+	return count;
+}
+
+static struct kobj_attribute size_attr = __ATTR_RO(size);
+
+static struct bin_attribute data_attr = __BIN_ATTR_RO(data, VARIABLE_MAX_SIZE);
+
+static struct bin_attribute update_attr = {
+	.attr = {.name = "update", .mode = 0200},
+	.size = VARIABLE_MAX_SIZE,
+	.write = update_write,
+};
+
+static struct bin_attribute  *secvar_bin_attrs[] = {
+	&data_attr,
+	&update_attr,
+	NULL,
+};
+
+static struct attribute *secvar_attrs[] = {
+	&size_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group secvar_attr_group = {
+	.attrs = secvar_attrs,
+	.bin_attrs = secvar_bin_attrs,
+};
+__ATTRIBUTE_GROUPS(secvar_attr);
+
+static struct kobj_type secvar_ktype = {
+	.sysfs_ops	= &kobj_sysfs_ops,
+	.default_groups = secvar_attr_groups,
+};
+
+static int secvar_sysfs_load(void)
+{
+	char *name;
+	uint64_t namesize = 0;
+	struct kobject *kobj;
+	int rc;
+
+	name = kzalloc(NAME_MAX_SIZE, GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+
+	do {
+		rc = secvar_ops->get_next(name, &namesize, NAME_MAX_SIZE);
+		if (rc) {
+			if (rc != -ENOENT)
+				pr_err("error getting secvar from firmware %d\n",
+					rc);
+			break;
+		}
+
+		kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
+		if (!kobj)
+			return -ENOMEM;
+
+		kobject_init(kobj, &secvar_ktype);
+
+		rc = kobject_add(kobj, &secvar_kset->kobj, "%s", name);
+		if (rc) {
+			pr_warn("kobject_add error %d for attribute: %s\n", rc,
+				name);
+			kobject_put(kobj);
+			kobj = NULL;
+		}
+
+		if (kobj)
+			kobject_uevent(kobj, KOBJ_ADD);
+
+	} while (!rc);
+
+	kfree(name);
+	return rc;
+}
+
+static int secvar_sysfs_init(void)
+{
+	if (!secvar_ops) {
+		pr_warn("secvar: failed to retrieve secvar operations.\n");
+		return -ENODEV;
+	}
+
+	secvar_kobj = kobject_create_and_add("secvar", firmware_kobj);
+	if (!secvar_kobj) {
+		pr_err("secvar: Failed to create firmware kobj\n");
+		return -ENOMEM;
+	}
+
+	secvar_kset = kset_create_and_add("vars", NULL, secvar_kobj);
+	if (!secvar_kset) {
+		pr_err("secvar: sysfs kobject registration failed.\n");
+		kobject_put(secvar_kobj);
+		return -ENOMEM;
+	}
+
+	secvar_sysfs_load();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(secvar_sysfs_init);
+
+static void secvar_sysfs_exit(void)
+{
+	kset_unregister(secvar_kset);
+	kobject_put(secvar_kobj);
+}
+EXPORT_SYMBOL_GPL(secvar_sysfs_exit);
+
+module_init(secvar_sysfs_init);
+module_exit(secvar_sysfs_exit);
+
+MODULE_AUTHOR("Nayna Jain <nayna@linux.ibm.com>");
+MODULE_DESCRIPTION("sysfs interface to POWER secure variables");
+MODULE_LICENSE("GPL");
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 3/4] x86/efi: move common keyring handler functions to new file
  2019-08-26 13:23 ` Nayna Jain
@ 2019-08-26 13:23   ` Nayna Jain
  -1 siblings, 0 replies; 37+ messages in thread
From: Nayna Jain @ 2019-08-26 13:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: linux-kernel, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Ard Biesheuvel, Jeremy Kerr, Matthew Garret,
	Mimi Zohar, Greg Kroah-Hartman, Claudio Carvalho, George Wilson,
	Elaine Palmer, Eric Ricther, Oliver O'Halloran, Nayna Jain

The handlers to add the keys to the .platform keyring and blacklisted
hashes to the .blacklist keyring is common for both the uefi and powerpc
mechanisms of loading the keys/hashes from the firmware.

This patch moves the common code from load_uefi.c to keyring_handler.c

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 security/integrity/Makefile                   |  3 +-
 .../platform_certs/keyring_handler.c          | 80 +++++++++++++++++++
 .../platform_certs/keyring_handler.h          | 32 ++++++++
 security/integrity/platform_certs/load_uefi.c | 67 +---------------
 4 files changed, 115 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 000000000000..c5ba695c10e3
--- /dev/null
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <linux/efi.h>
+#include <linux/slab.h>
+#include <keys/asymmetric-type.h>
+#include <keys/system_keyring.h>
+#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 000000000000..2462bfa08fe3
--- /dev/null
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef PLATFORM_CERTS_INTERNAL_H
+#define PLATFORM_CERTS_INTERNAL_H
+
+#include <linux/efi.h>
+
+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 get_handler_for_dbx(const efi_guid_t *sig_type);
+
+#endif
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index 81b19c52832b..4369204a19cd 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -9,6 +9,7 @@
 #include <keys/asymmetric-type.h>
 #include <keys/system_keyring.h>
 #include "../integrity.h"
+#include "keyring_handler.h"
 
 static efi_guid_t efi_cert_x509_guid __initdata = EFI_CERT_X509_GUID;
 static efi_guid_t efi_cert_x509_sha256_guid __initdata =
@@ -67,72 +68,6 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 	return db;
 }
 
-/*
- * 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.
- */
-static __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.
- */
-static __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;
-}
-
 /*
  * 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
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 3/4] x86/efi: move common keyring handler functions to new file
@ 2019-08-26 13:23   ` Nayna Jain
  0 siblings, 0 replies; 37+ messages in thread
From: Nayna Jain @ 2019-08-26 13:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

The handlers to add the keys to the .platform keyring and blacklisted
hashes to the .blacklist keyring is common for both the uefi and powerpc
mechanisms of loading the keys/hashes from the firmware.

This patch moves the common code from load_uefi.c to keyring_handler.c

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 security/integrity/Makefile                   |  3 +-
 .../platform_certs/keyring_handler.c          | 80 +++++++++++++++++++
 .../platform_certs/keyring_handler.h          | 32 ++++++++
 security/integrity/platform_certs/load_uefi.c | 67 +---------------
 4 files changed, 115 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 000000000000..c5ba695c10e3
--- /dev/null
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <linux/efi.h>
+#include <linux/slab.h>
+#include <keys/asymmetric-type.h>
+#include <keys/system_keyring.h>
+#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 000000000000..2462bfa08fe3
--- /dev/null
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef PLATFORM_CERTS_INTERNAL_H
+#define PLATFORM_CERTS_INTERNAL_H
+
+#include <linux/efi.h>
+
+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 get_handler_for_dbx(const efi_guid_t *sig_type);
+
+#endif
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index 81b19c52832b..4369204a19cd 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -9,6 +9,7 @@
 #include <keys/asymmetric-type.h>
 #include <keys/system_keyring.h>
 #include "../integrity.h"
+#include "keyring_handler.h"
 
 static efi_guid_t efi_cert_x509_guid __initdata = EFI_CERT_X509_GUID;
 static efi_guid_t efi_cert_x509_sha256_guid __initdata =
@@ -67,72 +68,6 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 	return db;
 }
 
-/*
- * 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.
- */
-static __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.
- */
-static __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;
-}
-
 /*
  * 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
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 4/4] powerpc: load firmware trusted keys/hashes into kernel keyring
  2019-08-26 13:23 ` Nayna Jain
@ 2019-08-26 13:23   ` Nayna Jain
  -1 siblings, 0 replies; 37+ messages in thread
From: Nayna Jain @ 2019-08-26 13:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: linux-kernel, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Ard Biesheuvel, Jeremy Kerr, Matthew Garret,
	Mimi Zohar, Greg Kroah-Hartman, Claudio Carvalho, George Wilson,
	Elaine Palmer, Eric Ricther, Oliver O'Halloran, Nayna Jain

The keys used to verify the Host OS kernel are managed by firmware as
secure variables. This patch loads the verification keys into the .platform
keyring and revocation hashes 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 <nayna@linux.ibm.com>
---
 security/integrity/Kconfig                    |  8 ++
 security/integrity/Makefile                   |  3 +
 .../integrity/platform_certs/load_powerpc.c   | 88 +++++++++++++++++++
 3 files changed, 99 insertions(+)
 create mode 100644 security/integrity/platform_certs/load_powerpc.c

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 0bae6adb63a9..26abee23e4e3 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -72,6 +72,14 @@ config LOAD_IPL_KEYS
        depends on S390
        def_bool y
 
+config LOAD_PPC_KEYS
+	bool "Enable loading of platform and blacklisted keys for POWER"
+	depends on INTEGRITY_PLATFORM_KEYRING
+	depends on PPC_SECURE_BOOT
+	help
+	  Enable loading of keys to the .platform keyring and blacklisted
+	  hashes 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 000000000000..359d5063d4da
--- /dev/null
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ *      - loads keys and hashes stored and controlled by the firmware.
+ */
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <asm/secboot.h>
+#include <asm/secvar.h>
+#include "keyring_handler.h"
+
+/*
+ * Get a certificate list blob from the named secure variable.
+ */
+static __init void *get_cert_list(u8 *key, unsigned long keylen, uint64_t *size)
+{
+	int rc;
+	void *db;
+
+	rc = secvar_ops->get(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 = secvar_ops->get(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 keys databases into the platform trusted
+ * keyring and the blacklisted X.509 cert SHA256 hashes into the blacklist
+ * keyring.
+ */
+static int __init load_powerpc_certs(void)
+{
+	void *db = NULL, *dbx = NULL;
+	uint64_t dbsize = 0, dbxsize = 0;
+	int rc = 0;
+
+	if (!secvar_ops)
+		return -ENODEV;
+
+	/* 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, &dbsize);
+	if (!db) {
+		pr_err("Couldn't get db list from firmware\n");
+	} else {
+		rc = parse_efi_signature_list("powerpc: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,  &dbxsize);
+	if (!dbx) {
+		pr_info("Couldn't get dbx list from firmware\n");
+	} else {
+		rc = parse_efi_signature_list("powerpc: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


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 4/4] powerpc: load firmware trusted keys/hashes into kernel keyring
@ 2019-08-26 13:23   ` Nayna Jain
  0 siblings, 0 replies; 37+ messages in thread
From: Nayna Jain @ 2019-08-26 13:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, George Wilson

The keys used to verify the Host OS kernel are managed by firmware as
secure variables. This patch loads the verification keys into the .platform
keyring and revocation hashes 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 <nayna@linux.ibm.com>
---
 security/integrity/Kconfig                    |  8 ++
 security/integrity/Makefile                   |  3 +
 .../integrity/platform_certs/load_powerpc.c   | 88 +++++++++++++++++++
 3 files changed, 99 insertions(+)
 create mode 100644 security/integrity/platform_certs/load_powerpc.c

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 0bae6adb63a9..26abee23e4e3 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -72,6 +72,14 @@ config LOAD_IPL_KEYS
        depends on S390
        def_bool y
 
+config LOAD_PPC_KEYS
+	bool "Enable loading of platform and blacklisted keys for POWER"
+	depends on INTEGRITY_PLATFORM_KEYRING
+	depends on PPC_SECURE_BOOT
+	help
+	  Enable loading of keys to the .platform keyring and blacklisted
+	  hashes 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 000000000000..359d5063d4da
--- /dev/null
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ *      - loads keys and hashes stored and controlled by the firmware.
+ */
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <asm/secboot.h>
+#include <asm/secvar.h>
+#include "keyring_handler.h"
+
+/*
+ * Get a certificate list blob from the named secure variable.
+ */
+static __init void *get_cert_list(u8 *key, unsigned long keylen, uint64_t *size)
+{
+	int rc;
+	void *db;
+
+	rc = secvar_ops->get(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 = secvar_ops->get(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 keys databases into the platform trusted
+ * keyring and the blacklisted X.509 cert SHA256 hashes into the blacklist
+ * keyring.
+ */
+static int __init load_powerpc_certs(void)
+{
+	void *db = NULL, *dbx = NULL;
+	uint64_t dbsize = 0, dbxsize = 0;
+	int rc = 0;
+
+	if (!secvar_ops)
+		return -ENODEV;
+
+	/* 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, &dbsize);
+	if (!db) {
+		pr_err("Couldn't get db list from firmware\n");
+	} else {
+		rc = parse_efi_signature_list("powerpc: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,  &dbxsize);
+	if (!dbx) {
+		pr_info("Couldn't get dbx list from firmware\n");
+	} else {
+		rc = parse_efi_signature_list("powerpc: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


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
  2019-08-26 13:23   ` Nayna Jain
@ 2019-08-26 14:01     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-26 14:01 UTC (permalink / raw)
  To: Nayna Jain
  Cc: linuxppc-dev, linux-efi, linux-integrity, linux-kernel,
	Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Ard Biesheuvel, Jeremy Kerr, Matthew Garret, Mimi Zohar,
	Claudio Carvalho, George Wilson, Elaine Palmer, Eric Ricther,
	Oliver O'Halloran

On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
> +static struct bin_attribute update_attr = {
> +	.attr = {.name = "update", .mode = 0200},
> +	.size = VARIABLE_MAX_SIZE,
> +	.write = update_write,
> +};

Ah, do we need a __BIN_ATTR_WO() macro for you?  That would make this
more obvious, right?

Other than that minor thing (not a complaint at all) looks much better
to me, nice work:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
@ 2019-08-26 14:01     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-26 14:01 UTC (permalink / raw)
  To: Nayna Jain
  Cc: linux-efi, Ard Biesheuvel, Eric Ricther, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, linuxppc-dev,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, linux-integrity, George Wilson

On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
> +static struct bin_attribute update_attr = {
> +	.attr = {.name = "update", .mode = 0200},
> +	.size = VARIABLE_MAX_SIZE,
> +	.write = update_write,
> +};

Ah, do we need a __BIN_ATTR_WO() macro for you?  That would make this
more obvious, right?

Other than that minor thing (not a complaint at all) looks much better
to me, nice work:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
  2019-08-26 14:01     ` Greg Kroah-Hartman
@ 2019-08-26 14:12       ` Nayna
  -1 siblings, 0 replies; 37+ messages in thread
From: Nayna @ 2019-08-26 14:12 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Nayna Jain
  Cc: linuxppc-dev, linux-efi, linux-integrity, linux-kernel,
	Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Ard Biesheuvel, Jeremy Kerr, Matthew Garret, Mimi Zohar,
	Claudio Carvalho, George Wilson, Elaine Palmer, Eric Ricther,
	Oliver O'Halloran



On 08/26/2019 10:01 AM, Greg Kroah-Hartman wrote:
> On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
>> +static struct bin_attribute update_attr = {
>> +	.attr = {.name = "update", .mode = 0200},
>> +	.size = VARIABLE_MAX_SIZE,
>> +	.write = update_write,
>> +};
> Ah, do we need a __BIN_ATTR_WO() macro for you?  That would make this
> more obvious, right?

Thanks Greg.  Yes, I agree it will be good to have that.

Thanks & Regards,
      - Nayna


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
@ 2019-08-26 14:12       ` Nayna
  0 siblings, 0 replies; 37+ messages in thread
From: Nayna @ 2019-08-26 14:12 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Nayna Jain
  Cc: linux-efi, Ard Biesheuvel, Eric Ricther, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, linuxppc-dev,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, linux-integrity, George Wilson



On 08/26/2019 10:01 AM, Greg Kroah-Hartman wrote:
> On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
>> +static struct bin_attribute update_attr = {
>> +	.attr = {.name = "update", .mode = 0200},
>> +	.size = VARIABLE_MAX_SIZE,
>> +	.write = update_write,
>> +};
> Ah, do we need a __BIN_ATTR_WO() macro for you?  That would make this
> more obvious, right?

Thanks Greg.  Yes, I agree it will be good to have that.

Thanks & Regards,
      - Nayna


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
  2019-08-26 13:23   ` Nayna Jain
@ 2019-08-26 14:56     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-26 14:56 UTC (permalink / raw)
  To: Nayna Jain
  Cc: linuxppc-dev, linux-efi, linux-integrity, linux-kernel,
	Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Ard Biesheuvel, Jeremy Kerr, Matthew Garret, Mimi Zohar,
	Claudio Carvalho, George Wilson, Elaine Palmer, Eric Ricther,
	Oliver O'Halloran

On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
> +static struct kobj_attribute size_attr = __ATTR_RO(size);

Wait, why not just normal ATTR_RO()?

> +static struct bin_attribute data_attr = __BIN_ATTR_RO(data, VARIABLE_MAX_SIZE);

And BIN_ATTR_RO() here?

Do those not work for you somehow?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
@ 2019-08-26 14:56     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-26 14:56 UTC (permalink / raw)
  To: Nayna Jain
  Cc: linux-efi, Ard Biesheuvel, Eric Ricther, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, linuxppc-dev,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, linux-integrity, George Wilson

On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
> +static struct kobj_attribute size_attr = __ATTR_RO(size);

Wait, why not just normal ATTR_RO()?

> +static struct bin_attribute data_attr = __BIN_ATTR_RO(data, VARIABLE_MAX_SIZE);

And BIN_ATTR_RO() here?

Do those not work for you somehow?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH] sysfs: add BIN_ATTR_WO() macro
  2019-08-26 14:12       ` Nayna
@ 2019-08-26 15:01         ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-26 15:01 UTC (permalink / raw)
  To: Nayna
  Cc: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity,
	linux-kernel, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Ard Biesheuvel, Jeremy Kerr, Matthew Garret,
	Mimi Zohar, Claudio Carvalho, George Wilson, Elaine Palmer,
	Eric Ricther, Oliver O'Halloran

This variant was missing from sysfs.h, I guess no one noticed it before.

Turns out the powerpc secure variable code can use it, so add it to the
tree for it, and potentially others to take advantage of, instead of
open-coding it.

Reported-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

I'll queue this up to my tree for 5.4-rc1, but if you want to take this
in your tree earlier, feel free to do so.

 include/linux/sysfs.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 965236795750..5420817ed317 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -196,6 +196,12 @@ struct bin_attribute {
 	.size	= _size,						\
 }
 
+#define __BIN_ATTR_WO(_name) {						\
+	.attr	= { .name = __stringify(_name), .mode = 0200 },		\
+	.store	= _name##_store,					\
+	.size	= _size,						\
+}
+
 #define __BIN_ATTR_RW(_name, _size)					\
 	__BIN_ATTR(_name, 0644, _name##_read, _name##_write, _size)
 
@@ -208,6 +214,9 @@ struct bin_attribute bin_attr_##_name = __BIN_ATTR(_name, _mode, _read,	\
 #define BIN_ATTR_RO(_name, _size)					\
 struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
 
+#define BIN_ATTR_WO(_name, _size)					\
+struct bin_attribute bin_attr_##_name = __BIN_ATTR_WO(_name, _size)
+
 #define BIN_ATTR_RW(_name, _size)					\
 struct bin_attribute bin_attr_##_name = __BIN_ATTR_RW(_name, _size)
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH] sysfs: add BIN_ATTR_WO() macro
@ 2019-08-26 15:01         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-26 15:01 UTC (permalink / raw)
  To: Nayna
  Cc: linux-efi, Ard Biesheuvel, Eric Ricther, Nayna Jain,
	linux-kernel, Mimi Zohar, Claudio Carvalho, Matthew Garret,
	linuxppc-dev, Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, linux-integrity, George Wilson

This variant was missing from sysfs.h, I guess no one noticed it before.

Turns out the powerpc secure variable code can use it, so add it to the
tree for it, and potentially others to take advantage of, instead of
open-coding it.

Reported-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

I'll queue this up to my tree for 5.4-rc1, but if you want to take this
in your tree earlier, feel free to do so.

 include/linux/sysfs.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 965236795750..5420817ed317 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -196,6 +196,12 @@ struct bin_attribute {
 	.size	= _size,						\
 }
 
+#define __BIN_ATTR_WO(_name) {						\
+	.attr	= { .name = __stringify(_name), .mode = 0200 },		\
+	.store	= _name##_store,					\
+	.size	= _size,						\
+}
+
 #define __BIN_ATTR_RW(_name, _size)					\
 	__BIN_ATTR(_name, 0644, _name##_read, _name##_write, _size)
 
@@ -208,6 +214,9 @@ struct bin_attribute bin_attr_##_name = __BIN_ATTR(_name, _mode, _read,	\
 #define BIN_ATTR_RO(_name, _size)					\
 struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
 
+#define BIN_ATTR_WO(_name, _size)					\
+struct bin_attribute bin_attr_##_name = __BIN_ATTR_WO(_name, _size)
+
 #define BIN_ATTR_RW(_name, _size)					\
 struct bin_attribute bin_attr_##_name = __BIN_ATTR_RW(_name, _size)
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
  2019-08-26 14:56     ` Greg Kroah-Hartman
@ 2019-08-26 15:46       ` Nayna
  -1 siblings, 0 replies; 37+ messages in thread
From: Nayna @ 2019-08-26 15:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Nayna Jain
  Cc: linuxppc-dev, linux-efi, linux-integrity, linux-kernel,
	Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Ard Biesheuvel, Jeremy Kerr, Matthew Garret, Mimi Zohar,
	Claudio Carvalho, George Wilson, Elaine Palmer, Eric Ricther,
	Oliver O'Halloran



On 08/26/2019 10:56 AM, Greg Kroah-Hartman wrote:
> On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
>> +static struct kobj_attribute size_attr = __ATTR_RO(size);
> Wait, why not just normal ATTR_RO()?

Oh!! Sorry. I am not seeing this macro in sysfs.h. am I missing something ?

>
>> +static struct bin_attribute data_attr = __BIN_ATTR_RO(data, VARIABLE_MAX_SIZE);
> And BIN_ATTR_RO() here?

This would have worked. I think I just thought to use the same way as 
__ATTR_RO().
I will change this to __BIN_ATTR_RO and use __BIN_ATTR_WO from your patch.

Thanks for the patch.

Thanks & Regards,
     - Nayna

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
@ 2019-08-26 15:46       ` Nayna
  0 siblings, 0 replies; 37+ messages in thread
From: Nayna @ 2019-08-26 15:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Nayna Jain
  Cc: linux-efi, Ard Biesheuvel, Eric Ricther, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, linuxppc-dev,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, linux-integrity, George Wilson



On 08/26/2019 10:56 AM, Greg Kroah-Hartman wrote:
> On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
>> +static struct kobj_attribute size_attr = __ATTR_RO(size);
> Wait, why not just normal ATTR_RO()?

Oh!! Sorry. I am not seeing this macro in sysfs.h. am I missing something ?

>
>> +static struct bin_attribute data_attr = __BIN_ATTR_RO(data, VARIABLE_MAX_SIZE);
> And BIN_ATTR_RO() here?

This would have worked. I think I just thought to use the same way as 
__ATTR_RO().
I will change this to __BIN_ATTR_RO and use __BIN_ATTR_WO from your patch.

Thanks for the patch.

Thanks & Regards,
     - Nayna

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
  2019-08-26 15:46       ` Nayna
@ 2019-08-26 15:57         ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-26 15:57 UTC (permalink / raw)
  To: Nayna
  Cc: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity,
	linux-kernel, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Ard Biesheuvel, Jeremy Kerr, Matthew Garret,
	Mimi Zohar, Claudio Carvalho, George Wilson, Elaine Palmer,
	Eric Ricther, Oliver O'Halloran

On Mon, Aug 26, 2019 at 11:46:11AM -0400, Nayna wrote:
> 
> 
> On 08/26/2019 10:56 AM, Greg Kroah-Hartman wrote:
> > On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
> > > +static struct kobj_attribute size_attr = __ATTR_RO(size);
> > Wait, why not just normal ATTR_RO()?
> 
> Oh!! Sorry. I am not seeing this macro in sysfs.h. am I missing something ?

Ugh, no, you are right, I thought it was there as the BIN_ATTR_RO() one
was there :)

> > > +static struct bin_attribute data_attr = __BIN_ATTR_RO(data, VARIABLE_MAX_SIZE);
> > And BIN_ATTR_RO() here?
> 
> This would have worked. I think I just thought to use the same way as
> __ATTR_RO().

Yes, that's fine to use, sorry for the noise.

greg k-h

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs
@ 2019-08-26 15:57         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-26 15:57 UTC (permalink / raw)
  To: Nayna
  Cc: linux-efi, Ard Biesheuvel, Eric Ricther, Nayna Jain,
	linux-kernel, Mimi Zohar, Claudio Carvalho, Matthew Garret,
	linuxppc-dev, Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, linux-integrity, George Wilson

On Mon, Aug 26, 2019 at 11:46:11AM -0400, Nayna wrote:
> 
> 
> On 08/26/2019 10:56 AM, Greg Kroah-Hartman wrote:
> > On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
> > > +static struct kobj_attribute size_attr = __ATTR_RO(size);
> > Wait, why not just normal ATTR_RO()?
> 
> Oh!! Sorry. I am not seeing this macro in sysfs.h. am I missing something ?

Ugh, no, you are right, I thought it was there as the BIN_ATTR_RO() one
was there :)

> > > +static struct bin_attribute data_attr = __BIN_ATTR_RO(data, VARIABLE_MAX_SIZE);
> > And BIN_ATTR_RO() here?
> 
> This would have worked. I think I just thought to use the same way as
> __ATTR_RO().

Yes, that's fine to use, sorry for the noise.

greg k-h

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 3/4] x86/efi: move common keyring handler functions to new file
  2019-08-26 13:23   ` Nayna Jain
@ 2019-09-02 11:55     ` Michael Ellerman
  -1 siblings, 0 replies; 37+ messages in thread
From: Michael Ellerman @ 2019-09-02 11:55 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: linux-kernel, Benjamin Herrenschmidt, Paul Mackerras,
	Ard Biesheuvel, Jeremy Kerr, Matthew Garret, Mimi Zohar,
	Greg Kroah-Hartman, Claudio Carvalho, George Wilson,
	Elaine Palmer, Eric Ricther, Oliver O'Halloran, Nayna Jain

Nayna Jain <nayna@linux.ibm.com> writes:

> The handlers to add the keys to the .platform keyring and blacklisted
> hashes to the .blacklist keyring is common for both the uefi and powerpc
> mechanisms of loading the keys/hashes from the firmware.
>
> This patch moves the common code from load_uefi.c to keyring_handler.c
>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  security/integrity/Makefile                   |  3 +-
>  .../platform_certs/keyring_handler.c          | 80 +++++++++++++++++++
>  .../platform_certs/keyring_handler.h          | 32 ++++++++
>  security/integrity/platform_certs/load_uefi.c | 67 +---------------
>  4 files changed, 115 insertions(+), 67 deletions(-)
>  create mode 100644 security/integrity/platform_certs/keyring_handler.c
>  create mode 100644 security/integrity/platform_certs/keyring_handler.h

This has no acks from security folks, though I'm not really clear on who
maintains those files.

Do I take it because it's mostly just code movement people are OK with
it going in via the powerpc tree?

cheers

> 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 000000000000..c5ba695c10e3
> --- /dev/null
> +++ b/security/integrity/platform_certs/keyring_handler.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/kernel.h>
> +#include <linux/sched.h>
> +#include <linux/cred.h>
> +#include <linux/err.h>
> +#include <linux/efi.h>
> +#include <linux/slab.h>
> +#include <keys/asymmetric-type.h>
> +#include <keys/system_keyring.h>
> +#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 000000000000..2462bfa08fe3
> --- /dev/null
> +++ b/security/integrity/platform_certs/keyring_handler.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef PLATFORM_CERTS_INTERNAL_H
> +#define PLATFORM_CERTS_INTERNAL_H
> +
> +#include <linux/efi.h>
> +
> +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 get_handler_for_dbx(const efi_guid_t *sig_type);
> +
> +#endif
> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> index 81b19c52832b..4369204a19cd 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -9,6 +9,7 @@
>  #include <keys/asymmetric-type.h>
>  #include <keys/system_keyring.h>
>  #include "../integrity.h"
> +#include "keyring_handler.h"
>  
>  static efi_guid_t efi_cert_x509_guid __initdata = EFI_CERT_X509_GUID;
>  static efi_guid_t efi_cert_x509_sha256_guid __initdata =
> @@ -67,72 +68,6 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
>  	return db;
>  }
>  
> -/*
> - * 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.
> - */
> -static __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.
> - */
> -static __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;
> -}
> -
>  /*
>   * 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
> -- 
> 2.20.1

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 3/4] x86/efi: move common keyring handler functions to new file
@ 2019-09-02 11:55     ` Michael Ellerman
  0 siblings, 0 replies; 37+ messages in thread
From: Michael Ellerman @ 2019-09-02 11:55 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Nayna Jain, linux-kernel,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Paul Mackerras,
	Jeremy Kerr, Elaine Palmer, Greg Kroah-Hartman,
	Oliver O'Halloran, George Wilson

Nayna Jain <nayna@linux.ibm.com> writes:

> The handlers to add the keys to the .platform keyring and blacklisted
> hashes to the .blacklist keyring is common for both the uefi and powerpc
> mechanisms of loading the keys/hashes from the firmware.
>
> This patch moves the common code from load_uefi.c to keyring_handler.c
>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  security/integrity/Makefile                   |  3 +-
>  .../platform_certs/keyring_handler.c          | 80 +++++++++++++++++++
>  .../platform_certs/keyring_handler.h          | 32 ++++++++
>  security/integrity/platform_certs/load_uefi.c | 67 +---------------
>  4 files changed, 115 insertions(+), 67 deletions(-)
>  create mode 100644 security/integrity/platform_certs/keyring_handler.c
>  create mode 100644 security/integrity/platform_certs/keyring_handler.h

This has no acks from security folks, though I'm not really clear on who
maintains those files.

Do I take it because it's mostly just code movement people are OK with
it going in via the powerpc tree?

cheers

> 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 000000000000..c5ba695c10e3
> --- /dev/null
> +++ b/security/integrity/platform_certs/keyring_handler.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/kernel.h>
> +#include <linux/sched.h>
> +#include <linux/cred.h>
> +#include <linux/err.h>
> +#include <linux/efi.h>
> +#include <linux/slab.h>
> +#include <keys/asymmetric-type.h>
> +#include <keys/system_keyring.h>
> +#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 000000000000..2462bfa08fe3
> --- /dev/null
> +++ b/security/integrity/platform_certs/keyring_handler.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef PLATFORM_CERTS_INTERNAL_H
> +#define PLATFORM_CERTS_INTERNAL_H
> +
> +#include <linux/efi.h>
> +
> +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 get_handler_for_dbx(const efi_guid_t *sig_type);
> +
> +#endif
> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> index 81b19c52832b..4369204a19cd 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -9,6 +9,7 @@
>  #include <keys/asymmetric-type.h>
>  #include <keys/system_keyring.h>
>  #include "../integrity.h"
> +#include "keyring_handler.h"
>  
>  static efi_guid_t efi_cert_x509_guid __initdata = EFI_CERT_X509_GUID;
>  static efi_guid_t efi_cert_x509_sha256_guid __initdata =
> @@ -67,72 +68,6 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
>  	return db;
>  }
>  
> -/*
> - * 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.
> - */
> -static __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.
> - */
> -static __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;
> -}
> -
>  /*
>   * 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
> -- 
> 2.20.1

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] sysfs: add BIN_ATTR_WO() macro
  2019-08-26 15:01         ` Greg Kroah-Hartman
@ 2019-09-03  3:37           ` Michael Ellerman
  -1 siblings, 0 replies; 37+ messages in thread
From: Michael Ellerman @ 2019-09-03  3:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Nayna
  Cc: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity,
	linux-kernel, Benjamin Herrenschmidt, Paul Mackerras,
	Ard Biesheuvel, Jeremy Kerr, Matthew Garret, Mimi Zohar,
	Claudio Carvalho, George Wilson, Elaine Palmer, Eric Ricther,
	Oliver O'Halloran

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> This variant was missing from sysfs.h, I guess no one noticed it before.
>
> Turns out the powerpc secure variable code can use it, so add it to the
> tree for it, and potentially others to take advantage of, instead of
> open-coding it.
>
> Reported-by: Nayna Jain <nayna@linux.ibm.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>
> I'll queue this up to my tree for 5.4-rc1, but if you want to take this
> in your tree earlier, feel free to do so.

OK. This series is blocked on the firmware support going in, so at the
moment it might miss v5.4 anyway. So this going via your tree is no
problem.

If it does make it into v5.4 we can do a fixup patch to use the new
macro once everything's in Linus' tree.

cheers

> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 965236795750..5420817ed317 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -196,6 +196,12 @@ struct bin_attribute {
>  	.size	= _size,						\
>  }
>  
> +#define __BIN_ATTR_WO(_name) {						\
> +	.attr	= { .name = __stringify(_name), .mode = 0200 },		\
> +	.store	= _name##_store,					\
> +	.size	= _size,						\
> +}
> +
>  #define __BIN_ATTR_RW(_name, _size)					\
>  	__BIN_ATTR(_name, 0644, _name##_read, _name##_write, _size)
>  
> @@ -208,6 +214,9 @@ struct bin_attribute bin_attr_##_name = __BIN_ATTR(_name, _mode, _read,	\
>  #define BIN_ATTR_RO(_name, _size)					\
>  struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
>  
> +#define BIN_ATTR_WO(_name, _size)					\
> +struct bin_attribute bin_attr_##_name = __BIN_ATTR_WO(_name, _size)
> +
>  #define BIN_ATTR_RW(_name, _size)					\
>  struct bin_attribute bin_attr_##_name = __BIN_ATTR_RW(_name, _size)
>  
> -- 
> 2.23.0

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] sysfs: add BIN_ATTR_WO() macro
@ 2019-09-03  3:37           ` Michael Ellerman
  0 siblings, 0 replies; 37+ messages in thread
From: Michael Ellerman @ 2019-09-03  3:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Nayna
  Cc: linux-efi, Ard Biesheuvel, Eric Ricther, Nayna Jain,
	linux-kernel, Mimi Zohar, Claudio Carvalho, Matthew Garret,
	linuxppc-dev, Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, linux-integrity, George Wilson

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> This variant was missing from sysfs.h, I guess no one noticed it before.
>
> Turns out the powerpc secure variable code can use it, so add it to the
> tree for it, and potentially others to take advantage of, instead of
> open-coding it.
>
> Reported-by: Nayna Jain <nayna@linux.ibm.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>
> I'll queue this up to my tree for 5.4-rc1, but if you want to take this
> in your tree earlier, feel free to do so.

OK. This series is blocked on the firmware support going in, so at the
moment it might miss v5.4 anyway. So this going via your tree is no
problem.

If it does make it into v5.4 we can do a fixup patch to use the new
macro once everything's in Linus' tree.

cheers

> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 965236795750..5420817ed317 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -196,6 +196,12 @@ struct bin_attribute {
>  	.size	= _size,						\
>  }
>  
> +#define __BIN_ATTR_WO(_name) {						\
> +	.attr	= { .name = __stringify(_name), .mode = 0200 },		\
> +	.store	= _name##_store,					\
> +	.size	= _size,						\
> +}
> +
>  #define __BIN_ATTR_RW(_name, _size)					\
>  	__BIN_ATTR(_name, 0644, _name##_read, _name##_write, _size)
>  
> @@ -208,6 +214,9 @@ struct bin_attribute bin_attr_##_name = __BIN_ATTR(_name, _mode, _read,	\
>  #define BIN_ATTR_RO(_name, _size)					\
>  struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
>  
> +#define BIN_ATTR_WO(_name, _size)					\
> +struct bin_attribute bin_attr_##_name = __BIN_ATTR_WO(_name, _size)
> +
>  #define BIN_ATTR_RW(_name, _size)					\
>  struct bin_attribute bin_attr_##_name = __BIN_ATTR_RW(_name, _size)
>  
> -- 
> 2.23.0

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 3/4] x86/efi: move common keyring handler functions to new file
  2019-09-02 11:55     ` Michael Ellerman
@ 2019-09-03 22:51       ` Mimi Zohar
  -1 siblings, 0 replies; 37+ messages in thread
From: Mimi Zohar @ 2019-09-03 22:51 UTC (permalink / raw)
  To: Michael Ellerman, Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: linux-kernel, Benjamin Herrenschmidt, Paul Mackerras,
	Ard Biesheuvel, Jeremy Kerr, Matthew Garret, Greg Kroah-Hartman,
	Claudio Carvalho, George Wilson, Elaine Palmer, Eric Ricther,
	Oliver O'Halloran, Josh Boyer, David Howells

(Cc'ing Josh Boyer, David Howells)

On Mon, 2019-09-02 at 21:55 +1000, Michael Ellerman wrote:
> Nayna Jain <nayna@linux.ibm.com> writes:
> 
> > The handlers to add the keys to the .platform keyring and blacklisted
> > hashes to the .blacklist keyring is common for both the uefi and powerpc
> > mechanisms of loading the keys/hashes from the firmware.
> >
> > This patch moves the common code from load_uefi.c to keyring_handler.c
> >
> > Signed-off-by: Nayna Jain <nayna@linux.ibm.com>

Acked-by: Mimi Zohar <zohar@linux.ibm.com>

> > ---
> >  security/integrity/Makefile                   |  3 +-
> >  .../platform_certs/keyring_handler.c          | 80 +++++++++++++++++++
> >  .../platform_certs/keyring_handler.h          | 32 ++++++++
> >  security/integrity/platform_certs/load_uefi.c | 67 +---------------
> >  4 files changed, 115 insertions(+), 67 deletions(-)
> >  create mode 100644 security/integrity/platform_certs/keyring_handler.c
> >  create mode 100644 security/integrity/platform_certs/keyring_handler.h
> 
> This has no acks from security folks, though I'm not really clear on who
> maintains those files.

I upstreamed David's, Josh's, and Nayna's patches, so that's probably
me.

> Do I take it because it's mostly just code movement people are OK with
> it going in via the powerpc tree?

Yes, the only reason for splitting load_uefi.c is for powerpc.  These
patches should be upstreamed together.  

Mimi


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 3/4] x86/efi: move common keyring handler functions to new file
@ 2019-09-03 22:51       ` Mimi Zohar
  0 siblings, 0 replies; 37+ messages in thread
From: Mimi Zohar @ 2019-09-03 22:51 UTC (permalink / raw)
  To: Michael Ellerman, Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: David Howells, Josh Boyer, Ard Biesheuvel, Eric Ricther,
	linux-kernel, Claudio Carvalho, Matthew Garret, Paul Mackerras,
	Jeremy Kerr, Elaine Palmer, Greg Kroah-Hartman,
	Oliver O'Halloran, George Wilson

(Cc'ing Josh Boyer, David Howells)

On Mon, 2019-09-02 at 21:55 +1000, Michael Ellerman wrote:
> Nayna Jain <nayna@linux.ibm.com> writes:
> 
> > The handlers to add the keys to the .platform keyring and blacklisted
> > hashes to the .blacklist keyring is common for both the uefi and powerpc
> > mechanisms of loading the keys/hashes from the firmware.
> >
> > This patch moves the common code from load_uefi.c to keyring_handler.c
> >
> > Signed-off-by: Nayna Jain <nayna@linux.ibm.com>

Acked-by: Mimi Zohar <zohar@linux.ibm.com>

> > ---
> >  security/integrity/Makefile                   |  3 +-
> >  .../platform_certs/keyring_handler.c          | 80 +++++++++++++++++++
> >  .../platform_certs/keyring_handler.h          | 32 ++++++++
> >  security/integrity/platform_certs/load_uefi.c | 67 +---------------
> >  4 files changed, 115 insertions(+), 67 deletions(-)
> >  create mode 100644 security/integrity/platform_certs/keyring_handler.c
> >  create mode 100644 security/integrity/platform_certs/keyring_handler.h
> 
> This has no acks from security folks, though I'm not really clear on who
> maintains those files.

I upstreamed David's, Josh's, and Nayna's patches, so that's probably
me.

> Do I take it because it's mostly just code movement people are OK with
> it going in via the powerpc tree?

Yes, the only reason for splitting load_uefi.c is for powerpc.  These
patches should be upstreamed together.  

Mimi


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 4/4] powerpc: load firmware trusted keys/hashes into kernel keyring
  2019-08-26 13:23   ` Nayna Jain
@ 2019-09-03 22:54     ` Mimi Zohar
  -1 siblings, 0 replies; 37+ messages in thread
From: Mimi Zohar @ 2019-09-03 22:54 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: linux-kernel, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Ard Biesheuvel, Jeremy Kerr, Matthew Garret,
	Greg Kroah-Hartman, Claudio Carvalho, George Wilson,
	Elaine Palmer, Eric Ricther, Oliver O'Halloran

On Mon, 2019-08-26 at 09:23 -0400, Nayna Jain wrote:
> The keys used to verify the Host OS kernel are managed by firmware as
> secure variables. This patch loads the verification keys into the .platform
> keyring and revocation hashes 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 <nayna@linux.ibm.com>

Feel free to add my tag after addressing the formatting issues.

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c
> new file mode 100644
> index 000000000000..359d5063d4da
> --- /dev/null
> +++ b/security/integrity/platform_certs/load_powerpc.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 IBM Corporation
> + * Author: Nayna Jain <nayna@linux.ibm.com>
> + *
> + *      - loads keys and hashes stored and controlled by the firmware.
> + */
> +#include <linux/kernel.h>
> +#include <linux/sched.h>
> +#include <linux/cred.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <asm/secboot.h>
> +#include <asm/secvar.h>
> +#include "keyring_handler.h"
> +
> +/*
> + * Get a certificate list blob from the named secure variable.
> + */
> +static __init void *get_cert_list(u8 *key, unsigned long keylen, uint64_t *size)
> +{
> +	int rc;
> +	void *db;
> +
> +	rc = secvar_ops->get(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 = secvar_ops->get(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 keys databases into the platform trusted
> + * keyring and the blacklisted X.509 cert SHA256 hashes into the blacklist
> + * keyring.
> + */
> +static int __init load_powerpc_certs(void)
> +{
> +	void *db = NULL, *dbx = NULL;
> +	uint64_t dbsize = 0, dbxsize = 0;
> +	int rc = 0;
> +
> +	if (!secvar_ops)
> +		return -ENODEV;
> +
> +	/* 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, &dbsize);
> +	if (!db) {
> +		pr_err("Couldn't get db list from firmware\n");
> +	} else {
> +		rc = parse_efi_signature_list("powerpc:db",
> +				db, dbsize, get_handler_for_db);
> +		if (rc)
> +			pr_err("Couldn't parse db signatures: %d\n",
> +					rc);

There's no need to split this line.

> +		kfree(db);
> +	}
> +
> +	dbx = get_cert_list("dbx", 3,  &dbxsize);
> +	if (!dbx) {
> +		pr_info("Couldn't get dbx list from firmware\n");
> +	} else {
> +		rc = parse_efi_signature_list("powerpc:dbx",
> +				dbx, dbxsize,
> +				get_handler_for_dbx);

Formatting of this line is off.

> +		if (rc)
> +			pr_err("Couldn't parse dbx signatures: %d\n", rc);
> +		kfree(dbx);
> +	}
> +
> +	return rc;
> +}
> +late_initcall(load_powerpc_certs);


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 4/4] powerpc: load firmware trusted keys/hashes into kernel keyring
@ 2019-09-03 22:54     ` Mimi Zohar
  0 siblings, 0 replies; 37+ messages in thread
From: Mimi Zohar @ 2019-09-03 22:54 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, linux-kernel, Claudio Carvalho,
	Matthew Garret, Greg Kroah-Hartman, Paul Mackerras, Jeremy Kerr,
	Elaine Palmer, Oliver O'Halloran, George Wilson

On Mon, 2019-08-26 at 09:23 -0400, Nayna Jain wrote:
> The keys used to verify the Host OS kernel are managed by firmware as
> secure variables. This patch loads the verification keys into the .platform
> keyring and revocation hashes 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 <nayna@linux.ibm.com>

Feel free to add my tag after addressing the formatting issues.

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c
> new file mode 100644
> index 000000000000..359d5063d4da
> --- /dev/null
> +++ b/security/integrity/platform_certs/load_powerpc.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 IBM Corporation
> + * Author: Nayna Jain <nayna@linux.ibm.com>
> + *
> + *      - loads keys and hashes stored and controlled by the firmware.
> + */
> +#include <linux/kernel.h>
> +#include <linux/sched.h>
> +#include <linux/cred.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <asm/secboot.h>
> +#include <asm/secvar.h>
> +#include "keyring_handler.h"
> +
> +/*
> + * Get a certificate list blob from the named secure variable.
> + */
> +static __init void *get_cert_list(u8 *key, unsigned long keylen, uint64_t *size)
> +{
> +	int rc;
> +	void *db;
> +
> +	rc = secvar_ops->get(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 = secvar_ops->get(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 keys databases into the platform trusted
> + * keyring and the blacklisted X.509 cert SHA256 hashes into the blacklist
> + * keyring.
> + */
> +static int __init load_powerpc_certs(void)
> +{
> +	void *db = NULL, *dbx = NULL;
> +	uint64_t dbsize = 0, dbxsize = 0;
> +	int rc = 0;
> +
> +	if (!secvar_ops)
> +		return -ENODEV;
> +
> +	/* 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, &dbsize);
> +	if (!db) {
> +		pr_err("Couldn't get db list from firmware\n");
> +	} else {
> +		rc = parse_efi_signature_list("powerpc:db",
> +				db, dbsize, get_handler_for_db);
> +		if (rc)
> +			pr_err("Couldn't parse db signatures: %d\n",
> +					rc);

There's no need to split this line.

> +		kfree(db);
> +	}
> +
> +	dbx = get_cert_list("dbx", 3,  &dbxsize);
> +	if (!dbx) {
> +		pr_info("Couldn't get dbx list from firmware\n");
> +	} else {
> +		rc = parse_efi_signature_list("powerpc:dbx",
> +				dbx, dbxsize,
> +				get_handler_for_dbx);

Formatting of this line is off.

> +		if (rc)
> +			pr_err("Couldn't parse dbx signatures: %d\n", rc);
> +		kfree(dbx);
> +	}
> +
> +	return rc;
> +}
> +late_initcall(load_powerpc_certs);


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] sysfs: add BIN_ATTR_WO() macro
  2019-09-03  3:37           ` Michael Ellerman
@ 2019-09-04 11:36             ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2019-09-04 11:36 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nayna, Nayna Jain, linuxppc-dev, linux-efi, linux-integrity,
	linux-kernel, Benjamin Herrenschmidt, Paul Mackerras,
	Ard Biesheuvel, Jeremy Kerr, Matthew Garret, Mimi Zohar,
	Claudio Carvalho, George Wilson, Elaine Palmer, Eric Ricther,
	Oliver O'Halloran

On Tue, Sep 03, 2019 at 01:37:02PM +1000, Michael Ellerman wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > This variant was missing from sysfs.h, I guess no one noticed it before.
> >
> > Turns out the powerpc secure variable code can use it, so add it to the
> > tree for it, and potentially others to take advantage of, instead of
> > open-coding it.
> >
> > Reported-by: Nayna Jain <nayna@linux.ibm.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >
> > I'll queue this up to my tree for 5.4-rc1, but if you want to take this
> > in your tree earlier, feel free to do so.
> 
> OK. This series is blocked on the firmware support going in, so at the
> moment it might miss v5.4 anyway. So this going via your tree is no
> problem.

Ok, will queue it up now, thanks!

greg k-h

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] sysfs: add BIN_ATTR_WO() macro
@ 2019-09-04 11:36             ` Greg Kroah-Hartman
  0 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2019-09-04 11:36 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linux-efi, Nayna, Ard Biesheuvel, Eric Ricther, Nayna Jain,
	linux-kernel, Mimi Zohar, Claudio Carvalho, Matthew Garret,
	linuxppc-dev, Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, linux-integrity, George Wilson

On Tue, Sep 03, 2019 at 01:37:02PM +1000, Michael Ellerman wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > This variant was missing from sysfs.h, I guess no one noticed it before.
> >
> > Turns out the powerpc secure variable code can use it, so add it to the
> > tree for it, and potentially others to take advantage of, instead of
> > open-coding it.
> >
> > Reported-by: Nayna Jain <nayna@linux.ibm.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >
> > I'll queue this up to my tree for 5.4-rc1, but if you want to take this
> > in your tree earlier, feel free to do so.
> 
> OK. This series is blocked on the firmware support going in, so at the
> moment it might miss v5.4 anyway. So this going via your tree is no
> problem.

Ok, will queue it up now, thanks!

greg k-h

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 3/4] x86/efi: move common keyring handler functions to new file
  2019-09-03 22:51       ` Mimi Zohar
@ 2019-09-05  3:59         ` Michael Ellerman
  -1 siblings, 0 replies; 37+ messages in thread
From: Michael Ellerman @ 2019-09-05  3:59 UTC (permalink / raw)
  To: Mimi Zohar, Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: linux-kernel, Benjamin Herrenschmidt, Paul Mackerras,
	Ard Biesheuvel, Jeremy Kerr, Matthew Garret, Greg Kroah-Hartman,
	Claudio Carvalho, George Wilson, Elaine Palmer, Eric Ricther,
	Oliver O'Halloran, Josh Boyer, David Howells

Mimi Zohar <zohar@linux.ibm.com> writes:
> (Cc'ing Josh Boyer, David Howells)
>
> On Mon, 2019-09-02 at 21:55 +1000, Michael Ellerman wrote:
>> Nayna Jain <nayna@linux.ibm.com> writes:
>> 
>> > The handlers to add the keys to the .platform keyring and blacklisted
>> > hashes to the .blacklist keyring is common for both the uefi and powerpc
>> > mechanisms of loading the keys/hashes from the firmware.
>> >
>> > This patch moves the common code from load_uefi.c to keyring_handler.c
>> >
>> > Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
>
> Acked-by: Mimi Zohar <zohar@linux.ibm.com>
>
>> > ---
>> >  security/integrity/Makefile                   |  3 +-
>> >  .../platform_certs/keyring_handler.c          | 80 +++++++++++++++++++
>> >  .../platform_certs/keyring_handler.h          | 32 ++++++++
>> >  security/integrity/platform_certs/load_uefi.c | 67 +---------------
>> >  4 files changed, 115 insertions(+), 67 deletions(-)
>> >  create mode 100644 security/integrity/platform_certs/keyring_handler.c
>> >  create mode 100644 security/integrity/platform_certs/keyring_handler.h
>> 
>> This has no acks from security folks, though I'm not really clear on who
>> maintains those files.
>
> I upstreamed David's, Josh's, and Nayna's patches, so that's probably
> me.
>
>> Do I take it because it's mostly just code movement people are OK with
>> it going in via the powerpc tree?
>
> Yes, the only reason for splitting load_uefi.c is for powerpc.  These
> patches should be upstreamed together.  

Thanks.

cheers

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 3/4] x86/efi: move common keyring handler functions to new file
@ 2019-09-05  3:59         ` Michael Ellerman
  0 siblings, 0 replies; 37+ messages in thread
From: Michael Ellerman @ 2019-09-05  3:59 UTC (permalink / raw)
  To: Mimi Zohar, Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: David Howells, Josh Boyer, Ard Biesheuvel, Eric Ricther,
	linux-kernel, Claudio Carvalho, Matthew Garret, Paul Mackerras,
	Jeremy Kerr, Elaine Palmer, Greg Kroah-Hartman,
	Oliver O'Halloran, George Wilson

Mimi Zohar <zohar@linux.ibm.com> writes:
> (Cc'ing Josh Boyer, David Howells)
>
> On Mon, 2019-09-02 at 21:55 +1000, Michael Ellerman wrote:
>> Nayna Jain <nayna@linux.ibm.com> writes:
>> 
>> > The handlers to add the keys to the .platform keyring and blacklisted
>> > hashes to the .blacklist keyring is common for both the uefi and powerpc
>> > mechanisms of loading the keys/hashes from the firmware.
>> >
>> > This patch moves the common code from load_uefi.c to keyring_handler.c
>> >
>> > Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
>
> Acked-by: Mimi Zohar <zohar@linux.ibm.com>
>
>> > ---
>> >  security/integrity/Makefile                   |  3 +-
>> >  .../platform_certs/keyring_handler.c          | 80 +++++++++++++++++++
>> >  .../platform_certs/keyring_handler.h          | 32 ++++++++
>> >  security/integrity/platform_certs/load_uefi.c | 67 +---------------
>> >  4 files changed, 115 insertions(+), 67 deletions(-)
>> >  create mode 100644 security/integrity/platform_certs/keyring_handler.c
>> >  create mode 100644 security/integrity/platform_certs/keyring_handler.h
>> 
>> This has no acks from security folks, though I'm not really clear on who
>> maintains those files.
>
> I upstreamed David's, Josh's, and Nayna's patches, so that's probably
> me.
>
>> Do I take it because it's mostly just code movement people are OK with
>> it going in via the powerpc tree?
>
> Yes, the only reason for splitting load_uefi.c is for powerpc.  These
> patches should be upstreamed together.  

Thanks.

cheers

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] sysfs: add BIN_ATTR_WO() macro
  2019-08-26 15:01         ` Greg Kroah-Hartman
  (?)
  (?)
@ 2019-10-01 18:08         ` Nayna
  2019-10-01 18:16           ` Greg Kroah-Hartman
  -1 siblings, 1 reply; 37+ messages in thread
From: Nayna @ 2019-10-01 18:08 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-efi, Ard Biesheuvel, Eric Ricther, Nayna Jain,
	linux-kernel, Mimi Zohar, Claudio Carvalho, Matthew Garret,
	linuxppc-dev, Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, linux-integrity, George Wilson

Hi Greg,


On 08/26/2019 11:01 AM, Greg Kroah-Hartman wrote:
> This variant was missing from sysfs.h, I guess no one noticed it before.
>
> Turns out the powerpc secure variable code can use it, so add it to the
> tree for it, and potentially others to take advantage of, instead of
> open-coding it.
>
> Reported-by: Nayna Jain <nayna@linux.ibm.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>
> I'll queue this up to my tree for 5.4-rc1, but if you want to take this
> in your tree earlier, feel free to do so.
>
>   include/linux/sysfs.h | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 965236795750..5420817ed317 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -196,6 +196,12 @@ struct bin_attribute {
>   	.size	= _size,						\
>   }
>   
> +#define __BIN_ATTR_WO(_name) {						\
> +	.attr	= { .name = __stringify(_name), .mode = 0200 },		\
> +	.store	= _name##_store,					\
> +	.size	= _size,						\
> +}
> +
>   #define __BIN_ATTR_RW(_name, _size)					\
>   	__BIN_ATTR(_name, 0644, _name##_read, _name##_write, _size)
>   
> @@ -208,6 +214,9 @@ struct bin_attribute bin_attr_##_name = __BIN_ATTR(_name, _mode, _read,	\
>   #define BIN_ATTR_RO(_name, _size)					\
>   struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
>   
> +#define BIN_ATTR_WO(_name, _size)					\
> +struct bin_attribute bin_attr_##_name = __BIN_ATTR_WO(_name, _size)
> +
>   #define BIN_ATTR_RW(_name, _size)					\
>   struct bin_attribute bin_attr_##_name = __BIN_ATTR_RW(_name, _size)
>   

I am sorry. I didn't notice it via inspection but there is a bug in this 
macro. When I actually try using it, compilation fails. Here's a likely 
patch:

diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 5420817ed317..fa7ee503fb76 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -196,9 +196,9 @@ struct bin_attribute {
         .size   = _size,                                                \
  }
  
-#define __BIN_ATTR_WO(_name) {                                         \
+#define __BIN_ATTR_WO(_name, _size) {                                  \
         .attr   = { .name = __stringify(_name), .mode = 0200 },         \
-       .store  = _name##_store,                                        \
+       .write  = _name##_write,                                        \
         .size   = _size,                                                \
  }


Thanks & Regards,
     - Nayna


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH] sysfs: add BIN_ATTR_WO() macro
  2019-10-01 18:08         ` Nayna
@ 2019-10-01 18:16           ` Greg Kroah-Hartman
  2019-10-01 18:55             ` Nayna
  0 siblings, 1 reply; 37+ messages in thread
From: Greg Kroah-Hartman @ 2019-10-01 18:16 UTC (permalink / raw)
  To: Nayna
  Cc: linux-efi, Ard Biesheuvel, Eric Ricther, Nayna Jain,
	linux-kernel, Mimi Zohar, Claudio Carvalho, Matthew Garret,
	linuxppc-dev, Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, linux-integrity, George Wilson

On Tue, Oct 01, 2019 at 02:08:53PM -0400, Nayna wrote:
> Hi Greg,
> 
> 
> On 08/26/2019 11:01 AM, Greg Kroah-Hartman wrote:
> > This variant was missing from sysfs.h, I guess no one noticed it before.
> > 
> > Turns out the powerpc secure variable code can use it, so add it to the
> > tree for it, and potentially others to take advantage of, instead of
> > open-coding it.
> > 
> > Reported-by: Nayna Jain <nayna@linux.ibm.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > 
> > I'll queue this up to my tree for 5.4-rc1, but if you want to take this
> > in your tree earlier, feel free to do so.
> > 
> >   include/linux/sysfs.h | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> > 
> > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> > index 965236795750..5420817ed317 100644
> > --- a/include/linux/sysfs.h
> > +++ b/include/linux/sysfs.h
> > @@ -196,6 +196,12 @@ struct bin_attribute {
> >   	.size	= _size,						\
> >   }
> > +#define __BIN_ATTR_WO(_name) {						\
> > +	.attr	= { .name = __stringify(_name), .mode = 0200 },		\
> > +	.store	= _name##_store,					\
> > +	.size	= _size,						\
> > +}
> > +
> >   #define __BIN_ATTR_RW(_name, _size)					\
> >   	__BIN_ATTR(_name, 0644, _name##_read, _name##_write, _size)
> > @@ -208,6 +214,9 @@ struct bin_attribute bin_attr_##_name = __BIN_ATTR(_name, _mode, _read,	\
> >   #define BIN_ATTR_RO(_name, _size)					\
> >   struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
> > +#define BIN_ATTR_WO(_name, _size)					\
> > +struct bin_attribute bin_attr_##_name = __BIN_ATTR_WO(_name, _size)
> > +
> >   #define BIN_ATTR_RW(_name, _size)					\
> >   struct bin_attribute bin_attr_##_name = __BIN_ATTR_RW(_name, _size)
> 
> I am sorry. I didn't notice it via inspection but there is a bug in this
> macro. When I actually try using it, compilation fails. Here's a likely
> patch:
> 
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 5420817ed317..fa7ee503fb76 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -196,9 +196,9 @@ struct bin_attribute {
>         .size   = _size,                                                \
>  }
> -#define __BIN_ATTR_WO(_name) {                                         \
> +#define __BIN_ATTR_WO(_name, _size) {                                  \
>         .attr   = { .name = __stringify(_name), .mode = 0200 },         \
> -       .store  = _name##_store,                                        \
> +       .write  = _name##_write,                                        \
>         .size   = _size,                                                \
>  }
> 

Heh, good catch.  Can you send a real patch for this that I can apply to
give you the proper credit for finding and fixing this?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] sysfs: add BIN_ATTR_WO() macro
  2019-10-01 18:16           ` Greg Kroah-Hartman
@ 2019-10-01 18:55             ` Nayna
  0 siblings, 0 replies; 37+ messages in thread
From: Nayna @ 2019-10-01 18:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-efi, Ard Biesheuvel, Eric Ricther, Nayna Jain,
	linux-kernel, Mimi Zohar, Claudio Carvalho, Matthew Garret,
	linuxppc-dev, Paul Mackerras, Jeremy Kerr, Elaine Palmer,
	Oliver O'Halloran, linux-integrity, George Wilson



On 10/01/2019 02:16 PM, Greg Kroah-Hartman wrote:
> On Tue, Oct 01, 2019 at 02:08:53PM -0400, Nayna wrote:
>> Hi Greg,
>>
>>
>> On 08/26/2019 11:01 AM, Greg Kroah-Hartman wrote:
>>> This variant was missing from sysfs.h, I guess no one noticed it before.
>>>
>>> Turns out the powerpc secure variable code can use it, so add it to the
>>> tree for it, and potentially others to take advantage of, instead of
>>> open-coding it.
>>>
>>> Reported-by: Nayna Jain <nayna@linux.ibm.com>
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> ---
>>>
>>> I'll queue this up to my tree for 5.4-rc1, but if you want to take this
>>> in your tree earlier, feel free to do so.
>>>
>>>    include/linux/sysfs.h | 9 +++++++++
>>>    1 file changed, 9 insertions(+)
>>>
>>> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
>>> index 965236795750..5420817ed317 100644
>>> --- a/include/linux/sysfs.h
>>> +++ b/include/linux/sysfs.h
>>> @@ -196,6 +196,12 @@ struct bin_attribute {
>>>    	.size	= _size,						\
>>>    }
>>> +#define __BIN_ATTR_WO(_name) {						\
>>> +	.attr	= { .name = __stringify(_name), .mode = 0200 },		\
>>> +	.store	= _name##_store,					\
>>> +	.size	= _size,						\
>>> +}
>>> +
>>>    #define __BIN_ATTR_RW(_name, _size)					\
>>>    	__BIN_ATTR(_name, 0644, _name##_read, _name##_write, _size)
>>> @@ -208,6 +214,9 @@ struct bin_attribute bin_attr_##_name = __BIN_ATTR(_name, _mode, _read,	\
>>>    #define BIN_ATTR_RO(_name, _size)					\
>>>    struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
>>> +#define BIN_ATTR_WO(_name, _size)					\
>>> +struct bin_attribute bin_attr_##_name = __BIN_ATTR_WO(_name, _size)
>>> +
>>>    #define BIN_ATTR_RW(_name, _size)					\
>>>    struct bin_attribute bin_attr_##_name = __BIN_ATTR_RW(_name, _size)
>> I am sorry. I didn't notice it via inspection but there is a bug in this
>> macro. When I actually try using it, compilation fails. Here's a likely
>> patch:
>>
>> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
>> index 5420817ed317..fa7ee503fb76 100644
>> --- a/include/linux/sysfs.h
>> +++ b/include/linux/sysfs.h
>> @@ -196,9 +196,9 @@ struct bin_attribute {
>>          .size   = _size,                                                \
>>   }
>> -#define __BIN_ATTR_WO(_name) {                                         \
>> +#define __BIN_ATTR_WO(_name, _size) {                                  \
>>          .attr   = { .name = __stringify(_name), .mode = 0200 },         \
>> -       .store  = _name##_store,                                        \
>> +       .write  = _name##_write,                                        \
>>          .size   = _size,                                                \
>>   }
>>
> Heh, good catch.  Can you send a real patch for this that I can apply to
> give you the proper credit for finding and fixing this?

Sure.. Thanks Greg !!

Thanks & Regards,
       - Nayna

^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2019-10-01 18:56 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-26 13:23 [PATCH v3 0/4] powerpc: expose secure variables to the kernel and userspace Nayna Jain
2019-08-26 13:23 ` Nayna Jain
2019-08-26 13:23 ` [PATCH v3 1/4] powerpc/powernv: Add OPAL API interface to access secure variable Nayna Jain
2019-08-26 13:23   ` Nayna Jain
2019-08-26 13:23 ` [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs Nayna Jain
2019-08-26 13:23   ` Nayna Jain
2019-08-26 14:01   ` Greg Kroah-Hartman
2019-08-26 14:01     ` Greg Kroah-Hartman
2019-08-26 14:12     ` Nayna
2019-08-26 14:12       ` Nayna
2019-08-26 15:01       ` [PATCH] sysfs: add BIN_ATTR_WO() macro Greg Kroah-Hartman
2019-08-26 15:01         ` Greg Kroah-Hartman
2019-09-03  3:37         ` Michael Ellerman
2019-09-03  3:37           ` Michael Ellerman
2019-09-04 11:36           ` Greg Kroah-Hartman
2019-09-04 11:36             ` Greg Kroah-Hartman
2019-10-01 18:08         ` Nayna
2019-10-01 18:16           ` Greg Kroah-Hartman
2019-10-01 18:55             ` Nayna
2019-08-26 14:56   ` [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs Greg Kroah-Hartman
2019-08-26 14:56     ` Greg Kroah-Hartman
2019-08-26 15:46     ` Nayna
2019-08-26 15:46       ` Nayna
2019-08-26 15:57       ` Greg Kroah-Hartman
2019-08-26 15:57         ` Greg Kroah-Hartman
2019-08-26 13:23 ` [PATCH v3 3/4] x86/efi: move common keyring handler functions to new file Nayna Jain
2019-08-26 13:23   ` Nayna Jain
2019-09-02 11:55   ` Michael Ellerman
2019-09-02 11:55     ` Michael Ellerman
2019-09-03 22:51     ` Mimi Zohar
2019-09-03 22:51       ` Mimi Zohar
2019-09-05  3:59       ` Michael Ellerman
2019-09-05  3:59         ` Michael Ellerman
2019-08-26 13:23 ` [PATCH v3 4/4] powerpc: load firmware trusted keys/hashes into kernel keyring Nayna Jain
2019-08-26 13:23   ` Nayna Jain
2019-09-03 22:54   ` Mimi Zohar
2019-09-03 22:54     ` Mimi Zohar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.