From 53cea34b0a0a03568e189f8dfe2eb06f938986c8 Mon Sep 17 00:00:00 2001 From: Michael Ellerman Date: Fri, 10 Feb 2023 19:03:39 +1100 Subject: powerpc/secvar: Use u64 in secvar_operations There's no reason for secvar_operations to use uint64_t vs the more common kernel type u64. The types are compatible, but they require different printk format strings which can lead to confusion. Change all the secvar related routines to use u64. Reviewed-by: Russell Currey Reviewed-by: Andrew Donnellan Signed-off-by: Andrew Donnellan Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/20230210080401.345462-5-ajd@linux.ibm.com --- security/integrity/platform_certs/load_powerpc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security/integrity') diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c index a2900cb85357..1e4f80a4e71c 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/security/integrity/platform_certs/load_powerpc.c @@ -18,7 +18,7 @@ /* * Get a certificate list blob from the named secure variable. */ -static __init void *get_cert_list(u8 *key, unsigned long keylen, uint64_t *size) +static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size) { int rc; void *db; @@ -51,7 +51,7 @@ static __init void *get_cert_list(u8 *key, unsigned long keylen, uint64_t *size) static int __init load_powerpc_certs(void) { void *db = NULL, *dbx = NULL; - uint64_t dbsize = 0, dbxsize = 0; + u64 dbsize = 0, dbxsize = 0; int rc = 0; struct device_node *node; -- cgit v1.2.3 From 3c8069b0c3832674abd80a5cf019c913e62de9a5 Mon Sep 17 00:00:00 2001 From: Russell Currey Date: Fri, 10 Feb 2023 19:04:00 +1100 Subject: integrity/powerpc: Improve error handling & reporting when loading certs A few improvements to load_powerpc.c: - include integrity.h for the pr_fmt() - move all error reporting out of get_cert_list() - use ERR_PTR() to better preserve error detail - don't use pr_err() for missing keys Reviewed-by: Mimi Zohar Signed-off-by: Russell Currey Signed-off-by: Andrew Donnellan Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/20230210080401.345462-26-ajd@linux.ibm.com --- security/integrity/platform_certs/load_powerpc.c | 26 ++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) (limited to 'security/integrity') diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c index 1e4f80a4e71c..dee51606d5f4 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/security/integrity/platform_certs/load_powerpc.c @@ -14,9 +14,15 @@ #include #include #include "keyring_handler.h" +#include "../integrity.h" /* * Get a certificate list blob from the named secure variable. + * + * Returns: + * - a pointer to a kmalloc'd buffer containing the cert list on success + * - NULL if the key does not exist + * - an ERR_PTR on error */ static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size) { @@ -25,19 +31,19 @@ static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size) rc = secvar_ops->get(key, keylen, NULL, size); if (rc) { - pr_err("Couldn't get size: %d\n", rc); - return NULL; + if (rc == -ENOENT) + return NULL; + return ERR_PTR(rc); } db = kmalloc(*size, GFP_KERNEL); if (!db) - return NULL; + return ERR_PTR(-ENOMEM); rc = secvar_ops->get(key, keylen, db, size); if (rc) { kfree(db); - pr_err("Error reading %s var: %d\n", key, rc); - return NULL; + return ERR_PTR(rc); } return db; @@ -69,7 +75,11 @@ static int __init load_powerpc_certs(void) */ db = get_cert_list("db", 3, &dbsize); if (!db) { - pr_err("Couldn't get db list from firmware\n"); + pr_info("Couldn't get db list from firmware\n"); + } else if (IS_ERR(db)) { + rc = PTR_ERR(db); + pr_err("Error reading db from firmware: %d\n", rc); + return rc; } else { rc = parse_efi_signature_list("powerpc:db", db, dbsize, get_handler_for_db); @@ -81,6 +91,10 @@ static int __init load_powerpc_certs(void) dbx = get_cert_list("dbx", 4, &dbxsize); if (!dbx) { pr_info("Couldn't get dbx list from firmware\n"); + } else if (IS_ERR(dbx)) { + rc = PTR_ERR(dbx); + pr_err("Error reading dbx from firmware: %d\n", rc); + return rc; } else { rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize, get_handler_for_dbx); -- cgit v1.2.3 From 4b3e71e9a34c48f370b6281e9477515d588e7b26 Mon Sep 17 00:00:00 2001 From: Russell Currey Date: Fri, 10 Feb 2023 19:04:01 +1100 Subject: integrity/powerpc: Support loading keys from PLPKS Add support for loading keys from the PLPKS on pseries machines, with the "ibm,plpks-sb-v1" format. The object format is expected to be the same, so there shouldn't be any functional differences between objects retrieved on powernv or pseries. Unlike on powernv, on pseries the format string isn't contained in the device tree. Use secvar_ops->format() to fetch the format string in a generic manner, rather than searching the device tree ourselves. (The current code searches the device tree for a node compatible with "ibm,edk2-compat-v1". This patch switches to calling secvar_ops->format(), which in the case of OPAL/powernv means opal_secvar_format(), which searches the device tree for a node compatible with "ibm,secvar-backend" and checks its "format" property. These are equivalent, as skiboot creates a node with both "ibm,edk2-compat-v1" and "ibm,secvar-backend" as compatible strings.) Signed-off-by: Russell Currey Signed-off-by: Andrew Donnellan Reviewed-by: Stefan Berger Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/20230210080401.345462-27-ajd@linux.ibm.com --- security/integrity/platform_certs/load_powerpc.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'security/integrity') diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c index dee51606d5f4..b9de70b90826 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/security/integrity/platform_certs/load_powerpc.c @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include "keyring_handler.h" @@ -59,16 +58,22 @@ static int __init load_powerpc_certs(void) void *db = NULL, *dbx = NULL; u64 dbsize = 0, dbxsize = 0; int rc = 0; - struct device_node *node; + ssize_t len; + char buf[32]; if (!secvar_ops) return -ENODEV; - /* The following only applies for the edk2-compat backend. */ - node = of_find_compatible_node(NULL, NULL, "ibm,edk2-compat-v1"); - if (!node) + len = secvar_ops->format(buf, sizeof(buf)); + if (len <= 0) return -ENODEV; + // Check for known secure boot implementations from OPAL or PLPKS + if (strcmp("ibm,edk2-compat-v1", buf) && strcmp("ibm,plpks-sb-v1", buf)) { + pr_err("Unsupported secvar implementation \"%s\", not loading certs\n", buf); + return -ENODEV; + } + /* * Get db, and dbx. They might not exist, so it isn't an error if we * can't get them. @@ -103,8 +108,6 @@ static int __init load_powerpc_certs(void) kfree(dbx); } - of_node_put(node); - return rc; } late_initcall(load_powerpc_certs); -- cgit v1.2.3