[Skiboot] [PATCH v6 07/20] secvar: overhaul secvar struct by removing static sized fields
Eric Richter
erichte at linux.ibm.com
Thu Sep 17 02:21:18 AEST 2020
From: Eric Richter <erichte at linux.vnet.ibm.com>
Originally, the secvar struct was intended to contain all the variable
information seperate from the linked list/etc metadata, so that copying
and serialization/deserialization could be handled by a single memcpy().
This is fragile, potentially compiler dependent, and doesn't account for
endianness. Therefore, this patch removes the static allocation for key, now
allocates a buffer for data, and completely removes the now unnecessary
secvar_node struct.
As a side effect, some of the secvar_util functionality has been tweaked where
it makes sense. Most notably alloc_secvar now takes in an extra argument as it
now has to allocate the key
Signed-off-by: Eric Richter <erichte at linux.ibm.com>
---
libstb/secvar/secvar.h | 24 ++++------
libstb/secvar/secvar_api.c | 68 +++++++++++++--------------
libstb/secvar/secvar_util.c | 93 ++++++++++++++++++-------------------
3 files changed, 88 insertions(+), 97 deletions(-)
diff --git a/libstb/secvar/secvar.h b/libstb/secvar/secvar.h
index b141b705..fe660217 100644
--- a/libstb/secvar/secvar.h
+++ b/libstb/secvar/secvar.h
@@ -16,24 +16,18 @@ enum {
};
-struct secvar_node {
- struct list_node link;
- struct secvar *var;
- uint64_t flags; // Flag for how *var should be stored
- uint64_t size; // How much space was allocated for data
-};
-
#define SECVAR_FLAG_VOLATILE 0x1 // Instructs storage driver to ignore variable on writes
#define SECVAR_FLAG_SECURE_STORAGE 0x2 // Hint for storage driver to select storage location
struct secvar {
+ struct list_node link;
uint64_t key_len;
uint64_t data_size;
- char key[SECVAR_MAX_KEY_LEN];
- char data[0];
+ uint64_t flags;
+ char *key;
+ char *data;
};
-
extern struct list_head variable_bank;
extern struct list_head update_bank;
extern int secvar_enabled;
@@ -44,13 +38,13 @@ extern struct secvar_backend_driver secvar_backend;
// Helper functions
void clear_bank_list(struct list_head *bank);
int copy_bank_list(struct list_head *dst, struct list_head *src);
-struct secvar_node *alloc_secvar(uint64_t size);
-struct secvar_node *new_secvar(const char *key, uint64_t key_len,
+struct secvar *alloc_secvar(uint64_t key_len, uint64_t data_size);
+struct secvar *new_secvar(const char *key, uint64_t key_len,
const char *data, uint64_t data_size,
uint64_t flags);
-int realloc_secvar(struct secvar_node *node, uint64_t size);
-void dealloc_secvar(struct secvar_node *node);
-struct secvar_node *find_secvar(const char *key, uint64_t key_len, struct list_head *bank);
+int realloc_secvar(struct secvar *node, uint64_t size);
+void dealloc_secvar(struct secvar *node);
+struct secvar *find_secvar(const char *key, uint64_t key_len, struct list_head *bank);
int is_key_empty(const char *key, uint64_t key_len);
int list_length(struct list_head *bank);
diff --git a/libstb/secvar/secvar_api.c b/libstb/secvar/secvar_api.c
index 62c32500..7245e859 100644
--- a/libstb/secvar/secvar_api.c
+++ b/libstb/secvar/secvar_api.c
@@ -11,7 +11,7 @@
static int64_t opal_secvar_get(const char *key, uint64_t key_len, void *data, uint64_t *data_size)
{
- struct secvar_node *node;
+ struct secvar *var;
int64_t rc = OPAL_SUCCESS;
if (!secvar_enabled)
@@ -26,18 +26,18 @@ static int64_t opal_secvar_get(const char *key, uint64_t key_len, void *data, ui
if (!data_size)
return OPAL_PARAMETER;
- node = find_secvar(key, key_len, &variable_bank);
- if (!node)
+ var = find_secvar(key, key_len, &variable_bank);
+ if (!var)
return OPAL_EMPTY; // Variable not found, bail early
if (!data)
rc = OPAL_SUCCESS;
- else if (*data_size < node->var->data_size)
+ else if (*data_size < var->data_size)
rc = OPAL_PARTIAL;
else
- memcpy(data, node->var->data, node->var->data_size);
+ memcpy(data, var->data, var->data_size);
- *data_size = node->var->data_size;
+ *data_size = var->data_size;
return rc;
}
@@ -46,7 +46,7 @@ opal_call(OPAL_SECVAR_GET, opal_secvar_get, 4);
static int64_t opal_secvar_get_next(char *key, uint64_t *key_len, uint64_t key_buf_size)
{
- struct secvar_node *node;
+ struct secvar *var;
if (!secvar_enabled)
return OPAL_UNSUPPORTED;
@@ -64,25 +64,25 @@ static int64_t opal_secvar_get_next(char *key, uint64_t *key_len, uint64_t key_b
return OPAL_PARAMETER;
if (!is_key_empty(key, *key_len)) {
- node = find_secvar(key, *key_len, &variable_bank);
- if (!node)
+ var = find_secvar(key, *key_len, &variable_bank);
+ if (!var)
return OPAL_PARAMETER;
- node = list_next(&variable_bank, node, link);
+ var = list_next(&variable_bank, var, link);
} else {
- node = list_top(&variable_bank, struct secvar_node, link);
+ var = list_top(&variable_bank, struct secvar, link);
}
- if (!node)
+ if (!var)
return OPAL_EMPTY;
- if (key_buf_size < node->var->key_len) {
- *key_len = node->var->key_len;
+ if (key_buf_size < var->key_len) {
+ *key_len = var->key_len;
return OPAL_PARTIAL;
}
- *key_len = node->var->key_len;
- memcpy(key, node->var->key, node->var->key_len);
+ *key_len = var->key_len;
+ memcpy(key, var->key, var->key_len);
return OPAL_SUCCESS;
}
@@ -91,7 +91,7 @@ opal_call(OPAL_SECVAR_GET_NEXT, opal_secvar_get_next, 3);
static int64_t opal_secvar_enqueue_update(const char *key, uint64_t key_len, void *data, uint64_t data_size)
{
- struct secvar_node *node;
+ struct secvar *var;
if (!secvar_enabled)
return OPAL_UNSUPPORTED;
@@ -114,41 +114,39 @@ static int64_t opal_secvar_enqueue_update(const char *key, uint64_t key_len, voi
if (is_key_empty(key, key_len))
return OPAL_PARAMETER;
- node = find_secvar(key, key_len, &update_bank);
+ var = find_secvar(key, key_len, &update_bank);
// Unstage an update
if (data_size == 0) {
- if (!node)
+ if (!var)
return OPAL_EMPTY;
- if (node->var)
- free(node->var);
- list_del(&node->link);
- free(node);
+ list_del(&var->link);
+ dealloc_secvar(var);
goto out;
}
- if (node) {
- list_del(&node->link);
+ if (var) {
+ list_del(&var->link);
// Realloc var if too small
- if (node->size < data_size) {
- if (realloc_secvar(node, data_size))
+ if (var->data_size < data_size) {
+ if (realloc_secvar(var, data_size))
return OPAL_NO_MEM;
} else {
- memset(node->var, 0x00, sizeof(struct secvar) + node->var->data_size);
+ memset(var->data, 0x00, var->data_size);
}
} else {
- node = alloc_secvar(data_size);
- if (!node)
+ var = alloc_secvar(key_len, data_size);
+ if (!var)
return OPAL_NO_MEM;
}
- memcpy(node->var->key, key, key_len);
- node->var->key_len = key_len;
- memcpy(node->var->data, data, data_size);
- node->var->data_size = data_size;
+ memcpy(var->key, key, key_len);
+ var->key_len = key_len;
+ memcpy(var->data, data, data_size);
+ var->data_size = data_size;
- list_add_tail(&update_bank, &node->link);
+ list_add_tail(&update_bank, &var->link);
out:
if (secvar_storage.write_bank(&update_bank, SECVAR_UPDATE_BANK))
diff --git a/libstb/secvar/secvar_util.c b/libstb/secvar/secvar_util.c
index 3aadbd98..533170a3 100644
--- a/libstb/secvar/secvar_util.c
+++ b/libstb/secvar/secvar_util.c
@@ -13,33 +13,25 @@
void clear_bank_list(struct list_head *bank)
{
- struct secvar_node *node, *next;
+ struct secvar *var, *next;
if (!bank)
return;
- list_for_each_safe(bank, node, next, link) {
- list_del(&node->link);
- dealloc_secvar(node);
+ list_for_each_safe(bank, var, next, link) {
+ list_del(&var->link);
+ dealloc_secvar(var);
}
}
int copy_bank_list(struct list_head *dst, struct list_head *src)
{
- struct secvar_node *node, *tmp;
+ struct secvar *var, *tmp;
- list_for_each(src, node, link) {
+ list_for_each(src, var, link) {
/* Allocate new secvar using actual data size */
- tmp = alloc_secvar(node->var->data_size);
- if (!tmp)
- return OPAL_NO_MEM;
-
- /* Copy over flags metadata */
- tmp->flags = node->flags;
-
- /* Full-clone over the secvar struct */
- memcpy(tmp->var, node->var, tmp->size + sizeof(struct secvar));
-
+ tmp = new_secvar(var->key, var->key_len, var->data,
+ var->data_size, var->flags);
/* Append to new list */
list_add_tail(dst, &tmp->link);
}
@@ -47,30 +39,38 @@ int copy_bank_list(struct list_head *dst, struct list_head *src)
return OPAL_SUCCESS;
}
-struct secvar_node *alloc_secvar(uint64_t size)
+struct secvar *alloc_secvar(uint64_t key_len, uint64_t data_size)
{
- struct secvar_node *ret;
+ struct secvar *ret;
- ret = zalloc(sizeof(struct secvar_node));
+ ret = zalloc(sizeof(struct secvar));
if (!ret)
return NULL;
- ret->var = zalloc(sizeof(struct secvar) + size);
- if (!ret->var) {
+ ret->key = zalloc(key_len);
+ if (!ret->key) {
+ free(ret->key);
+ return NULL;
+ }
+
+ ret->data = zalloc(data_size);
+ if (!ret->data) {
+ free(ret->key);
free(ret);
return NULL;
}
- ret->size = size;
+ ret->key_len = key_len;
+ ret->data_size = data_size;
return ret;
}
-struct secvar_node *new_secvar(const char *key, uint64_t key_len,
+struct secvar *new_secvar(const char *key, uint64_t key_len,
const char *data, uint64_t data_size,
uint64_t flags)
{
- struct secvar_node *ret;
+ struct secvar *ret;
if (!key)
return NULL;
@@ -79,58 +79,57 @@ struct secvar_node *new_secvar(const char *key, uint64_t key_len,
if ((!data) && (data_size))
return NULL;
- ret = alloc_secvar(data_size);
+ ret = alloc_secvar(key_len, data_size);
if (!ret)
return NULL;
- ret->var->key_len = key_len;
- ret->var->data_size = data_size;
- memcpy(ret->var->key, key, key_len);
+ memcpy(ret->key, key, key_len);
ret->flags = flags;
if (data)
- memcpy(ret->var->data, data, data_size);
+ memcpy(ret->data, data, data_size);
return ret;
}
-int realloc_secvar(struct secvar_node *node, uint64_t size)
+int realloc_secvar(struct secvar *var, uint64_t size)
{
void *tmp;
- if (node->size >= size)
+ if (var->data_size >= size)
return 0;
- tmp = zalloc(sizeof(struct secvar) + size);
+ tmp = zalloc(size);
if (!tmp)
return -1;
- memcpy(tmp, node->var, sizeof(struct secvar) + node->size);
- free(node->var);
- node->var = tmp;
+ memcpy(tmp, var->data, var->data_size);
+ free(var->data);
+ var->data = tmp;
return 0;
}
-void dealloc_secvar(struct secvar_node *node)
+void dealloc_secvar(struct secvar *var)
{
- if (!node)
+ if (!var)
return;
- free(node->var);
- free(node);
+ free(var->key);
+ free(var->data);
+ free(var);
}
-struct secvar_node *find_secvar(const char *key, uint64_t key_len, struct list_head *bank)
+struct secvar *find_secvar(const char *key, uint64_t key_len, struct list_head *bank)
{
- struct secvar_node *node = NULL;
+ struct secvar *var = NULL;
- list_for_each(bank, node, link) {
+ list_for_each(bank, var, link) {
// Prevent matching shorter key subsets / bail early
- if (key_len != node->var->key_len)
+ if (key_len != var->key_len)
continue;
- if (!memcmp(key, node->var->key, key_len))
- return node;
+ if (!memcmp(key, var->key, key_len))
+ return var;
}
return NULL;
@@ -150,9 +149,9 @@ int is_key_empty(const char *key, uint64_t key_len)
int list_length(struct list_head *bank)
{
int ret = 0;
- struct secvar_node *node;
+ struct secvar *var;
- list_for_each(bank, node, link)
+ list_for_each(bank, var, link)
ret++;
return ret;
--
2.21.1
More information about the Skiboot
mailing list