[PATCH v2 1/8] state framework enhancements

[PATCH v2 1/8] state framework enhancements

[Marc Kleine-Budde]

Hello,

this series fixes some problems and enhances the state support in barebox.

Marc

changes since v1:
* 1+2: add of_find_path_by_phandle()
* 3: don't open code OF to device path conversion in state_probe use of_find_path_by_phandle() instead


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 1/8] of_path: of_find_path() factor out device detection logic into separate function

[Marc Kleine-Budde]

This patch factors out the device detection logic into separate function, so
that it can be used from another function.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/of/of_path.c | 81 ++++++++++++++++++++++++++++------------------------
 1 file changed, 44 insertions(+), 37 deletions(-)

diff --git a/drivers/of/of_path.c b/drivers/of/of_path.c
index 992972c9b52e..b3ef26c6a50d 100644
--- a/drivers/of/of_path.c
+++ b/drivers/of/of_path.c
@@ -106,6 +106,48 @@ out:
 	return ret;
 }
 
+int __of_find_path(struct device_node *node, const char *propname, char **outpath, unsigned flags)
+{
+	struct of_path op = {};
+	const char *str;
+	bool add_bb = false;
+	int i, ret;
+
+	op.dev = of_find_device_by_node_path(node->full_name);
+	if (!op.dev) {
+		op.dev = of_find_device_by_node_path(node->parent->full_name);
+		if (!op.dev)
+			return -ENODEV;
+	}
+
+	device_detect(op.dev);
+
+	op.cdev = cdev_by_device_node(node);
+
+	i = 1;
+
+	while (propname) {
+		ret = of_property_read_string_index(node, propname, i++, &str);
+		if (ret)
+			break;
+
+		ret = of_path_parse_one(&op, str);
+		if (ret)
+			return ret;
+	}
+
+	if (!op.cdev)
+		return -ENOENT;
+
+	if ((flags & OF_FIND_PATH_FLAGS_BB) && op.cdev->mtd &&
+	    mtd_can_have_bb(op.cdev->mtd))
+		add_bb = true;
+
+	*outpath = asprintf("/dev/%s%s", op.cdev->name, add_bb ? ".bb" : "");
+
+	return 0;
+}
+
 /**
  * of_find_path - translate a path description in the devicetree to a barebox
  *                path
@@ -134,11 +176,8 @@ out:
  */
 int of_find_path(struct device_node *node, const char *propname, char **outpath, unsigned flags)
 {
-	struct of_path op = {};
 	struct device_node *rnode;
-	const char *path, *str;
-	bool add_bb = false;
-	int i, ret;
+	const char *path;
 
 	path = of_get_property(node, propname, NULL);
 	if (!path)
@@ -148,37 +187,5 @@ int of_find_path(struct device_node *node, const char *propname, char **outpath,
 	if (!rnode)
 		return -ENODEV;
 
-	op.dev = of_find_device_by_node_path(rnode->full_name);
-	if (!op.dev) {
-		op.dev = of_find_device_by_node_path(rnode->parent->full_name);
-		if (!op.dev)
-			return -ENODEV;
-	}
-
-	device_detect(op.dev);
-
-	op.cdev = cdev_by_device_node(rnode);
-
-	i = 1;
-
-	while (1) {
-		ret = of_property_read_string_index(node, propname, i++, &str);
-		if (ret)
-			break;
-
-		ret = of_path_parse_one(&op, str);
-		if (ret)
-			return ret;
-	}
-
-	if (!op.cdev)
-		return -ENOENT;
-
-	if ((flags & OF_FIND_PATH_FLAGS_BB) && op.cdev->mtd &&
-	    mtd_can_have_bb(op.cdev->mtd))
-		add_bb = true;
-
-	*outpath = asprintf("/dev/%s%s", op.cdev->name, add_bb ? ".bb" : "");
-
-	return 0;
+	return __of_find_path(rnode, propname, outpath, flags);
 }
-- 
2.6.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 2/8] of_path: add of_find_path_by_phandle()

[Marc Kleine-Budde]

This patch adds the function of_find_path_by_phandle(), which is similar to
of_find_path(), but it takes a phandle not a barebox path description.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/of/of_path.c | 22 ++++++++++++++++++++++
 include/of.h         |  1 +
 2 files changed, 23 insertions(+)

diff --git a/drivers/of/of_path.c b/drivers/of/of_path.c
index b3ef26c6a50d..dae7eb628acb 100644
--- a/drivers/of/of_path.c
+++ b/drivers/of/of_path.c
@@ -149,6 +149,28 @@ int __of_find_path(struct device_node *node, const char *propname, char **outpat
 }
 
 /**
+ * of_find_path_by_phandle - translate a phanle in the devicetree to a
+ *                           barebox device path
+ *
+ * @node: the node containing the phandle pointing to the device tree node
+ * @phandle: the phandle pointing to the devicetree node
+ * @outpath: if this function returns 0 outpath will contain the path belonging
+ *           to the input path description. Must be freed with free().
+ * @flags: use OF_FIND_PATH_FLAGS_BB to return the .bb device if available
+ *
+ */
+int of_find_path_by_phandle(struct device_node *node, const char *phandle, char **outpath, unsigned flags)
+{
+	struct device_node *rnode;
+
+	rnode = of_parse_phandle(node, phandle, 0);
+	if (!rnode)
+		return -EINVAL;
+
+	return __of_find_path(rnode, NULL, outpath, flags);
+}
+
+/**
  * of_find_path - translate a path description in the devicetree to a barebox
  *                path
  *
diff --git a/include/of.h b/include/of.h
index e0ebc39b742d..c1c17e70b389 100644
--- a/include/of.h
+++ b/include/of.h
@@ -246,6 +246,7 @@ void of_add_memory_bank(struct device_node *node, bool dump, int r,
 struct device_d *of_find_device_by_node_path(const char *path);
 #define OF_FIND_PATH_FLAGS_BB 1		/* return .bb device if available */
 int of_find_path(struct device_node *node, const char *propname, char **outpath, unsigned flags);
+int of_find_path_by_phandle(struct device_node *node, const char *phandle, char **outpath, unsigned flags);
 int of_register_fixup(int (*fixup)(struct device_node *, void *), void *context);
 int of_unregister_fixup(int (*fixup)(struct device_node *, void *), void *context);
 struct device_node *of_find_node_by_alias(struct device_node *root,
-- 
2.6.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 3/8] state: make use of of_find_path_by_phandle() and add return -EPROBE_DEFER if device is not available

[Marc Kleine-Budde]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/misc/state.c | 37 ++++++++-----------------------------
 1 file changed, 8 insertions(+), 29 deletions(-)

diff --git a/drivers/misc/state.c b/drivers/misc/state.c
index f3e366480fa2..4cda55b6efc6 100644
--- a/drivers/misc/state.c
+++ b/drivers/misc/state.c
@@ -24,7 +24,6 @@
 static int state_probe(struct device_d *dev)
 {
 	struct device_node *np = dev->device_node;
-	struct device_node *partition_node;
 	struct state *state;
 	const char *alias;
 	const char *backend_type = NULL;
@@ -50,35 +49,15 @@ static int state_probe(struct device_d *dev)
 	}
 
 	/* guess if of_path is a path, not a phandle */
-	if (of_path[0] == '/' && len > 1) {
+	if (of_path[0] == '/' && len > 1)
 		ret = of_find_path(np, "backend", &path, 0);
-		if (ret)
-			goto out_release;
-	} else {
-		struct device_d *dev;
-		struct cdev *cdev;
-
-		partition_node = of_parse_phandle(np, "backend", 0);
-		if (!partition_node) {
-			ret = -ENODEV;
-			goto out_release;
-		}
-
-		dev = of_find_device_by_node(partition_node);
-		if (!list_is_singular(&dev->cdevs)) {
-			ret = -ENODEV;
-			goto out_release;
-		}
-
-		cdev = list_first_entry(&dev->cdevs, struct cdev, devices_list);
-		if (!cdev) {
-			ret = -ENODEV;
-			goto out_release;
-		}
-
-		path = asprintf("/dev/%s", cdev->name);
-		of_path = partition_node->full_name;
-	}
+	else
+		ret = of_find_path_by_phandle(np, "backend", &path, 0);
+
+	if (ret == -ENODEV)
+		ret = -EPROBE_DEFER;
+	if (ret)
+		goto out_release;
 
 	ret = of_property_read_string(np, "backend-type", &backend_type);
 	if (ret) {
-- 
2.6.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 4/8] state: use name of device node as name if alias is not available

[Marc Kleine-Budde]

Wihtout this patch, when using more than one state device an alias is
mandatory, otherwise the there will be two state devices with the name state.

This patch fixes the limitation by using the device node's name if no alias is
defined.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/misc/state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/state.c b/drivers/misc/state.c
index 4cda55b6efc6..e056b28f96c2 100644
--- a/drivers/misc/state.c
+++ b/drivers/misc/state.c
@@ -36,7 +36,7 @@ static int state_probe(struct device_d *dev)
 
 	alias = of_alias_get(np);
 	if (!alias)
-		alias = "state";
+		alias = np->name;
 
 	state = state_new_from_node(alias, np);
 	if (IS_ERR(state))
-- 
2.6.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 5/8] state: disable load command

[Marc Kleine-Budde]

From: Enrico Jorns <ejo@pengutronix.de>

Explicitly loading environment is not required as it will be loaded if
available during device probing

Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 commands/state.c     | 19 +++++--------------
 common/state.c       | 45 ++++++++++++++++++---------------------------
 drivers/misc/state.c |  1 -
 include/state.h      |  1 -
 4 files changed, 23 insertions(+), 43 deletions(-)

diff --git a/commands/state.c b/commands/state.c
index 59d1eb850215..4b51759e3ecd 100644
--- a/commands/state.c
+++ b/commands/state.c
@@ -21,26 +21,20 @@ static int do_state(int argc, char *argv[])
 {
 	int opt, ret = 0;
 	struct state *state = NULL;
-	int do_save = 0, do_load = 0;
+	int do_save = 0;
 	const char *statename = "state";
 
-	while ((opt = getopt(argc, argv, "sl")) > 0) {
+	while ((opt = getopt(argc, argv, "s")) > 0) {
 		switch (opt) {
 		case 's':
 			do_save = 1;
 			break;
-		case 'l':
-			do_load = 1;
-			break;
 		default:
 			return COMMAND_ERROR_USAGE;
 		}
 	}
 
-	if (do_save && do_load)
-		return COMMAND_ERROR_USAGE;
-
-	if (!do_save && !do_load) {
+	if (!do_save) {
 		state_info();
 		return 0;
 	}
@@ -56,8 +50,6 @@ static int do_state(int argc, char *argv[])
 
 	if (do_save)
 		ret = state_save(state);
-	else if (do_load)
-		ret = state_load(state);
 
 	return ret;
 }
@@ -67,13 +59,12 @@ BAREBOX_CMD_HELP_TEXT("Usage: state [OPTIONS] [STATENAME]")
 BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("options:")
 BAREBOX_CMD_HELP_OPT ("-s", "save state")
-BAREBOX_CMD_HELP_OPT ("-l", "load state")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(state)
 	.cmd		= do_state,
-	BAREBOX_CMD_DESC("load and save state information")
-	BAREBOX_CMD_OPTS("[-sl] [STATENAME]")
+	BAREBOX_CMD_DESC("save state information")
+	BAREBOX_CMD_OPTS("[-s] [STATENAME]")
 	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
 	BAREBOX_CMD_HELP(cmd_state_help)
 BAREBOX_CMD_END
diff --git a/common/state.c b/common/state.c
index 117a6867597d..fdb5564af3bd 100644
--- a/common/state.c
+++ b/common/state.c
@@ -51,7 +51,6 @@ struct state {
 };
 
 struct state_backend {
-	int (*load)(struct state_backend *backend, struct state *state);
 	int (*save)(struct state_backend *backend, struct state *state);
 	const char *name;
 	const char *of_path;
@@ -1049,30 +1048,6 @@ int state_get_name(const struct state *state, char const **name)
 }
 
 /*
- * state_load - load a state from the backing store
- *
- * @state	The state instance to load
- */
-int state_load(struct state *state)
-{
-	int ret;
-
-	if (!state->backend)
-		return -ENOSYS;
-
-	ret = state->backend->load(state->backend, state);
-	if (ret) {
-		dev_warn(&state->dev, "load failed\n");
-		state->dirty = 1;
-	} else {
-		dev_info(&state->dev, "load successful\n");
-		state->dirty = 0;
-	}
-
-	return ret;
-}
-
-/*
  * state_save - save a state to the backing store
  *
  * @state	The state instance to save
@@ -1226,7 +1201,6 @@ int state_backend_dtb_file(struct state *state, const char *of_path, const char
 	backend_dtb = xzalloc(sizeof(*backend_dtb));
 	backend = &backend_dtb->backend;
 
-	backend->load = state_backend_dtb_load;
 	backend->save = state_backend_dtb_save;
 	backend->of_path = xstrdup(of_path);
 	backend->path = xstrdup(path);
@@ -1238,6 +1212,15 @@ int state_backend_dtb_file(struct state *state, const char *of_path, const char
 	if (!ret && !(meminfo.flags & MTD_NO_ERASE))
 		backend_dtb->need_erase = true;
 
+	ret = state_backend_dtb_load(backend, state);
+	if (ret) {
+		dev_warn(&state->dev, "load failed - using defaults\n");
+	} else {
+		dev_info(&state->dev, "load successful\n");
+		state->dirty = 0;
+	}
+
+	/* ignore return value of load() */
 	return 0;
 }
 
@@ -1548,7 +1531,6 @@ int state_backend_raw_file(struct state *state, const char *of_path,
 	backend_raw = xzalloc(sizeof(*backend_raw));
 	backend = &backend_raw->backend;
 
-	backend->load = state_backend_raw_load;
 	backend->save = state_backend_raw_save;
 	backend->of_path = xstrdup(of_path);
 	backend->path = xstrdup(path);
@@ -1583,6 +1565,15 @@ int state_backend_raw_file(struct state *state, const char *of_path,
 		goto err;
 	}
 
+	ret = state_backend_raw_load(backend, state);
+	if (ret) {
+		dev_warn(&state->dev, "load failed - using defaults\n");
+	} else {
+		dev_info(&state->dev, "load successful\n");
+		state->dirty = 0;
+	}
+
+	/* ignore return value of load() */
 	return 0;
 err:
 	free(backend_raw);
diff --git a/drivers/misc/state.c b/drivers/misc/state.c
index e056b28f96c2..d519cb7fffdd 100644
--- a/drivers/misc/state.c
+++ b/drivers/misc/state.c
@@ -78,7 +78,6 @@ static int state_probe(struct device_d *dev)
 	dev_info(dev, "backend: %s, path: %s, of_path: %s\n", backend_type, path, of_path);
 	free(path);
 
-	state_load(state);
 	return 0;
 
  out_free:
diff --git a/include/state.h b/include/state.h
index 08c4e8654b9e..b3966fd99e79 100644
--- a/include/state.h
+++ b/include/state.h
@@ -17,7 +17,6 @@ struct state *state_by_name(const char *name);
 struct state *state_by_node(const struct device_node *node);
 int state_get_name(const struct state *state, char const **name);
 
-int state_load(struct state *state);
 int state_save(struct state *state);
 void state_info(void);
 
-- 
2.6.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 6/8] crypto: add simple keystore

[Marc Kleine-Budde]

This patch adds a simple keystore to barebox. The keystore implements a simple
key-value store to hold arbitrary values.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 crypto/Kconfig            |  6 ++++
 crypto/Makefile           |  1 +
 crypto/keystore.c         | 80 +++++++++++++++++++++++++++++++++++++++++++++++
 include/crypto/keystore.h | 26 +++++++++++++++
 4 files changed, 113 insertions(+)
 create mode 100644 crypto/keystore.c
 create mode 100644 include/crypto/keystore.h

diff --git a/crypto/Kconfig b/crypto/Kconfig
index ef807dec68de..10b34912b007 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -84,3 +84,9 @@ config CRYPTO_PBKDF2
 	select DIGEST
 	select DIGEST_SHA1_GENERIC
 	bool
+
+config CRYPTO_KEYSTORE
+	bool "Keystore"
+	help
+	  This is a simple keystore, which can be used to pass keys
+	  between several components via simple interface.
diff --git a/crypto/Makefile b/crypto/Makefile
index f39de718e5e7..c6d17787ca4e 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_DIGEST_SHA384_GENERIC)	+= sha4.o
 obj-$(CONFIG_DIGEST_SHA512_GENERIC)	+= sha4.o
 
 obj-$(CONFIG_CRYPTO_PBKDF2)	+= pbkdf2.o
+obj-$(CONFIG_CRYPTO_KEYSTORE)	+= keystore.o
diff --git a/crypto/keystore.c b/crypto/keystore.c
new file mode 100644
index 000000000000..90b470fe6717
--- /dev/null
+++ b/crypto/keystore.c
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2015 Pengutronix, Marc Kleine-Budde <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation.
+ */
+
+#include <common.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+
+static LIST_HEAD(keystore_list);
+
+#define for_each_key(key) list_for_each_entry(key, &keystore_list, list)
+
+struct keystore_key {
+	struct list_head list;
+	const char *name;
+	const u8 *secret;
+	int secret_len;
+};
+
+static int keystore_compare(struct list_head *a, struct list_head *b)
+{
+	const char *na = list_entry(a, struct keystore_key, list)->name;
+	const char *nb = list_entry(b, struct keystore_key, list)->name;
+
+	return strcmp(na, nb);
+}
+
+/**
+ * @param[in] name Name of the secret to get
+ * @param[out] secret Double pointer to memory representing the secret, do _not_ free() after use
+ * @param[out] secret_len Pointer to length of the secret
+ */
+int keystore_get_secret(const char *name, const u8 **secret, int *secret_len)
+{
+	struct keystore_key *key;
+
+	for_each_key(key) {
+		if (!strcmp(name, key->name)) {
+			if (!secret || !secret_len)
+				return 0;
+
+			*secret = key->secret;
+			*secret_len = key->secret_len;
+
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+/**
+ * @param[in] name Name of the secret to set
+ * @param[in] secret Pointer to memory holding the secret
+ * @param[in] secret_len Length of the secret
+ */
+int keystore_set_secret(const char *name, const u8 *secret, int secret_len)
+{
+	struct keystore_key *key;
+	int ret;
+
+	/* check if key is already in store */
+	ret = keystore_get_secret(name, NULL, NULL);
+	if (!ret)
+		return -EBUSY;
+
+	key = xzalloc(sizeof(*key));
+	INIT_LIST_HEAD(&key->list);
+	key->name = xstrdup(name);
+	key->secret = xmemdup(secret, secret_len);
+	key->secret_len = secret_len;
+
+	list_add_sort(&key->list, &keystore_list, keystore_compare);
+
+	return 0;
+}
diff --git a/include/crypto/keystore.h b/include/crypto/keystore.h
new file mode 100644
index 000000000000..29915854b851
--- /dev/null
+++ b/include/crypto/keystore.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2015 Pengutronix, Marc Kleine-Budde <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation.
+ */
+
+#ifndef __CRYPTO_KEYSTORE_H
+#define __CRYPTO_KEYSTORE_H
+
+#ifdef CONFIG_CRYPTO_KEYSTORE
+int keystore_get_secret(const char *name, const u8 **secret, int *secret_len);
+int keystore_set_secret(const char *name, const u8 *secret, int secret_len);
+#else
+static inline int keystore_get_secret(const char *name, const u8 **secret, int *secret_len)
+{
+	return -EINVAL;
+}
+static inline int keystore_set_secret(const char *name, const u8 *secret, int secret_len)
+{
+	return 0;
+}
+#endif
+
+#endif
-- 
2.6.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 7/8] state: prepare raw backend for hmac support

[Marc Kleine-Budde]

This patch cleans up the raw backend, so that adding hmac support in the next
patch is easier.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 common/state.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/common/state.c b/common/state.c
index fdb5564af3bd..d37f4ab4b539 100644
--- a/common/state.c
+++ b/common/state.c
@@ -1254,7 +1254,7 @@ static int backend_raw_load_one(struct state_backend_raw *backend_raw,
 	struct backend_raw_header header = {};
 	unsigned long max_len;
 	int ret;
-	void *buf;
+	void *buf, *data;
 
 	max_len = backend_raw->stride;
 
@@ -1292,13 +1292,18 @@ static int backend_raw_load_one(struct state_backend_raw *backend_raw,
 		return -EINVAL;
 	}
 
-	buf = xzalloc(header.data_len);
+	buf = xzalloc(sizeof(header) + header.data_len);
+	data = buf + sizeof(header);
+
+	ret = lseek(fd, offset, SEEK_SET);
+	if (ret < 0)
+		goto out_free;
 
-	ret = read_full(fd, buf, header.data_len);
+	ret = read_full(fd, buf, sizeof(header) + header.data_len);
 	if (ret < 0)
 		goto out_free;
 
-	crc = crc32(0, buf, header.data_len);
+	crc = crc32(0, data, header.data_len);
 	if (crc != header.data_crc) {
 		dev_err(&state->dev,
 			"invalid crc, calculated 0x%08x, found 0x%08x\n",
@@ -1310,7 +1315,7 @@ static int backend_raw_load_one(struct state_backend_raw *backend_raw,
 	list_for_each_entry(sv, &state->variables, list) {
 		if (sv->start + sv->size > header.data_len)
 			break;
-		memcpy(sv->raw, buf + sv->start, sv->size);
+		memcpy(sv->raw, data + sv->start, sv->size);
 	}
 
 	free(buf);
-- 
2.6.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 8/8] state: backend_raw: add hamc support

[Marc Kleine-Budde]

This patch adds hmac support to the raw backend.

With this patch, modifications of the header or data of a state partition can
be detected, as the hmac woudln't match anymore. The hmac relies on a shared
secret, which is requested from the keystore, with keystore_get_secret() using
the name of the state partition as the "name" of the secret.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 common/state.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 109 insertions(+), 8 deletions(-)

diff --git a/common/state.c b/common/state.c
index d37f4ab4b539..234b4c01244c 100644
--- a/common/state.c
+++ b/common/state.c
@@ -15,6 +15,7 @@
  */
 
 #include <common.h>
+#include <digest.h>
 #include <environment.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -28,6 +29,8 @@
 #include <state.h>
 #include <xfuncs.h>
 
+#include <crypto/keystore.h>
+
 #include <linux/mtd/mtd-abi.h>
 #include <linux/mtd/mtd.h>
 #include <linux/list.h>
@@ -41,7 +44,7 @@ struct state_backend;
 
 struct state {
 	struct device_d dev;
-	const struct device_node *root;
+	struct device_node *root;
 	struct list_head variables;
 	const char *name;
 	struct list_head list;
@@ -55,6 +58,7 @@ struct state_backend {
 	const char *name;
 	const char *of_path;
 	const char *path;
+	struct digest *digest;
 };
 
 enum state_variable_type {
@@ -948,6 +952,16 @@ static int of_state_fixup(struct device_node *root, void *ctx)
 	if (ret)
 		goto out;
 
+	if (state->backend->digest) {
+		p = of_new_property(new_node, "algo",
+				    digest_name(state->backend->digest),
+				    strlen(digest_name(state->backend->digest)) + 1);
+		if (!p) {
+			ret = -ENOMEM;
+			goto out;
+		}
+	}
+
 	/* address-cells + size-cells */
 	ret = of_property_write_u32(new_node, "#address-cells", 1);
 	if (ret)
@@ -1230,7 +1244,7 @@ int state_backend_dtb_file(struct state *state, const char *of_path, const char
 struct state_backend_raw {
 	struct state_backend backend;
 	unsigned long size_data; /* The raw data size (without header) */
-	unsigned long size_full; /* The size header + raw data */
+	unsigned long size_full; /* The size header + raw data + hmac */
 	unsigned long stride; /* The stride size in bytes of the copies */
 	off_t offset; /* offset in the storage file */
 	size_t size; /* size of the storage area */
@@ -1253,8 +1267,9 @@ static int backend_raw_load_one(struct state_backend_raw *backend_raw,
 	struct state_variable *sv;
 	struct backend_raw_header header = {};
 	unsigned long max_len;
+	int d_len = 0;
 	int ret;
-	void *buf, *data;
+	void *buf, *data, *hmac;
 
 	max_len = backend_raw->stride;
 
@@ -1285,6 +1300,11 @@ static int backend_raw_load_one(struct state_backend_raw *backend_raw,
 		return -EINVAL;
 	}
 
+	if (backend_raw->backend.digest) {
+		d_len = digest_length(backend_raw->backend.digest);
+		max_len -= d_len;
+	}
+
 	if (header.data_len > max_len) {
 		dev_err(&state->dev,
 			"invalid data_len %u in header, max is %lu\n",
@@ -1292,14 +1312,15 @@ static int backend_raw_load_one(struct state_backend_raw *backend_raw,
 		return -EINVAL;
 	}
 
-	buf = xzalloc(sizeof(header) + header.data_len);
+	buf = xzalloc(sizeof(header) + header.data_len + d_len);
 	data = buf + sizeof(header);
+	hmac = data + header.data_len;
 
 	ret = lseek(fd, offset, SEEK_SET);
 	if (ret < 0)
 		goto out_free;
 
-	ret = read_full(fd, buf, sizeof(header) + header.data_len);
+	ret = read_full(fd, buf, sizeof(header) + header.data_len + d_len);
 	if (ret < 0)
 		goto out_free;
 
@@ -1312,6 +1333,23 @@ static int backend_raw_load_one(struct state_backend_raw *backend_raw,
 		goto out_free;
 	}
 
+	if (backend_raw->backend.digest) {
+		struct digest *d = backend_raw->backend.digest;
+
+		ret = digest_init(d);
+		if (ret)
+			goto out_free;
+
+		/* hmac over header and data */
+		ret = digest_update(d, buf, sizeof(header) + header.data_len);
+		if (ret)
+			goto out_free;
+
+		ret = digest_verify(d, hmac);
+		if (ret < 0)
+			goto out_free;
+	}
+
 	list_for_each_entry(sv, &state->variables, list) {
 		if (sv->start + sv->size > header.data_len)
 			break;
@@ -1392,7 +1430,7 @@ static int state_backend_raw_save(struct state_backend *backend,
 	struct state_backend_raw *backend_raw = container_of(backend,
 			struct state_backend_raw, backend);
 	int ret = 0, fd, i;
-	void *buf, *data;
+	void *buf, *data, *hmac;
 	struct backend_raw_header *header;
 	struct state_variable *sv;
 
@@ -1400,6 +1438,7 @@ static int state_backend_raw_save(struct state_backend *backend,
 
 	header = buf;
 	data = buf + sizeof(*header);
+	hmac = data + backend_raw->size_data;
 
 	list_for_each_entry(sv, &state->variables, list)
 		memcpy(data + sv->start, sv->raw, sv->size);
@@ -1410,6 +1449,23 @@ static int state_backend_raw_save(struct state_backend *backend,
 	header->header_crc = crc32(0, header,
 				   sizeof(*header) - sizeof(uint32_t));
 
+	if (backend_raw->backend.digest) {
+		struct digest *d = backend_raw->backend.digest;
+
+		ret = digest_init(d);
+		if (ret)
+			goto out_free;
+
+		/* hmac over header and data */
+		ret = digest_update(d, buf, sizeof(*header) + backend_raw->size_data);
+		if (ret)
+			goto out_free;
+
+		ret = digest_final(d, hmac);
+		if (ret < 0)
+			goto out_free;
+	}
+
 	fd = open(backend->path, O_WRONLY);
 	if (fd < 0)
 		goto out_free;
@@ -1494,6 +1550,43 @@ static int state_backend_raw_file_get_size(const char *path, size_t *out_size)
 	return ret;
 }
 
+static int state_backend_raw_file_init_digest(struct state *state, struct state_backend_raw *backend_raw)
+{
+	struct digest *digest;
+	const char *algo;
+	const unsigned char *key;
+	int key_len, ret;
+
+	ret = of_property_read_string(state->root, "algo", &algo);
+	if (ret == -EINVAL)	/* -EINVAL == does not exist */
+		return 0;
+	else if (ret)
+		return ret;
+
+	ret = keystore_get_secret(state->name, &key, &key_len);
+	if (ret == -ENOENT)	/* -ENOENT == does not exist */
+		return -EPROBE_DEFER;
+	else if (ret)
+		return ret;
+
+	digest = digest_alloc(algo);
+	if (!digest) {
+		dev_info(&state->dev, "algo %s not found - probe deferred\n", algo);
+		return -EPROBE_DEFER;
+	}
+
+	ret = digest_set_key(digest, key, key_len);
+	if (ret) {
+		digest_free(digest);
+		return ret;
+	}
+
+	backend_raw->backend.digest = digest;
+	backend_raw->size_full = digest_length(digest);
+
+	return 0;
+}
+
 /*
  * state_backend_raw_file - create a raw file backend store for a state instance
  *
@@ -1534,8 +1627,14 @@ int state_backend_raw_file(struct state *state, const char *of_path,
 		return -EINVAL;
 
 	backend_raw = xzalloc(sizeof(*backend_raw));
-	backend = &backend_raw->backend;
 
+	ret = state_backend_raw_file_init_digest(state, backend_raw);
+	if (ret) {
+		free(backend_raw);
+		return ret;
+	}
+
+	backend = &backend_raw->backend;
 	backend->save = state_backend_raw_save;
 	backend->of_path = xstrdup(of_path);
 	backend->path = xstrdup(path);
@@ -1545,7 +1644,7 @@ int state_backend_raw_file(struct state *state, const char *of_path,
 	backend_raw->size_data = sv->start + sv->size;
 	backend_raw->offset = offset;
 	backend_raw->size = size;
-	backend_raw->size_full = backend_raw->size_data +
+	backend_raw->size_full += backend_raw->size_data +
 		sizeof(struct backend_raw_header);
 
 	state->backend = backend;
@@ -1581,6 +1680,8 @@ int state_backend_raw_file(struct state *state, const char *of_path,
 	/* ignore return value of load() */
 	return 0;
 err:
+	digest_free(backend_raw->backend.digest);
+
 	free(backend_raw);
 	return ret;
 }
-- 
2.6.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 8/8] state: backend_raw: add hamc support

[Jan Lübbe]

On Di, 2015-10-20 at 10:39 +0200, Marc Kleine-Budde wrote:
> This patch adds hmac support to the raw backend.

hmac is misspelled hamc in the subject.

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 1/8] of_path: of_find_path() factor out device detection logic into separate function

[Sascha Hauer]

On Tue, Oct 20, 2015 at 10:39:05AM +0200, Marc Kleine-Budde wrote:
> This patch factors out the device detection logic into separate function, so
> that it can be used from another function.
> 
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/of/of_path.c | 81 ++++++++++++++++++++++++++++------------------------
>  1 file changed, 44 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/of/of_path.c b/drivers/of/of_path.c
> index 992972c9b52e..b3ef26c6a50d 100644
> --- a/drivers/of/of_path.c
> +++ b/drivers/of/of_path.c
> @@ -106,6 +106,48 @@ out:
>  	return ret;
>  }
>  
> +int __of_find_path(struct device_node *node, const char *propname, char **outpath, unsigned flags)

static please.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 8/8] state: backend_raw: add hamc support

[Sascha Hauer]

On Tue, Oct 20, 2015 at 10:39:12AM +0200, Marc Kleine-Budde wrote:
> This patch adds hmac support to the raw backend.
> 
> With this patch, modifications of the header or data of a state partition can
> be detected, as the hmac woudln't match anymore. The hmac relies on a shared
> secret, which is requested from the keystore, with keystore_get_secret() using
> the name of the state partition as the "name" of the secret.
> 
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  common/state.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 109 insertions(+), 8 deletions(-)
> 
> diff --git a/common/state.c b/common/state.c
> index d37f4ab4b539..234b4c01244c 100644
> --- a/common/state.c
> +++ b/common/state.c
> @@ -15,6 +15,7 @@
>   */
>  
>  #include <common.h>
> +#include <digest.h>
>  #include <environment.h>
>  #include <errno.h>
>  #include <fcntl.h>
> @@ -28,6 +29,8 @@
>  #include <state.h>
>  #include <xfuncs.h>
>  
> +#include <crypto/keystore.h>
> +
>  #include <linux/mtd/mtd-abi.h>
>  #include <linux/mtd/mtd.h>
>  #include <linux/list.h>
> @@ -41,7 +44,7 @@ struct state_backend;
>  
>  struct state {
>  	struct device_d dev;
> -	const struct device_node *root;
> +	struct device_node *root;
>  	struct list_head variables;
>  	const char *name;
>  	struct list_head list;
> @@ -55,6 +58,7 @@ struct state_backend {
>  	const char *name;
>  	const char *of_path;
>  	const char *path;
> +	struct digest *digest;
>  };
>  
>  enum state_variable_type {
> @@ -948,6 +952,16 @@ static int of_state_fixup(struct device_node *root, void *ctx)
>  	if (ret)
>  		goto out;
>  
> +	if (state->backend->digest) {
> +		p = of_new_property(new_node, "algo",
> +				    digest_name(state->backend->digest),
> +				    strlen(digest_name(state->backend->digest)) + 1);
> +		if (!p) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
> +	}
> +
>  	/* address-cells + size-cells */
>  	ret = of_property_write_u32(new_node, "#address-cells", 1);
>  	if (ret)
> @@ -1230,7 +1244,7 @@ int state_backend_dtb_file(struct state *state, const char *of_path, const char
>  struct state_backend_raw {
>  	struct state_backend backend;
>  	unsigned long size_data; /* The raw data size (without header) */
> -	unsigned long size_full; /* The size header + raw data */
> +	unsigned long size_full; /* The size header + raw data + hmac */
>  	unsigned long stride; /* The stride size in bytes of the copies */
>  	off_t offset; /* offset in the storage file */
>  	size_t size; /* size of the storage area */
> @@ -1253,8 +1267,9 @@ static int backend_raw_load_one(struct state_backend_raw *backend_raw,
>  	struct state_variable *sv;
>  	struct backend_raw_header header = {};
>  	unsigned long max_len;
> +	int d_len = 0;
>  	int ret;
> -	void *buf, *data;
> +	void *buf, *data, *hmac;
>  
>  	max_len = backend_raw->stride;
>  
> @@ -1285,6 +1300,11 @@ static int backend_raw_load_one(struct state_backend_raw *backend_raw,
>  		return -EINVAL;
>  	}
>  
> +	if (backend_raw->backend.digest) {
> +		d_len = digest_length(backend_raw->backend.digest);
> +		max_len -= d_len;
> +	}
> +
>  	if (header.data_len > max_len) {
>  		dev_err(&state->dev,
>  			"invalid data_len %u in header, max is %lu\n",
> @@ -1292,14 +1312,15 @@ static int backend_raw_load_one(struct state_backend_raw *backend_raw,
>  		return -EINVAL;
>  	}
>  
> -	buf = xzalloc(sizeof(header) + header.data_len);
> +	buf = xzalloc(sizeof(header) + header.data_len + d_len);
>  	data = buf + sizeof(header);
> +	hmac = data + header.data_len;
>  
>  	ret = lseek(fd, offset, SEEK_SET);
>  	if (ret < 0)
>  		goto out_free;
>  
> -	ret = read_full(fd, buf, sizeof(header) + header.data_len);
> +	ret = read_full(fd, buf, sizeof(header) + header.data_len + d_len);
>  	if (ret < 0)
>  		goto out_free;
>  
> @@ -1312,6 +1333,23 @@ static int backend_raw_load_one(struct state_backend_raw *backend_raw,
>  		goto out_free;
>  	}
>  
> +	if (backend_raw->backend.digest) {
> +		struct digest *d = backend_raw->backend.digest;
> +
> +		ret = digest_init(d);
> +		if (ret)
> +			goto out_free;
> +
> +		/* hmac over header and data */
> +		ret = digest_update(d, buf, sizeof(header) + header.data_len);
> +		if (ret)
> +			goto out_free;
> +
> +		ret = digest_verify(d, hmac);
> +		if (ret < 0)
> +			goto out_free;
> +	}
> +
>  	list_for_each_entry(sv, &state->variables, list) {
>  		if (sv->start + sv->size > header.data_len)
>  			break;
> @@ -1392,7 +1430,7 @@ static int state_backend_raw_save(struct state_backend *backend,
>  	struct state_backend_raw *backend_raw = container_of(backend,
>  			struct state_backend_raw, backend);
>  	int ret = 0, fd, i;
> -	void *buf, *data;
> +	void *buf, *data, *hmac;
>  	struct backend_raw_header *header;
>  	struct state_variable *sv;
>  
> @@ -1400,6 +1438,7 @@ static int state_backend_raw_save(struct state_backend *backend,
>  
>  	header = buf;
>  	data = buf + sizeof(*header);
> +	hmac = data + backend_raw->size_data;
>  
>  	list_for_each_entry(sv, &state->variables, list)
>  		memcpy(data + sv->start, sv->raw, sv->size);
> @@ -1410,6 +1449,23 @@ static int state_backend_raw_save(struct state_backend *backend,
>  	header->header_crc = crc32(0, header,
>  				   sizeof(*header) - sizeof(uint32_t));
>  
> +	if (backend_raw->backend.digest) {
> +		struct digest *d = backend_raw->backend.digest;
> +
> +		ret = digest_init(d);
> +		if (ret)
> +			goto out_free;
> +
> +		/* hmac over header and data */
> +		ret = digest_update(d, buf, sizeof(*header) + backend_raw->size_data);
> +		if (ret)
> +			goto out_free;
> +
> +		ret = digest_final(d, hmac);
> +		if (ret < 0)
> +			goto out_free;
> +	}
> +
>  	fd = open(backend->path, O_WRONLY);
>  	if (fd < 0)
>  		goto out_free;
> @@ -1494,6 +1550,43 @@ static int state_backend_raw_file_get_size(const char *path, size_t *out_size)
>  	return ret;
>  }
>  
> +static int state_backend_raw_file_init_digest(struct state *state, struct state_backend_raw *backend_raw)
> +{
> +	struct digest *digest;
> +	const char *algo;
> +	const unsigned char *key;
> +	int key_len, ret;
> +
> +	ret = of_property_read_string(state->root, "algo", &algo);

This needs an update to Documentation/devicetree/bindings/barebox/barebox,state.rst

> +	if (ret == -EINVAL)	/* -EINVAL == does not exist */
> +		return 0;

-EINVAL is such a widespread error value. Maybe better explicitly test
for existence with of_find_property?

> +	else if (ret)
> +		return ret;
> +
> +	ret = keystore_get_secret(state->name, &key, &key_len);
> +	if (ret == -ENOENT)	/* -ENOENT == does not exist */
> +		return -EPROBE_DEFER;
> +	else if (ret)
> +		return ret;
> +
> +	digest = digest_alloc(algo);
> +	if (!digest) {
> +		dev_info(&state->dev, "algo %s not found - probe deferred\n", algo);
> +		return -EPROBE_DEFER;
> +	}
> +
> +	ret = digest_set_key(digest, key, key_len);
> +	if (ret) {
> +		digest_free(digest);
> +		return ret;
> +	}
> +
> +	backend_raw->backend.digest = digest;
> +	backend_raw->size_full = digest_length(digest);
> +
> +	return 0;
> +}
> +
>  /*
>   * state_backend_raw_file - create a raw file backend store for a state instance
>   *
> @@ -1534,8 +1627,14 @@ int state_backend_raw_file(struct state *state, const char *of_path,
>  		return -EINVAL;
>  
>  	backend_raw = xzalloc(sizeof(*backend_raw));
> -	backend = &backend_raw->backend;
>  
> +	ret = state_backend_raw_file_init_digest(state, backend_raw);
> +	if (ret) {
> +		free(backend_raw);
> +		return ret;
> +	}

Maybe better make this configurable with correct dependencies
(CONFIG_CRYPTO_KEYSTORE, CONFIG_DIGEST) rather than depending on the
user selecting the implicit dependencies manually?

Sascha

> +
> +	backend = &backend_raw->backend;
>  	backend->save = state_backend_raw_save;
>  	backend->of_path = xstrdup(of_path);
>  	backend->path = xstrdup(path);
> @@ -1545,7 +1644,7 @@ int state_backend_raw_file(struct state *state, const char *of_path,
>  	backend_raw->size_data = sv->start + sv->size;
>  	backend_raw->offset = offset;
>  	backend_raw->size = size;
> -	backend_raw->size_full = backend_raw->size_data +
> +	backend_raw->size_full += backend_raw->size_data +
>  		sizeof(struct backend_raw_header);
>  
>  	state->backend = backend;
> @@ -1581,6 +1680,8 @@ int state_backend_raw_file(struct state *state, const char *of_path,
>  	/* ignore return value of load() */
>  	return 0;
>  err:
> +	digest_free(backend_raw->backend.digest);
> +
>  	free(backend_raw);
>  	return ret;
>  }
> -- 
> 2.6.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 8/8] state: backend_raw: add hamc support

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 2341 bytes --]

On 10/21/2015 09:13 AM, Sascha Hauer wrote:
>> +static int state_backend_raw_file_init_digest(struct state *state, struct state_backend_raw *backend_raw)
>> +{
>> +	struct digest *digest;
>> +	const char *algo;
>> +	const unsigned char *key;
>> +	int key_len, ret;
>> +
>> +	ret = of_property_read_string(state->root, "algo", &algo);
> 
> This needs an update to Documentation/devicetree/bindings/barebox/barebox,state.rst

ok

>> +	if (ret == -EINVAL)	/* -EINVAL == does not exist */
>> +		return 0;
> 
> -EINVAL is such a widespread error value. Maybe better explicitly test
> for existence with of_find_property?

ok

>> +	else if (ret)
>> +		return ret;
>> +
>> +	ret = keystore_get_secret(state->name, &key, &key_len);
>> +	if (ret == -ENOENT)	/* -ENOENT == does not exist */
>> +		return -EPROBE_DEFER;
>> +	else if (ret)
>> +		return ret;
>> +
>> +	digest = digest_alloc(algo);
>> +	if (!digest) {
>> +		dev_info(&state->dev, "algo %s not found - probe deferred\n", algo);
>> +		return -EPROBE_DEFER;
>> +	}
>> +
>> +	ret = digest_set_key(digest, key, key_len);
>> +	if (ret) {
>> +		digest_free(digest);
>> +		return ret;
>> +	}
>> +
>> +	backend_raw->backend.digest = digest;
>> +	backend_raw->size_full = digest_length(digest);
>> +
>> +	return 0;
>> +}
>> +
>>  /*
>>   * state_backend_raw_file - create a raw file backend store for a state instance
>>   *
>> @@ -1534,8 +1627,14 @@ int state_backend_raw_file(struct state *state, const char *of_path,
>>  		return -EINVAL;
>>  
>>  	backend_raw = xzalloc(sizeof(*backend_raw));
>> -	backend = &backend_raw->backend;
>>  
>> +	ret = state_backend_raw_file_init_digest(state, backend_raw);
>> +	if (ret) {
>> +		free(backend_raw);
>> +		return ret;
>> +	}
> 
> Maybe better make this configurable with correct dependencies
> (CONFIG_CRYPTO_KEYSTORE, CONFIG_DIGEST) rather than depending on the
> user selecting the implicit dependencies manually?

There are noops so that it compile even if KEYSTORE and DIGEST is not
selected.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 8/8] state: backend_raw: add hamc support

[Sascha Hauer]

On Wed, Oct 21, 2015 at 10:56:01AM +0200, Marc Kleine-Budde wrote:
> On 10/21/2015 09:13 AM, Sascha Hauer wrote:
> >> +static int state_backend_raw_file_init_digest(struct state *state, struct state_backend_raw *backend_raw)
> >> +{
> >> +	struct digest *digest;
> >> +	const char *algo;
> >> +	const unsigned char *key;
> >> +	int key_len, ret;
> >> +
> >> +	ret = of_property_read_string(state->root, "algo", &algo);
> > 
> > This needs an update to Documentation/devicetree/bindings/barebox/barebox,state.rst
> 
> ok
> 
> >> +	if (ret == -EINVAL)	/* -EINVAL == does not exist */
> >> +		return 0;
> > 
> > -EINVAL is such a widespread error value. Maybe better explicitly test
> > for existence with of_find_property?
> 
> ok
> 
> >> +	else if (ret)
> >> +		return ret;
> >> +
> >> +	ret = keystore_get_secret(state->name, &key, &key_len);
> >> +	if (ret == -ENOENT)	/* -ENOENT == does not exist */
> >> +		return -EPROBE_DEFER;
> >> +	else if (ret)
> >> +		return ret;
> >> +
> >> +	digest = digest_alloc(algo);
> >> +	if (!digest) {
> >> +		dev_info(&state->dev, "algo %s not found - probe deferred\n", algo);
> >> +		return -EPROBE_DEFER;
> >> +	}
> >> +
> >> +	ret = digest_set_key(digest, key, key_len);
> >> +	if (ret) {
> >> +		digest_free(digest);
> >> +		return ret;
> >> +	}
> >> +
> >> +	backend_raw->backend.digest = digest;
> >> +	backend_raw->size_full = digest_length(digest);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >>  /*
> >>   * state_backend_raw_file - create a raw file backend store for a state instance
> >>   *
> >> @@ -1534,8 +1627,14 @@ int state_backend_raw_file(struct state *state, const char *of_path,
> >>  		return -EINVAL;
> >>  
> >>  	backend_raw = xzalloc(sizeof(*backend_raw));
> >> -	backend = &backend_raw->backend;
> >>  
> >> +	ret = state_backend_raw_file_init_digest(state, backend_raw);
> >> +	if (ret) {
> >> +		free(backend_raw);
> >> +		return ret;
> >> +	}
> > 
> > Maybe better make this configurable with correct dependencies
> > (CONFIG_CRYPTO_KEYSTORE, CONFIG_DIGEST) rather than depending on the
> > user selecting the implicit dependencies manually?
> 
> There are noops so that it compile even if KEYSTORE and DIGEST is not
> selected.

I know, but letting the probe defer indefinitely is not a nice way to
tell the user that he misconfigured barebox. Also it's easy to add
something to the HMAC state code that doesn't have noops in the next
step without realizing it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 1/8] of_path: of_find_path() factor out device detection logic into separate function

[Trent Piepho]

On Tue, 2015-10-20 at 10:39 +0200, Marc Kleine-Budde wrote:
> This patch factors out the device detection logic into separate function, so
> that it can be used from another function.

The use of device-path = &mmc0, "partname:0"; breaks because it tries to
get the "partname:0" string by looking at "device-path" in &mmc0.  It
should be looking for device-path in the original node.
 
> +int __of_find_path(struct device_node *node, const char *propname, char **outpath, unsigned flags)
> +{

> +	while (propname) {
> +		ret = of_property_read_string_index(node, propname, i++, &str);

Here 'node' is the device node pointed to by propname in the node
originally passed to of_find_path().  I.e., &mmc0.


>  int of_find_path(struct device_node *node, const char *propname, char **outpath, unsigned flags)
>  {
> -
> -	while (1) {
> -		ret = of_property_read_string_index(node, propname, i++, &str);
> -		if (ret)

But here 'node' is the node that is passed to of_find_path.

> +	return __of_find_path(rnode, propname, outpath, flags);

Because here you pass rnode, not node, to __of_find_path() and then use
it where the original code used node.
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH] of_path: Fix bug with partitions, simply code

[Trent Piepho]

In commit f72b64815619b41e026dd3eb9d53bf9855f84628 using a path that
has a partition description broke.

Fix this and simplify the code some.

There is no need to loop over each string in the path property:  it's
defined to have at most one partition description, no extant dts has
more than one, and how it would handle more than one didn't make sense
anyway.

Once not looping, __of_find_path() just needs the partition
description text, not both the original node and property name to look
it up from.

When using a partition description, don't lookup the cdev of the node
just to replace it with the partition's cdev.

Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
---
 drivers/of/of_path.c | 47 +++++++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/of/of_path.c b/drivers/of/of_path.c
index 6903905..9016147 100644
--- a/drivers/of/of_path.c
+++ b/drivers/of/of_path.c
@@ -106,12 +106,22 @@ out:
 	return ret;
 }
 
-static int __of_find_path(struct device_node *node, const char *propname, char **outpath, unsigned flags)
+/**
+ * __of_find_path
+ *
+ * @node: The node to find the cdev for, can be the device or a
+ *        partition in the device
+ * @part: Optionally, a description of a parition of @node.  See of_find_path
+ * @outpath: if this function returns 0 outpath will contain the path belonging
+ *           to the input path description. Must be freed with free().
+ * @flags: use OF_FIND_PATH_FLAGS_BB to return the .bb device if available
+ *
+ */
+static int __of_find_path(struct device_node *node, const char *part, char **outpath, unsigned flags)
 {
-	struct of_path op = {};
-	const char *str;
+	struct of_path op;
 	bool add_bb = false;
-	int i, ret;
+	int ret;
 
 	op.dev = of_find_device_by_node_path(node->full_name);
 	if (!op.dev) {
@@ -122,23 +132,18 @@ static int __of_find_path(struct device_node *node, const char *propname, char *
 
 	device_detect(op.dev);
 
-	op.cdev = cdev_by_device_node(node);
-
-	i = 1;
-
-	while (propname) {
-		ret = of_property_read_string_index(node, propname, i++, &str);
-		if (ret)
-			break;
-
-		ret = of_path_parse_one(&op, str);
+	if (part) {
+		/* Find a partition inside op.dev */
+		ret = of_path_parse_one(&op, part);
 		if (ret)
 			return ret;
+	} else {
+		/* node points directly to device */
+		op.cdev = cdev_by_device_node(node);
+		if (!op.cdev)
+			return -ENOENT;
 	}
 
-	if (!op.cdev)
-		return -ENOENT;
-
 	if ((flags & OF_FIND_PATH_FLAGS_BB) && op.cdev->mtd &&
 	    mtd_can_have_bb(op.cdev->mtd))
 		add_bb = true;
@@ -193,6 +198,8 @@ int of_find_path(struct device_node *node, const char *propname, char **outpath,
 {
 	struct device_node *rnode;
 	const char *path;
+	const char *part;
+	int ret;
 
 	path = of_get_property(node, propname, NULL);
 	if (!path)
@@ -202,5 +209,9 @@ int of_find_path(struct device_node *node, const char *propname, char **outpath,
 	if (!rnode)
 		return -ENODEV;
 
-	return __of_find_path(rnode, propname, outpath, flags);
+	ret = of_property_read_string_index(node, propname, 1, &part);
+	if (ret)
+		part = NULL;
+
+	return __of_find_path(rnode, part, outpath, flags);
 }
-- 
1.8.3.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] of_path: Fix bug with partitions, simply code

[Sascha Hauer]

On Fri, Dec 11, 2015 at 12:07:34AM +0000, Trent Piepho wrote:
> In commit f72b64815619b41e026dd3eb9d53bf9855f84628 using a path that
> has a partition description broke.
> 
> Fix this and simplify the code some.

Could you please separate the fix from the cleanup?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] of_path: Fix bug with partitions, simply code

[Trent Piepho]

On Fri, 2015-12-11 at 10:35 +0100, Sascha Hauer wrote:
> On Fri, Dec 11, 2015 at 12:07:34AM +0000, Trent Piepho wrote:
> > In commit f72b64815619b41e026dd3eb9d53bf9855f84628 using a path that
> > has a partition description broke.
> > 
> > Fix this and simplify the code some.
> 
> Could you please separate the fix from the cleanup?

The fix would be to add an unnecessary extra parameter to
__of_find_path() that just gets deleted in the cleanup.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] of_path: Fix bug with partitions, simply code

[Sascha Hauer]

On Fri, Dec 11, 2015 at 12:07:34AM +0000, Trent Piepho wrote:
> In commit f72b64815619b41e026dd3eb9d53bf9855f84628 using a path that
> has a partition description broke.

Strange, I don't have this commit. Where is it, could you add the
Subject to this commit?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox