Re: [PATCH v6 1/4] powerpc/powernv: Add OPAL API interface to access secure variable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/5/2019 12:24 AM, Eric Richter wrote:

From: Nayna Jain <nayna@xxxxxxxxxxxxx>

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.

I feel splitting this patch into smaller set of changes would make it easier to review. For instance roughly as below:

 1, opal-api.h which adds the #defines  OPAL_SECVAR_ and the API signature.
 2, secvar.h then adds secvar_operations struct
 3, powerpc/kernel for the Interface definitions
 4, powernv/opal-secvar.c for the API implementations
 5, powernv/opal-call.c for the API calls
 6, powernv/opal.c for the secvar init calls.

diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index 378e3997845a..c1f25a760eb1 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -211,7 +211,10 @@
  #define OPAL_MPIPL_UPDATE			173
  #define OPAL_MPIPL_REGISTER_TAG			174
  #define OPAL_MPIPL_QUERY_TAG			175
-#define OPAL_LAST				175
+#define OPAL_SECVAR_GET				176
+#define OPAL_SECVAR_GET_NEXT			177
+#define OPAL_SECVAR_ENQUEUE_UPDATE		178
+#define OPAL_LAST				178

Please fix the indentation for the #defines


+static int opal_get_variable(const char *key, uint64_t ksize,
+			     u8 *data, uint64_t *dsize)
+{
+	int rc;
+
+	if (!key || !dsize)
+		return -EINVAL;
+
+	*dsize = cpu_to_be64(*dsize);
+
+	rc = opal_secvar_get(key, ksize, data, dsize);
+
+	*dsize = be64_to_cpu(*dsize);

Should the return status (rc) from opal_secvar_get be checked before attempting to do the conversion (be64_to_cpu)?

+static int opal_get_next_variable(const char *key, uint64_t *keylen,
+				  uint64_t keybufsize)
+{
+	int rc;
+
+	if (!key || !keylen)
+		return -EINVAL;
+
+	*keylen = cpu_to_be64(*keylen);
+
+	rc = opal_secvar_get_next(key, keylen, keybufsize);
+
+	*keylen = be64_to_cpu(*keylen);

Same comment as above - should rc be checke before attempting to convert?

+
+	return opal_status_to_err(rc);
+}
+
+static int opal_set_variable(const char *key, uint64_t ksize, u8 *data,
+			     uint64_t dsize)
+{
+	int rc;
+
+	if (!key || !data)
+		return -EINVAL;

Is the key and data received here from a trusted caller? If not, should there be some validation checks done here before enqueuing the data?

 -lakshmi




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux