[v5,11/25] powerpc/secvar: Allow backend to populate static list of variable names

Message ID 20230131063928.388035-12-ajd@linux.ibm.com
State New
Headers
Series pSeries dynamic secure boot secvar interface + platform keyring loading |

Commit Message

Andrew Donnellan Jan. 31, 2023, 6:39 a.m. UTC
  Currently, the list of variables is populated by calling
secvar_ops->get_next() repeatedly, which is explicitly modelled on the
OPAL API (including the keylen parameter).

For the upcoming PLPKS backend, we have a static list of variable names.
It is messy to fit that into get_next(), so instead, let the backend put
a NULL-terminated array of variable names into secvar_ops->var_names,
which will be used if get_next() is undefined.

Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Russell Currey <ruscur@russell.cc>

---

v3: New patch (ajd/mpe)
---
 arch/powerpc/include/asm/secvar.h  |  4 ++
 arch/powerpc/kernel/secvar-sysfs.c | 67 ++++++++++++++++++++----------
 2 files changed, 50 insertions(+), 21 deletions(-)
  

Comments

Stefan Berger Jan. 31, 2023, 3:54 p.m. UTC | #1
On 1/31/23 01:39, Andrew Donnellan wrote:
> Currently, the list of variables is populated by calling
> secvar_ops->get_next() repeatedly, which is explicitly modelled on the
> OPAL API (including the keylen parameter).
> 
> For the upcoming PLPKS backend, we have a static list of variable names.
> It is messy to fit that into get_next(), so instead, let the backend put
> a NULL-terminated array of variable names into secvar_ops->var_names,
> which will be used if get_next() is undefined.
> 
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> 
> ---
> 
> v3: New patch (ajd/mpe)
> ---
>   arch/powerpc/include/asm/secvar.h  |  4 ++
>   arch/powerpc/kernel/secvar-sysfs.c | 67 ++++++++++++++++++++----------
>   2 files changed, 50 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
> index 011a53a8076c..4828e0ab7e3c 100644
> --- a/arch/powerpc/include/asm/secvar.h
> +++ b/arch/powerpc/include/asm/secvar.h
> @@ -21,6 +21,10 @@ struct secvar_operations {
>   	ssize_t (*format)(char *buf, size_t bufsize);
>   	int (*max_size)(u64 *max_size);
>   	const struct attribute **config_attrs;
> +
> +	// NULL-terminated array of fixed variable names
> +	// Only used if get_next() isn't provided
> +	const char * const *var_names;
>   };
>   
>   #ifdef CONFIG_PPC_SECURE_BOOT
> diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
> index 7df32be86507..2cbc60b37e4e 100644
> --- a/arch/powerpc/kernel/secvar-sysfs.c
> +++ b/arch/powerpc/kernel/secvar-sysfs.c
> @@ -157,9 +157,31 @@ static int secvar_sysfs_config(struct kobject *kobj)
>   	return 0;
>   }
>   
> -static int secvar_sysfs_load(void)
> +static int add_var(const char *name)
>   {
>   	struct kobject *kobj;
> +	int rc;
> +
> +	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);
> +		return rc;
> +	}
> +
> +	kobject_uevent(kobj, KOBJ_ADD);
> +	return 0;
> +}
> +
> +static int secvar_sysfs_load(void)
> +{
>   	u64 namesize = 0;
>   	char *name;
>   	int rc;
> @@ -179,31 +201,26 @@ static int secvar_sysfs_load(void)
>   			break;
>   		}
>   
> -		kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
> -		if (!kobj) {
> -			rc = -ENOMEM;
> -			break;
> -		}
> -
> -		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);
> -
> +		rc = add_var(name);
>   	} while (!rc);
>   
>   	kfree(name);
>   	return rc;
>   }
>   
> +static int secvar_sysfs_load_static(void)
> +{
> +	const char * const *name_ptr = secvar_ops->var_names;
> +	int rc;

Missing newline ?

> +	while (*name_ptr) {
> +		rc = add_var(*name_ptr);
> +		if (rc)
> +			return rc;
> +		name_ptr++;
> +	}
> +	return 0;
> +}
> +
>   static int secvar_sysfs_init(void)
>   {
>   	int rc;
> @@ -245,7 +262,15 @@ static int secvar_sysfs_init(void)
>   		goto err;
>   	}
>   
> -	secvar_sysfs_load();
> +	if (secvar_ops->get_next)
> +		rc = secvar_sysfs_load();
> +	else
> +		rc = secvar_sysfs_load_static();
> +
> +	if (rc) {
> +		pr_err("Failed to create variable attributes\n");
> +		goto err;
> +	}
>   
>   	return 0;
>   err:

With the above change:

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
  

Patch

diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
index 011a53a8076c..4828e0ab7e3c 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -21,6 +21,10 @@  struct secvar_operations {
 	ssize_t (*format)(char *buf, size_t bufsize);
 	int (*max_size)(u64 *max_size);
 	const struct attribute **config_attrs;
+
+	// NULL-terminated array of fixed variable names
+	// Only used if get_next() isn't provided
+	const char * const *var_names;
 };
 
 #ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
index 7df32be86507..2cbc60b37e4e 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -157,9 +157,31 @@  static int secvar_sysfs_config(struct kobject *kobj)
 	return 0;
 }
 
-static int secvar_sysfs_load(void)
+static int add_var(const char *name)
 {
 	struct kobject *kobj;
+	int rc;
+
+	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);
+		return rc;
+	}
+
+	kobject_uevent(kobj, KOBJ_ADD);
+	return 0;
+}
+
+static int secvar_sysfs_load(void)
+{
 	u64 namesize = 0;
 	char *name;
 	int rc;
@@ -179,31 +201,26 @@  static int secvar_sysfs_load(void)
 			break;
 		}
 
-		kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
-		if (!kobj) {
-			rc = -ENOMEM;
-			break;
-		}
-
-		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);
-
+		rc = add_var(name);
 	} while (!rc);
 
 	kfree(name);
 	return rc;
 }
 
+static int secvar_sysfs_load_static(void)
+{
+	const char * const *name_ptr = secvar_ops->var_names;
+	int rc;
+	while (*name_ptr) {
+		rc = add_var(*name_ptr);
+		if (rc)
+			return rc;
+		name_ptr++;
+	}
+	return 0;
+}
+
 static int secvar_sysfs_init(void)
 {
 	int rc;
@@ -245,7 +262,15 @@  static int secvar_sysfs_init(void)
 		goto err;
 	}
 
-	secvar_sysfs_load();
+	if (secvar_ops->get_next)
+		rc = secvar_sysfs_load();
+	else
+		rc = secvar_sysfs_load_static();
+
+	if (rc) {
+		pr_err("Failed to create variable attributes\n");
+		goto err;
+	}
 
 	return 0;
 err: