[PATCH 0/3] FIT Support

[PATCH 0/3] FIT Support

[Marc Kleine-Budde]

Hello,

taking over Jan's work. This is the current state of the FIT support patches,
which are used in production on a custom mx6 board. The FIT loading code has
basically been rewritten from scratch, as the original U-Boot code uses libfdt
and barebox's DT support works on an in-memory tree.

These patches apply on origin/next.
The two patches from Sascha are required for the basic RSA support.
The third patch adds FIT support.

regards,
Marc


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

[PATCH 1/3] crypto: add enum

[Marc Kleine-Budde]

From: Sascha Hauer <s.hauer@pengutronix.de>

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 arch/arm/crypto/sha1_glue.c   |  1 +
 arch/arm/crypto/sha256_glue.c |  2 ++
 crypto/digest.c               | 43 ++++++++++++++++++++++++++++++++++++++++++-
 crypto/md5.c                  |  1 +
 crypto/sha1.c                 |  1 +
 crypto/sha2.c                 |  2 ++
 crypto/sha4.c                 |  2 ++
 include/digest.h              | 23 +++++++++++++++++++++++
 8 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/arch/arm/crypto/sha1_glue.c b/arch/arm/crypto/sha1_glue.c
index 176aa9ec69e0..57cd9d101458 100644
--- a/arch/arm/crypto/sha1_glue.c
+++ b/arch/arm/crypto/sha1_glue.c
@@ -119,6 +119,7 @@ static struct digest_algo m = {
 		.name		=	"sha1",
 		.driver_name	=	"sha1-asm",
 		.priority	=	150,
+		.algo		=	HASH_ALGO_SHA1,
 	},
 
 	.init	=	sha1_init,
diff --git a/arch/arm/crypto/sha256_glue.c b/arch/arm/crypto/sha256_glue.c
index f8086f6ac7f7..e649609a8eef 100644
--- a/arch/arm/crypto/sha256_glue.c
+++ b/arch/arm/crypto/sha256_glue.c
@@ -173,6 +173,7 @@ static struct digest_algo sha224 = {
 		.name		=	"sha224",
 		.driver_name 	=	"sha224-asm",
 		.priority	=	150,
+		.algo		=	HASH_ALGO_SHA224,
 	},
 
 	.length	=	SHA224_DIGEST_SIZE,
@@ -195,6 +196,7 @@ static struct digest_algo sha256 = {
 		.name		=	"sha256",
 		.driver_name 	=	"sha256-asm",
 		.priority	=	150,
+		.algo		=	HASH_ALGO_SHA256,
 	},
 
 	.length	=	SHA256_DIGEST_SIZE,
diff --git a/crypto/digest.c b/crypto/digest.c
index a90e4ff79f89..46600f246ece 100644
--- a/crypto/digest.c
+++ b/crypto/digest.c
@@ -116,7 +116,27 @@ static struct digest_algo *digest_algo_get_by_name(const char *name)
 	list_for_each_entry(tmp, &digests, list) {
 		if (strcmp(tmp->base.name, name) != 0)
 			continue;
-		
+
+		if (tmp->base.priority <= priority)
+			continue;
+
+		d = tmp;
+		priority = tmp->base.priority;
+	}
+
+	return d;
+}
+
+static struct digest_algo *digest_algo_get_by_algo(enum hash_algo algo)
+{
+	struct digest_algo *d = NULL;
+	struct digest_algo *tmp;
+	int priority = -1;
+
+	list_for_each_entry(tmp, &digests, list) {
+		if (tmp->base.algo != algo)
+			continue;
+
 		if (tmp->base.priority <= priority)
 			continue;
 
@@ -160,6 +180,27 @@ struct digest *digest_alloc(const char *name)
 }
 EXPORT_SYMBOL_GPL(digest_alloc);
 
+struct digest *digest_alloc_by_algo(enum hash_algo hash_algo)
+{
+	struct digest *d;
+	struct digest_algo *algo;
+
+	algo = digest_algo_get_by_algo(hash_algo);
+	if (!algo)
+		return NULL;
+
+	d = xzalloc(sizeof(*d));
+	d->algo = algo;
+	d->ctx = xzalloc(algo->ctx_length);
+	if (d->algo->alloc(d)) {
+		digest_free(d);
+		return NULL;
+	}
+
+	return d;
+}
+EXPORT_SYMBOL_GPL(digest_alloc_by_algo);
+
 void digest_free(struct digest *d)
 {
 	if (!d)
diff --git a/crypto/md5.c b/crypto/md5.c
index 23892babce6c..f8f52bf4a505 100644
--- a/crypto/md5.c
+++ b/crypto/md5.c
@@ -293,6 +293,7 @@ static struct digest_algo md5 = {
 		.name		= "md5",
 		.driver_name	= "md5-generic",
 		.priority	= 0,
+		.algo		= HASH_ALGO_MD5,
 	},
 	.init = digest_md5_init,
 	.update = digest_md5_update,
diff --git a/crypto/sha1.c b/crypto/sha1.c
index a3de2719d8fc..cbde4d28e475 100644
--- a/crypto/sha1.c
+++ b/crypto/sha1.c
@@ -287,6 +287,7 @@ static struct digest_algo m = {
 		.name		=	"sha1",
 		.driver_name	=	"sha1-generic",
 		.priority	=	0,
+		.algo		=	HASH_ALGO_SHA1,
 	},
 
 	.init		= sha1_init,
diff --git a/crypto/sha2.c b/crypto/sha2.c
index 6ac552702807..df566c8a4b3b 100644
--- a/crypto/sha2.c
+++ b/crypto/sha2.c
@@ -327,6 +327,7 @@ static struct digest_algo m224 = {
 		.name		=	"sha224",
 		.driver_name	=	"sha224-generic",
 		.priority	=	0,
+		.algo		=	HASH_ALGO_SHA224,
 	},
 
 	.init		= sha224_init,
@@ -352,6 +353,7 @@ static struct digest_algo m256 = {
 		.name		=	"sha256",
 		.driver_name	=	"sha256-generic",
 		.priority	=	0,
+		.algo		=	HASH_ALGO_SHA256,
 	},
 
 	.init		= sha256_init,
diff --git a/crypto/sha4.c b/crypto/sha4.c
index 187a91ef584d..4ce37b73e4ae 100644
--- a/crypto/sha4.c
+++ b/crypto/sha4.c
@@ -246,6 +246,7 @@ static struct digest_algo m384 = {
 		.name		=	"sha384",
 		.driver_name	=	"sha384-generic",
 		.priority	=	0,
+		.algo		=	HASH_ALGO_SHA384,
 	},
 
 	.init		= sha384_init,
@@ -272,6 +273,7 @@ static struct digest_algo m512 = {
 		.name		=	"sha512",
 		.driver_name	=	"sha512-generic",
 		.priority	=	0,
+		.algo		=	HASH_ALGO_SHA512,
 	},
 
 	.init		= sha512_init,
diff --git a/include/digest.h b/include/digest.h
index 3a9d305963ef..fe30cc27e0fc 100644
--- a/include/digest.h
+++ b/include/digest.h
@@ -23,12 +23,34 @@
 
 struct digest;
 
+enum hash_algo {
+	HASH_ALGO_MD4,
+	HASH_ALGO_MD5,
+	HASH_ALGO_SHA1,
+	HASH_ALGO_RIPE_MD_160,
+	HASH_ALGO_SHA224,
+	HASH_ALGO_SHA256,
+	HASH_ALGO_SHA384,
+	HASH_ALGO_SHA512,
+	HASH_ALGO_RIPE_MD_128,
+	HASH_ALGO_RIPE_MD_256,
+	HASH_ALGO_RIPE_MD_320,
+	HASH_ALGO_WP_256,
+	HASH_ALGO_WP_384,
+	HASH_ALGO_WP_512,
+	HASH_ALGO_TGR_128,
+	HASH_ALGO_TGR_160,
+	HASH_ALGO_TGR_192,
+	HASH_ALGO__LAST
+};
+
 struct crypto_alg {
 	char *name;
 	char *driver_name;
 	int priority;
 #define DIGEST_ALGO_NEED_KEY	(1 << 0)
 	unsigned int flags;
+	enum hash_algo algo;
 };
 
 struct digest_algo {
@@ -65,6 +87,7 @@ void digest_algo_unregister(struct digest_algo *d);
 void digest_algo_prints(const char *prefix);
 
 struct digest *digest_alloc(const char *name);
+struct digest *digest_alloc_by_algo(enum hash_algo);
 void digest_free(struct digest *d);
 
 int digest_file_window(struct digest *d, const char *filename,
-- 
2.6.2


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

[PATCH 2/3] crypto: add RSA support

[Marc Kleine-Budde]

From: Sascha Hauer <s.hauer@pengutronix.de>

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 crypto/Kconfig              |   3 +
 crypto/Makefile             |   1 +
 crypto/rsa.c                | 420 ++++++++++++++++++++++++++++++++++++++++++++
 include/asm-generic/errno.h |   5 +
 include/rsa.h               |  54 ++++++
 5 files changed, 483 insertions(+)
 create mode 100644 crypto/rsa.c
 create mode 100644 include/rsa.h

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 41145a312d8e..ca6f03a1edd1 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -87,6 +87,9 @@ config CRYPTO_PBKDF2
 	select DIGEST_SHA1_GENERIC
 	bool
 
+config CRYPTO_RSA
+	bool
+
 config CRYPTO_KEYSTORE
 	bool "Keystore"
 	help
diff --git a/crypto/Makefile b/crypto/Makefile
index c6d17787ca4e..b4fb3d8da01f 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -11,4 +11,5 @@ obj-$(CONFIG_DIGEST_SHA384_GENERIC)	+= sha4.o
 obj-$(CONFIG_DIGEST_SHA512_GENERIC)	+= sha4.o
 
 obj-$(CONFIG_CRYPTO_PBKDF2)	+= pbkdf2.o
+obj-$(CONFIG_CRYPTO_RSA)	+= rsa.o
 obj-$(CONFIG_CRYPTO_KEYSTORE)	+= keystore.o
diff --git a/crypto/rsa.c b/crypto/rsa.c
new file mode 100644
index 000000000000..3caee3ce7525
--- /dev/null
+++ b/crypto/rsa.c
@@ -0,0 +1,420 @@
+/*
+ * Copyright (c) 2013, Google Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <malloc.h>
+#include <of.h>
+#include <digest.h>
+#include <asm/types.h>
+#include <asm/byteorder.h>
+#include <errno.h>
+#include <rsa.h>
+#include <asm/types.h>
+#include <asm/unaligned.h>
+
+#define UINT64_MULT32(v, multby)  (((uint64_t)(v)) * ((uint32_t)(multby)))
+
+#define get_unaligned_be32(a) fdt32_to_cpu(*(uint32_t *)a)
+#define put_unaligned_be32(a, b) (*(uint32_t *)(b) = cpu_to_fdt32(a))
+
+/* Default public exponent for backward compatibility */
+#define RSA_DEFAULT_PUBEXP	65537
+
+/* This is the minimum/maximum key size we support, in bits */
+#define RSA_MIN_KEY_BITS	1024
+#define RSA_MAX_KEY_BITS	4096
+
+/**
+ * subtract_modulus() - subtract modulus from the given value
+ *
+ * @key:	Key containing modulus to subtract
+ * @num:	Number to subtract modulus from, as little endian word array
+ */
+static void subtract_modulus(const struct rsa_public_key *key, uint32_t num[])
+{
+	int64_t acc = 0;
+	uint i;
+
+	for (i = 0; i < key->len; i++) {
+		acc += (uint64_t)num[i] - key->modulus[i];
+		num[i] = (uint32_t)acc;
+		acc >>= 32;
+	}
+}
+
+/**
+ * greater_equal_modulus() - check if a value is >= modulus
+ *
+ * @key:	Key containing modulus to check
+ * @num:	Number to check against modulus, as little endian word array
+ * @return 0 if num < modulus, 1 if num >= modulus
+ */
+static int greater_equal_modulus(const struct rsa_public_key *key,
+				 uint32_t num[])
+{
+	int i;
+
+	for (i = (int)key->len - 1; i >= 0; i--) {
+		if (num[i] < key->modulus[i])
+			return 0;
+		if (num[i] > key->modulus[i])
+			return 1;
+	}
+
+	return 1;  /* equal */
+}
+
+/**
+ * montgomery_mul_add_step() - Perform montgomery multiply-add step
+ *
+ * Operation: montgomery result[] += a * b[] / n0inv % modulus
+ *
+ * @key:	RSA key
+ * @result:	Place to put result, as little endian word array
+ * @a:		Multiplier
+ * @b:		Multiplicand, as little endian word array
+ */
+static void montgomery_mul_add_step(const struct rsa_public_key *key,
+		uint32_t result[], const uint32_t a, const uint32_t b[])
+{
+	uint64_t acc_a, acc_b;
+	uint32_t d0;
+	uint i;
+
+	acc_a = (uint64_t)a * b[0] + result[0];
+	d0 = (uint32_t)acc_a * key->n0inv;
+	acc_b = (uint64_t)d0 * key->modulus[0] + (uint32_t)acc_a;
+	for (i = 1; i < key->len; i++) {
+		acc_a = (acc_a >> 32) + (uint64_t)a * b[i] + result[i];
+		acc_b = (acc_b >> 32) + (uint64_t)d0 * key->modulus[i] +
+				(uint32_t)acc_a;
+		result[i - 1] = (uint32_t)acc_b;
+	}
+
+	acc_a = (acc_a >> 32) + (acc_b >> 32);
+
+	result[i - 1] = (uint32_t)acc_a;
+
+	if (acc_a >> 32)
+		subtract_modulus(key, result);
+}
+
+/**
+ * montgomery_mul() - Perform montgomery mutitply
+ *
+ * Operation: montgomery result[] = a[] * b[] / n0inv % modulus
+ *
+ * @key:	RSA key
+ * @result:	Place to put result, as little endian word array
+ * @a:		Multiplier, as little endian word array
+ * @b:		Multiplicand, as little endian word array
+ */
+static void montgomery_mul(const struct rsa_public_key *key,
+		uint32_t result[], uint32_t a[], const uint32_t b[])
+{
+	uint i;
+
+	for (i = 0; i < key->len; ++i)
+		result[i] = 0;
+	for (i = 0; i < key->len; ++i)
+		montgomery_mul_add_step(key, result, a[i], b);
+}
+
+/**
+ * num_pub_exponent_bits() - Number of bits in the public exponent
+ *
+ * @key:	RSA key
+ * @num_bits:	Storage for the number of public exponent bits
+ */
+static int num_public_exponent_bits(const struct rsa_public_key *key,
+		int *num_bits)
+{
+	uint64_t exponent;
+	int exponent_bits;
+	const uint max_bits = (sizeof(exponent) * 8);
+
+	exponent = key->exponent;
+	exponent_bits = 0;
+
+	if (!exponent) {
+		*num_bits = exponent_bits;
+		return 0;
+	}
+
+	for (exponent_bits = 1; exponent_bits < max_bits + 1; ++exponent_bits)
+		if (!(exponent >>= 1)) {
+			*num_bits = exponent_bits;
+			return 0;
+		}
+
+	return -EINVAL;
+}
+
+/**
+ * is_public_exponent_bit_set() - Check if a bit in the public exponent is set
+ *
+ * @key:	RSA key
+ * @pos:	The bit position to check
+ */
+static int is_public_exponent_bit_set(const struct rsa_public_key *key,
+		int pos)
+{
+	return key->exponent & (1ULL << pos);
+}
+
+/**
+ * pow_mod() - in-place public exponentiation
+ *
+ * @key:	RSA key
+ * @inout:	Big-endian word array containing value and result
+ */
+static int pow_mod(const struct rsa_public_key *key, void *__inout)
+{
+	uint32_t *inout = __inout;
+	uint32_t *result, *ptr;
+	uint i;
+	int j, k;
+	uint32_t val[RSA_MAX_KEY_BITS / 32], acc[RSA_MAX_KEY_BITS / 32], tmp[RSA_MAX_KEY_BITS / 32];
+	uint32_t a_scaled[RSA_MAX_KEY_BITS / 32];
+
+	/* Sanity check for stack size - key->len is in 32-bit words */
+	if (key->len > RSA_MAX_KEY_BITS / 32) {
+		debug("RSA key words %u exceeds maximum %d\n", key->len,
+		      RSA_MAX_KEY_BITS / 32);
+		return -EINVAL;
+	}
+
+	result = tmp;  /* Re-use location. */
+
+	/* Convert from big endian byte array to little endian word array. */
+	for (i = 0, ptr = inout + key->len - 1; i < key->len; i++, ptr--)
+		val[i] = get_unaligned_be32(ptr);
+
+	if (0 != num_public_exponent_bits(key, &k))
+		return -EINVAL;
+
+	if (k < 2) {
+		debug("Public exponent is too short (%d bits, minimum 2)\n",
+		      k);
+		return -EINVAL;
+	}
+
+	if (!is_public_exponent_bit_set(key, 0)) {
+		debug("LSB of RSA public exponent must be set.\n");
+		return -EINVAL;
+	}
+
+	/* the bit at e[k-1] is 1 by definition, so start with: C := M */
+	montgomery_mul(key, acc, val, key->rr); /* acc = a * RR / R mod n */
+	/* retain scaled version for intermediate use */
+	memcpy(a_scaled, acc, key->len * sizeof(a_scaled[0]));
+
+	for (j = k - 2; j > 0; --j) {
+		montgomery_mul(key, tmp, acc, acc); /* tmp = acc^2 / R mod n */
+
+		if (is_public_exponent_bit_set(key, j)) {
+			/* acc = tmp * val / R mod n */
+			montgomery_mul(key, acc, tmp, a_scaled);
+		} else {
+			/* e[j] == 0, copy tmp back to acc for next operation */
+			memcpy(acc, tmp, key->len * sizeof(acc[0]));
+		}
+	}
+
+	/* the bit at e[0] is always 1 */
+	montgomery_mul(key, tmp, acc, acc); /* tmp = acc^2 / R mod n */
+	montgomery_mul(key, acc, tmp, val); /* acc = tmp * a / R mod M */
+	memcpy(result, acc, key->len * sizeof(result[0]));
+
+	/* Make sure result < mod; result is at most 1x mod too large. */
+	if (greater_equal_modulus(key, result))
+		subtract_modulus(key, result);
+
+	/* Convert to bigendian byte array */
+	for (i = key->len - 1, ptr = inout; (int)i >= 0; i--, ptr++)
+		put_unaligned_be32(result[i], ptr);
+	return 0;
+}
+
+/*
+ * Hash algorithm OIDs plus ASN.1 DER wrappings [RFC4880 sec 5.2.2].
+ */
+static const u8 RSA_digest_info_MD5[] = {
+	0x30, 0x20, 0x30, 0x0C, 0x06, 0x08,
+	0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x02, 0x05, /* OID */
+	0x05, 0x00, 0x04, 0x10
+};
+
+static const u8 RSA_digest_info_SHA1[] = {
+	0x30, 0x21, 0x30, 0x09, 0x06, 0x05,
+	0x2B, 0x0E, 0x03, 0x02, 0x1A,
+	0x05, 0x00, 0x04, 0x14
+};
+
+static const u8 RSA_digest_info_RIPE_MD_160[] = {
+	0x30, 0x21, 0x30, 0x09, 0x06, 0x05,
+	0x2B, 0x24, 0x03, 0x02, 0x01,
+	0x05, 0x00, 0x04, 0x14
+};
+
+static const u8 RSA_digest_info_SHA224[] = {
+	0x30, 0x2d, 0x30, 0x0d, 0x06, 0x09,
+	0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x04,
+	0x05, 0x00, 0x04, 0x1C
+};
+
+static const u8 RSA_digest_info_SHA256[] = {
+	0x30, 0x31, 0x30, 0x0d, 0x06, 0x09,
+	0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01,
+	0x05, 0x00, 0x04, 0x20
+};
+
+static const u8 RSA_digest_info_SHA384[] = {
+	0x30, 0x41, 0x30, 0x0d, 0x06, 0x09,
+	0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02,
+	0x05, 0x00, 0x04, 0x30
+};
+
+static const u8 RSA_digest_info_SHA512[] = {
+	0x30, 0x51, 0x30, 0x0d, 0x06, 0x09,
+	0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03,
+	0x05, 0x00, 0x04, 0x40
+};
+
+static const struct {
+	const u8 *data;
+	size_t size;
+} RSA_ASN1_templates[] = {
+#define _(X) { RSA_digest_info_##X, sizeof(RSA_digest_info_##X) }
+	[HASH_ALGO_MD5]		= _(MD5),
+	[HASH_ALGO_SHA1]	= _(SHA1),
+	[HASH_ALGO_RIPE_MD_160]	= _(RIPE_MD_160),
+	[HASH_ALGO_SHA256]	= _(SHA256),
+	[HASH_ALGO_SHA384]	= _(SHA384),
+	[HASH_ALGO_SHA512]	= _(SHA512),
+	[HASH_ALGO_SHA224]	= _(SHA224),
+#undef _
+};
+
+int rsa_verify(const struct rsa_public_key *key, const uint8_t *sig,
+			  const uint32_t sig_len, const uint8_t *hash,
+			  enum hash_algo algo)
+{
+	int ret = 0;
+	uint8_t buf[RSA_MAX_SIG_BITS / 8];
+	int i;
+	unsigned PS_end, T_offset;
+	const u8 *asn1_template = RSA_ASN1_templates[algo].data;
+	size_t asn1_size = RSA_ASN1_templates[algo].size;
+	struct digest *d = digest_alloc_by_algo(algo);
+
+	if (!d)
+		return -EOPNOTSUPP;
+
+	if (sig_len != (key->len * sizeof(uint32_t))) {
+		debug("Signature is of incorrect length %d, should be %d\n", sig_len,
+				key->len * sizeof(uint32_t));
+		ret = -EINVAL;
+		goto out_free_digest;
+	}
+
+	/* Sanity check for stack size */
+	if (sig_len > RSA_MAX_SIG_BITS / 8) {
+		debug("Signature length %u exceeds maximum %d\n", sig_len,
+		      RSA_MAX_SIG_BITS / 8);
+		ret = -EINVAL;
+		goto out_free_digest;
+	}
+
+	memcpy(buf, sig, sig_len);
+
+	ret = pow_mod(key, buf);
+	if (ret)
+		goto out_free_digest;
+
+	T_offset = sig_len - (asn1_size + digest_length(d));
+
+	PS_end = T_offset - 1;
+	if (buf[PS_end] != 0x00) {
+		pr_err(" = -EBADMSG [EM[T-1] == %02u]", buf[PS_end]);
+		ret = -EBADMSG;
+		goto out_free_digest;
+	}
+
+	for (i = 2; i < PS_end; i++) {
+		if (buf[i] != 0xff) {
+			pr_err(" = -EBADMSG [EM[PS%x] == %02u]", i - 2, buf[i]);
+			ret = -EBADMSG;
+			goto out_free_digest;
+		}
+	}
+
+	if (memcmp(asn1_template, buf + T_offset, asn1_size) != 0) {
+		pr_err(" = -EBADMSG [EM[T] ASN.1 mismatch]");
+		ret = -EBADMSG;
+		goto out_free_digest;
+	}
+
+	if (memcmp(hash, buf + T_offset + asn1_size, digest_length(d)) != 0) {
+		pr_err(" = -EKEYREJECTED [EM[T] hash mismatch]");
+		ret = -EKEYREJECTED;
+		goto out_free_digest;
+	}
+
+ out_free_digest:
+	digest_free(d);
+
+	return ret;
+}
+
+static void rsa_convert_big_endian(uint32_t *dst, const uint32_t *src, int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++)
+		dst[i] = fdt32_to_cpu(src[len - 1 - i]);
+}
+
+int rsa_of_read_key(struct device_node *node, struct rsa_public_key *key)
+{
+	const void *modulus, *rr;
+	const uint64_t *public_exponent;
+	int length;
+
+	of_property_read_u32(node, "rsa,num-bits", &key->len);
+	of_property_read_u32(node, "rsa,n0-inverse", &key->n0inv);
+
+	public_exponent = of_get_property(node, "rsa,exponent", &length);
+	if (!public_exponent || length < sizeof(*public_exponent))
+		key->exponent = RSA_DEFAULT_PUBEXP;
+	else
+		key->exponent = fdt64_to_cpu(*public_exponent);
+
+	modulus = of_get_property(node, "rsa,modulus", NULL);
+	rr = of_get_property(node, "rsa,r-squared", NULL);
+
+	if (!key->len || !modulus || !rr) {
+		debug("%s: Missing RSA key info", __func__);
+		return -EFAULT;
+	}
+
+	/* Sanity check for stack size */
+	if (key->len > RSA_MAX_KEY_BITS || key->len < RSA_MIN_KEY_BITS) {
+		debug("RSA key bits %u outside allowed range %d..%d\n",
+		      key->len, RSA_MIN_KEY_BITS, RSA_MAX_KEY_BITS);
+		return -EFAULT;
+	}
+
+	key->len /= sizeof(uint32_t) * 8;
+
+	key->modulus = xzalloc(RSA_MAX_KEY_BITS / 8);
+	key->rr = xzalloc(RSA_MAX_KEY_BITS / 8);
+
+	rsa_convert_big_endian(key->modulus, modulus, key->len);
+	rsa_convert_big_endian(key->rr, rr, key->len);
+
+	return 0;
+}
diff --git a/include/asm-generic/errno.h b/include/asm-generic/errno.h
index 6072f7b605bb..7d99a9537050 100644
--- a/include/asm-generic/errno.h
+++ b/include/asm-generic/errno.h
@@ -126,6 +126,11 @@
 
 #define	ENOMEDIUM	123	/* No medium found */
 #define	EMEDIUMTYPE	124	/* Wrong medium type */
+#define	ECANCELED	125	/* Operation Canceled */
+#define	ENOKEY		126	/* Required key not available */
+#define	EKEYEXPIRED	127	/* Key has expired */
+#define	EKEYREVOKED	128	/* Key has been revoked */
+#define	EKEYREJECTED	129	/* Key was rejected by service */
 
 /* Should never be seen by user programs */
 #define ERESTARTSYS	512
diff --git a/include/rsa.h b/include/rsa.h
new file mode 100644
index 000000000000..feb8c3120023
--- /dev/null
+++ b/include/rsa.h
@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2013, Google Inc.
+ *
+ * (C) Copyright 2008 Semihalf
+ *
+ * (C) Copyright 2000-2006
+ * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _RSA_H
+#define _RSA_H
+
+#include <errno.h>
+#include <digest.h>
+
+/**
+ * struct rsa_public_key - holder for a public key
+ *
+ * An RSA public key consists of a modulus (typically called N), the inverse
+ * and R^2, where R is 2^(# key bits).
+ */
+
+struct rsa_public_key {
+	uint len;		/* len of modulus[] in number of uint32_t */
+	uint32_t n0inv;		/* -1 / modulus[0] mod 2^32 */
+	uint32_t *modulus;	/* modulus as little endian array */
+	uint32_t *rr;		/* R^2 as little endian array */
+	uint64_t exponent;	/* public exponent */
+};
+
+/**
+ * rsa_verify() - Verify a signature against some data
+ *
+ * Verify a RSA PKCS1.5 signature against an expected hash.
+ *
+ * @info:	Specifies key and FIT information
+ * @data:	Pointer to the input data
+ * @data_len:	Data length
+ * @sig:	Signature
+ * @sig_len:	Number of bytes in signature
+ * @return 0 if verified, -ve on error
+ */
+int rsa_verify(const struct rsa_public_key *key, const uint8_t *sig,
+			  const uint32_t sig_len, const uint8_t *hash,
+			  enum hash_algo algo);
+
+/* This is the maximum signature length that we support, in bits */
+#define RSA_MAX_SIG_BITS	4096
+
+int rsa_of_read_key(struct device_node *node, struct rsa_public_key *key);
+
+#endif
-- 
2.6.2


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

[PATCH 3/3] bootm: add initial FIT support

[Marc Kleine-Budde]

From: Jan Luebbe <jlu@pengutronix.de>

This implementation is inspired by U-Boot's FIT support. Instead of
using libfdt (which does not exist in barebox), configuration signatures
are verified by using a simplified DT parser based on barebox's own
code.

Currently, only signed configurations with hashed images are supported,
as the other variants are less useful for verified boot. Compatible FIT
images can be created using U-Boot's mkimage tool.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 arch/arm/lib/bootm.c |  74 +++++++
 commands/Kconfig     |   8 +
 common/Kconfig       |   6 +
 common/Makefile      |   1 +
 common/image-fit.c   | 590 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/image-fit.h  |  42 ++++
 6 files changed, 721 insertions(+)
 create mode 100644 common/image-fit.c
 create mode 100644 include/image-fit.h

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 7bb9b436560c..22c2d6017e7c 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -553,6 +553,78 @@ BAREBOX_MAGICVAR(aimage_noverwrite_bootargs, "Disable overwrite of the bootargs
 BAREBOX_MAGICVAR(aimage_noverwrite_tags, "Disable overwrite of the tags addr with the one present in aimage");
 #endif
 
+#include <image-fit.h>
+
+static int do_bootm_arm_fit(struct image_data *data)
+{
+	struct fit_handle *handle;
+	int ret;
+	unsigned long mem_free;
+	unsigned long mem_start, mem_size;
+
+	handle = fit_open(data->os_file, data->os_num, data->verbose);
+	if (!handle)
+		return -EINVAL;
+
+	ret = sdram_start_and_size(&mem_start, &mem_size);
+	if (ret)
+		return ret;
+
+	/* no support for custom load address */
+	data->os_address = mem_start + PAGE_ALIGN(handle->kernel_size * 4);
+	data->os_res = request_sdram_region("fit-kernel", data->os_address, handle->kernel_size);
+	if (!data->os_res) {
+		pr_err("Cannot request region 0x%08lx - 0x%08lx\n",
+				data->os_address, handle->kernel_size);
+		ret = -ENOMEM;
+		goto err_out;
+	}
+	memcpy((void *)data->os_res->start, handle->kernel, handle->kernel_size);
+
+	/*
+	 * Put oftree/initrd close behind compressed kernel image to avoid
+	 * placing it outside of the kernels lowmem.
+	 */
+	if (handle->initrd_size) {
+		data->initrd_address = PAGE_ALIGN(data->os_res->end + SZ_1M);
+		data->initrd_res = request_sdram_region("fit-initrd", data->initrd_address, handle->initrd_size);
+		if (!data->initrd_res) {
+			ret = -ENOMEM;
+			goto err_out;
+		}
+		memcpy((void *)data->initrd_res->start, handle->initrd, handle->initrd_size);
+	}
+
+	data->of_root_node = of_unflatten_dtb(handle->oftree);
+	if (!data->of_root_node) {
+		pr_err("unable to unflatten devicetree\n");
+		ret = -EINVAL;
+		goto err_out;
+	}
+
+	/*
+	 * Put devicetree right after initrd if present or after the kernel
+	 * if not.
+	 */
+	if (data->initrd_res)
+		mem_free = PAGE_ALIGN(data->initrd_res->end);
+	else
+		mem_free = PAGE_ALIGN(data->os_res->end + SZ_1M);
+
+	return __do_bootm_linux(data, mem_free, 0);
+
+err_out:
+	if (handle)
+		fit_close(handle);
+	return ret;
+}
+
+static struct image_handler arm_fit_handler = {
+        .name = "FIT image",
+        .bootm = do_bootm_arm_fit,
+        .filetype = filetype_oftree,
+};
+
 static struct binfmt_hook binfmt_aimage_hook = {
 	.type = filetype_aimage,
 	.exec = "bootm",
@@ -578,6 +650,8 @@ static int armlinux_register_image_handler(void)
 		register_image_handler(&aimage_handler);
 		binfmt_register(&binfmt_aimage_hook);
 	}
+	if (IS_BUILTIN(CONFIG_CMD_BOOTM_FITIMAGE))
+	        register_image_handler(&arm_fit_handler);
 	binfmt_register(&binfmt_arm_zimage_hook);
 	binfmt_register(&binfmt_barebox_hook);
 
diff --git a/commands/Kconfig b/commands/Kconfig
index 1743670ed33c..b89627209f5a 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -418,6 +418,14 @@ config CMD_BOOTM_AIMAGE
 	help
 	  Support using Android Images.
 
+config CMD_BOOTM_FITIMAGE
+	bool
+	prompt "FIT image support"
+	select FITIMAGE
+	depends on CMD_BOOTM && ARM
+	help
+	  Support using FIT Images.
+
 config CMD_BOOTU
 	tristate
 	default y
diff --git a/common/Kconfig b/common/Kconfig
index 8e7950968c3e..d824b5e35f04 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -66,6 +66,12 @@ config UIMAGE
 	select CRC32
 	bool
 
+config FITIMAGE
+	bool
+	select OFTREE
+	select DIGEST
+	select CRYPTO_RSA
+
 config LOGBUF
 	bool
 
diff --git a/common/Makefile b/common/Makefile
index 56e6becec078..ffaf8e7b42eb 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_SHELL_HUSH)	+= hush.o
 obj-$(CONFIG_SHELL_SIMPLE)	+= parser.o
 obj-$(CONFIG_STATE)		+= state.o
 obj-$(CONFIG_UIMAGE)		+= image.o uimage.o
+obj-$(CONFIG_FITIMAGE)		+= image-fit.o
 obj-$(CONFIG_MENUTREE)		+= menutree.o
 obj-$(CONFIG_EFI_GUID)		+= efi-guid.o
 obj-$(CONFIG_EFI_DEVICEPATH)	+= efi-devicepath.o
diff --git a/common/image-fit.c b/common/image-fit.c
new file mode 100644
index 000000000000..fc95ed4d8118
--- /dev/null
+++ b/common/image-fit.c
@@ -0,0 +1,590 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright (C) Jan Lübbe, 2014
+ */
+
+#include <common.h>
+#include <init.h>
+#include <boot.h>
+#include <libfile.h>
+#include <fdt.h>
+#include <digest.h>
+#include <of.h>
+#include <fs.h>
+#include <malloc.h>
+#include <linux/ctype.h>
+#include <asm/byteorder.h>
+#include <errno.h>
+#include <linux/err.h>
+#include <stringlist.h>
+#include <rsa.h>
+#include <image-fit.h>
+
+#define FDT_MAX_DEPTH 32
+#define FDT_MAX_PATH_LEN 200
+
+#define CHECK_LEVEL_NONE 0
+#define CHECK_LEVEL_HASH 1
+#define CHECK_LEVEL_SIG 2
+#define CHECK_LEVEL_MAX 3
+
+static uint32_t dt_struct_advance(struct fdt_header *f, uint32_t dt, int size)
+{
+	dt += size;
+	dt = ALIGN(dt, 4);
+
+	if (dt > f->off_dt_struct + f->size_dt_struct)
+		return 0;
+
+	return dt;
+}
+
+static char *dt_string(struct fdt_header *f, char *strstart, uint32_t ofs)
+{
+	if (ofs > f->size_dt_strings)
+		return NULL;
+	else
+		return strstart + ofs;
+}
+
+static int of_read_string_list(struct device_node *np, const char *name, struct string_list *sl)
+{
+	struct property *prop;
+	const char *s;
+
+	of_property_for_each_string(np, name, prop, s) {
+		string_list_add(sl, s);
+	}
+
+	return prop ? 0 : -EINVAL;
+}
+
+static int fit_digest(void *fit, struct digest *digest,
+		      struct string_list *inc_nodes, struct string_list *exc_props,
+		      uint32_t hashed_strings_start, uint32_t hashed_strings_size)
+{
+	struct fdt_header *fdt = fit;
+	uint32_t dt_struct;
+	void *dt_strings;
+	struct fdt_header f;
+	int stack[FDT_MAX_DEPTH];
+	char path[FDT_MAX_PATH_LEN];
+	char *end;
+	uint32_t tag;
+	int start = -1;
+	int depth = -1;
+	int want = 0;
+
+	f.totalsize = fdt32_to_cpu(fdt->totalsize);
+	f.off_dt_struct = fdt32_to_cpu(fdt->off_dt_struct);
+	f.size_dt_struct = fdt32_to_cpu(fdt->size_dt_struct);
+	f.off_dt_strings = fdt32_to_cpu(fdt->off_dt_strings);
+	f.size_dt_strings = fdt32_to_cpu(fdt->size_dt_strings);
+
+	if (hashed_strings_start > f.size_dt_strings ||
+	    hashed_strings_size > f.size_dt_strings ||
+	    hashed_strings_start + hashed_strings_size > f.size_dt_strings) {
+		pr_err("%s: hashed-strings too large\n", __func__);
+		return -EINVAL;
+	}
+
+	dt_struct = f.off_dt_struct;
+	dt_strings = (void *)fdt + f.off_dt_strings;
+
+	end = path;
+	*end = '\0';
+
+	do {
+		const struct fdt_property *fdt_prop;
+		const struct fdt_node_header *fnh;
+		const char *name;
+		int include = 0;
+		int stop_at = 0;
+		int offset = dt_struct;
+		int maxlen, len;
+
+		tag = be32_to_cpu(*(uint32_t *)(fit + dt_struct));
+
+		switch (tag) {
+		case FDT_BEGIN_NODE:
+			fnh = fit + dt_struct;
+			name = fnh->name;
+			maxlen = (unsigned long)fdt + f.off_dt_struct +
+				f.size_dt_struct - (unsigned long)name;
+
+			len = strnlen(name, maxlen + 1);
+			if (len > maxlen)
+				return -ESPIPE;
+
+			dt_struct = dt_struct_advance(&f, dt_struct,
+						      sizeof(struct fdt_node_header) + len + 1);
+
+			depth++;
+			if (depth == FDT_MAX_DEPTH)
+				return -ESPIPE;
+			if (end - path + 2 + len >= FDT_MAX_PATH_LEN)
+				return -ESPIPE;
+			if (end != path + 1)
+				*end++ = '/';
+			strcpy(end, name);
+			end += len;
+			stack[depth] = want;
+			if (want == 1)
+				stop_at = offset;
+			if (string_list_contains(inc_nodes, path))
+				want = 2;
+			else if (want)
+				want--;
+			else
+				stop_at = offset;
+			include = want;
+
+			break;
+
+		case FDT_END_NODE:
+			dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE);
+
+			include = want;
+			want = stack[depth--];
+			while (end > path && *--end != '/')
+				;
+			*end = '\0';
+
+			break;
+
+		case FDT_PROP:
+			fdt_prop = fit + dt_struct;
+			len = fdt32_to_cpu(fdt_prop->len);
+
+			name = dt_string(&f, dt_strings, fdt32_to_cpu(fdt_prop->nameoff));
+			if (!name)
+				return -ESPIPE;
+
+			dt_struct = dt_struct_advance(&f, dt_struct,
+						      sizeof(struct fdt_property) + len);
+
+			include = want >= 2;
+			stop_at = offset;
+			if (string_list_contains(exc_props, name))
+				include = 0;
+
+			break;
+
+		case FDT_NOP:
+			dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE);
+
+			include = want >= 2;
+			stop_at = offset;
+
+			break;
+
+		case FDT_END:
+			dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE);
+
+			include = 1;
+
+			break;
+
+		default:
+			pr_err("%s: Unknown tag 0x%08X\n", __func__, tag);
+			return -EINVAL;
+		}
+
+		if (!dt_struct)
+			return -ESPIPE;
+
+		pr_debug("%s: include %d, want %d, offset 0x%x, len 0x%x\n",
+			 path, include, want, offset, dt_struct-offset);
+
+		if (include && start == -1)
+			start = offset;
+
+		if (!include && start != -1) {
+			pr_debug("region: 0x%p+0x%x\n", fit+start, offset-start);
+			digest_update(digest, fit+start, offset-start);
+			start = -1;
+		}
+	} while (tag != FDT_END);
+
+	pr_debug("region: 0x%p+0x%x\n", fit+start, dt_struct-start);
+	digest_update(digest, fit+start, dt_struct-start);
+
+	pr_debug("strings: 0x%p+0x%x\n", dt_strings+hashed_strings_start, hashed_strings_size);
+	digest_update(digest, dt_strings+hashed_strings_start, hashed_strings_size);
+
+	return 0;
+}
+
+/*
+ * The consistency of the FTD structure was already checked by of_unflatten_dtb()
+ */
+static int fit_verify_signature(struct device_node *sig_node, void *fit)
+{
+	uint32_t hashed_strings_start, hashed_strings_size;
+	struct string_list inc_nodes, exc_props;
+	struct rsa_public_key key = {};
+	struct digest *digest;
+	int sig_len;
+	const char *algo_name, *key_name, *sig_value;
+	char *key_path;
+	struct device_node *key_node;
+	enum hash_algo algo;
+	void *hash;
+	int ret;
+
+	if (of_property_read_string(sig_node, "algo", &algo_name)) {
+		pr_err("algo not found\n");
+		ret = -EINVAL;
+		goto out;
+	}
+	if (strcmp(algo_name, "sha1,rsa2048") == 0) {
+		algo = HASH_ALGO_SHA1;
+	} else if (strcmp(algo_name, "sha256,rsa4096") == 0) {
+		algo = HASH_ALGO_SHA256;
+	} else	{
+		pr_err("unknown algo %s\n", algo_name);
+		ret = -EINVAL;
+		goto out;
+	}
+	digest = digest_alloc_by_algo(algo);
+	if (!digest) {
+		pr_err("unsupported algo %s\n", algo_name);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	sig_value = of_get_property(sig_node, "value", &sig_len);
+	if (!sig_value) {
+		pr_err("signature value not found\n");
+		ret = -EINVAL;
+		goto out_free_digest;
+	}
+
+	if (of_property_read_string(sig_node, "key-name-hint", &key_name)) {
+		pr_err("key name not found\n");
+		ret = -EINVAL;
+		goto out_free_digest;
+	}
+	key_path = asprintf("/signature/key-%s", key_name);
+	if (!key_name) {
+		ret = -ENOMEM;
+		goto out_free_digest;
+	}
+	key_node = of_find_node_by_path(key_path);
+	free(key_path);
+	if (!key_node) {
+		pr_info("failed to find key node\n");
+		ret = -ENOENT;
+		goto out_free_digest;
+	}
+
+	ret = rsa_of_read_key(key_node, &key);
+	if (ret) {
+		pr_info("failed to read key\n");
+		ret = -ENOENT;
+		goto out_free_digest;
+	}
+
+	if (of_property_read_u32_index(sig_node, "hashed-strings", 0, &hashed_strings_start)) {
+		pr_err("%s: hashed-strings start not found\n", __func__);
+		ret = -EINVAL;
+		goto out_free_digest;
+	}
+	if (of_property_read_u32_index(sig_node, "hashed-strings", 1, &hashed_strings_size)) {
+		pr_err("%s: hashed-strings size not found\n", __func__);
+		ret = -EINVAL;
+		goto out_free_digest;
+	}
+
+	string_list_init(&inc_nodes);
+	string_list_init(&exc_props);
+
+	if (of_read_string_list(sig_node, "hashed-nodes", &inc_nodes)) {
+		pr_err("%s: hashed-nodes invalid\n", __func__);
+		ret = -EINVAL;
+		goto out_sl;
+	}
+
+	string_list_add(&exc_props, "data");
+
+	digest_init(digest);
+	ret = fit_digest(fit, digest, &inc_nodes, &exc_props, hashed_strings_start, hashed_strings_size);
+	hash = xzalloc(digest_length(digest));
+	digest_final(digest, hash);
+
+	ret = rsa_verify(&key, sig_value, sig_len, hash, algo);
+	if (ret) {
+		pr_info("sig BAD\n");
+		ret = CHECK_LEVEL_NONE;
+	} else {
+		pr_info("sig OK\n");
+		ret = CHECK_LEVEL_SIG;
+	}
+
+	free(hash);
+ out_sl:
+	string_list_free(&inc_nodes);
+	string_list_free(&exc_props);
+ out_free_digest:
+	digest_free(digest);
+ out:
+	return ret;
+}
+
+static int fit_verify_hash(struct device_node *hash, const void *data, int data_len)
+{
+	struct digest *d;
+	const char *algo;
+	const char *value_read;
+	char *value_calc;
+	int hash_len;
+
+	value_read = of_get_property(hash, "value", &hash_len);
+	if (!value_read) {
+		pr_err("value not found\n");
+		return CHECK_LEVEL_NONE;
+	}
+
+	if (of_property_read_string(hash, "algo", &algo)) {
+		pr_err("algo not found\n");
+		return -EINVAL;
+	}
+
+	d = digest_alloc(algo);
+	if (!d) {
+		pr_err("unsupported algo %s\n", algo);
+		return -EINVAL;
+	}
+
+	if (hash_len != digest_length(d)) {
+		pr_err("invalid hash length %d\n", hash_len);
+		digest_free(d);
+		return -EINVAL;
+	}
+
+	value_calc = xmalloc(hash_len);
+
+	digest_init(d);
+	digest_update(d, data, data_len);
+	digest_final(d, value_calc);
+
+	if (memcmp(value_read, value_calc, hash_len)) {
+		pr_info("hash BAD\n");
+		digest_free(d);
+		return CHECK_LEVEL_NONE;
+	} else {
+		pr_info("hash OK\n");
+		digest_free(d);
+		return CHECK_LEVEL_HASH;
+	}
+}
+
+static int fit_open_image(struct fit_handle *handle, const char* unit)
+{
+	struct device_node *image = NULL, *hash;
+	const char *type = NULL, *desc;
+	const void *data;
+	int data_len;
+	int ret, level;
+
+	image = of_get_child_by_name(handle->root, "images");
+	if (!image)
+		return -ENOENT;
+
+	image = of_get_child_by_name(image, unit);
+	if (!image)
+		return -ENOENT;
+
+	if (of_property_read_string(image, "description", &desc)) {
+		pr_info("FIT image '%s' (no description)\n", unit);
+	} else {
+		pr_info("FIT image '%s': '%s'\n", unit, desc);
+	}
+
+	of_property_read_string(image, "type", &type);
+	if (!type)
+		return -EINVAL;
+
+	data = of_get_property(image, "data", &data_len);
+	if (!data) {
+		pr_err("data not found\n");
+		return -EINVAL;
+	}
+
+	level = CHECK_LEVEL_MAX;
+	for_each_child_of_node(image, hash) {
+		if (handle->verbose)
+			of_print_nodes(hash, 0);
+		ret = fit_verify_hash(hash, data, data_len);
+		if (ret < 0)
+			return ret;
+		level = min(level, ret);
+	}
+	if (level == CHECK_LEVEL_MAX) {
+		return -EINVAL;
+	}
+
+	if (level == CHECK_LEVEL_HASH) {
+		if (strcmp(type, "kernel") == 0 ||
+		    strcmp(type, "kernel_noload") == 0) {
+			handle->kernel = data;
+			handle->kernel_size = data_len;
+		} else if (strcmp(type, "flat_dt") == 0) {
+			handle->oftree = data;
+			handle->oftree_size = data_len;
+		} else if (strcmp(type, "ramdisk") == 0) {
+			handle->initrd = data;
+			handle->initrd_size = data_len;
+		} else {
+			pr_info("unknown image type %s, ignoring\n", type);
+		}
+	}
+
+	return level;
+}
+
+static int fit_open_configuration(struct fit_handle *handle, int num)
+{
+	struct device_node *conf_node = NULL, *sig_node;
+	char unit_name[10];
+	const char *unit, *desc;
+	int ret, level;
+
+	conf_node = of_get_child_by_name(handle->root, "configurations");
+	if (!conf_node)
+		return -ENOENT;
+
+	if (num) {
+		snprintf(unit_name, sizeof(unit_name), "conf@%d", num);
+		unit = unit_name;
+	} else if (of_property_read_string(conf_node, "default", &unit)) {
+		unit = "conf@1";
+	}
+
+	conf_node = of_get_child_by_name(conf_node, unit);
+	if (!conf_node) {
+		pr_err("FIT configuration '%s' not found\n", unit);
+		return -ENOENT;
+	}
+
+	if (of_property_read_string(conf_node, "description", &desc)) {
+		pr_info("FIT configuration '%s' (no description)\n", unit);
+	} else {
+		pr_info("FIT configuration '%s': '%s'\n", unit, desc);
+	}
+
+	level = CHECK_LEVEL_MAX;
+	for_each_child_of_node(conf_node, sig_node) {
+		if (handle->verbose)
+			of_print_nodes(sig_node, 0);
+		ret = fit_verify_signature(sig_node, handle->fit);
+		if (ret < 0)
+			return ret;
+		level = min(level, ret);
+	}
+	if (level == CHECK_LEVEL_MAX)
+		return -EINVAL;
+
+	if (level != CHECK_LEVEL_SIG)
+		return -EINVAL;
+
+	if (of_property_read_string(conf_node, "kernel", &unit) == 0)
+		level = min(level, fit_open_image(handle, unit));
+	else
+		return -ENOENT;
+
+	if (of_property_read_string(conf_node, "fdt", &unit) == 0)
+		level = min(level, fit_open_image(handle, unit));
+
+	if (of_property_read_string(conf_node, "ramdisk", &unit) == 0)
+		level = min(level, fit_open_image(handle, unit));
+
+	if (level != CHECK_LEVEL_HASH)
+		return -EINVAL;
+
+	return 0;
+}
+
+struct fit_handle *fit_open(const char *filename, int num, bool verbose)
+{
+	struct fit_handle *handle = NULL;
+	const char *desc;
+
+	handle = xzalloc(sizeof(struct fit_handle));
+
+	handle->verbose = verbose;
+
+	handle->fit = read_file(filename, &handle->size);
+	if (!handle->fit) {
+		pr_err("unable to read %s: %s\n", filename, strerror(errno));
+		goto err;
+	}
+
+	handle->root = of_unflatten_dtb(handle->fit);
+	if (IS_ERR(handle->root)) {
+		goto err;
+	}
+
+	if (of_property_read_string(handle->root, "description", &desc)) {
+		pr_info("FIT '%s' (no description)\n", filename);
+	} else {
+		pr_info("FIT '%s': '%s'\n", filename, desc);
+	}
+
+	if (fit_open_configuration(handle, num))
+		goto err;
+
+	return handle;
+ err:
+	if (handle->root)
+		of_delete_node(handle->root);
+	if (handle->fit)
+		free(handle->fit);
+	free(handle);
+
+	return NULL;
+}
+
+void fit_close(struct fit_handle *handle)
+{
+	if (handle->root)
+		of_delete_node(handle->root);
+	if (handle->fit)
+		free(handle->fit);
+	free(handle);
+}
+
+#ifdef CONFIG_SANDBOX
+static int do_bootm_sandbox_fit(struct image_data *data)
+{
+	struct fit_handle *handle;
+	handle = fit_open(data->os_file, data->os_num, data->verbose);
+	if (handle)
+		fit_close(handle);
+	return 0;
+}
+
+static struct image_handler sandbox_fit_handler = {
+	.name = "FIT image",
+	.bootm = do_bootm_sandbox_fit,
+	.filetype = filetype_oftree,
+};
+
+static int sandbox_fit_register(void)
+{
+	return register_image_handler(&sandbox_fit_handler);
+}
+late_initcall(sandbox_fit_register);
+#endif
diff --git a/include/image-fit.h b/include/image-fit.h
new file mode 100644
index 000000000000..bcbc859ead37
--- /dev/null
+++ b/include/image-fit.h
@@ -0,0 +1,42 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright (C) Jan Lübbe, 2014
+ */
+
+#ifndef __IMAGE_FIT_H__
+#define __IMAGE_FIT_H__
+
+#include <linux/types.h>
+
+struct fit_handle {
+	void *fit;
+	size_t size;
+
+	bool verbose;
+
+	struct device_node *root;
+
+	const void *kernel;
+	unsigned long kernel_size;
+	const void *oftree;
+	unsigned long oftree_size;
+	const void *initrd;
+	unsigned long initrd_size;
+};
+
+struct fit_handle *fit_open(const char *filename, int num, bool verbose);
+void fit_close(struct fit_handle *handle);
+
+#endif	/* __IMAGE_FIT_H__ */
-- 
2.6.2


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

Re: [PATCH 3/3] bootm: add initial FIT support

[Yegor Yefremov]

Hi Marc,

thanks for reposting the patches.

On Tue, Jan 5, 2016 at 9:11 AM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> From: Jan Luebbe <jlu@pengutronix.de>
>
> This implementation is inspired by U-Boot's FIT support. Instead of
> using libfdt (which does not exist in barebox), configuration signatures
> are verified by using a simplified DT parser based on barebox's own
> code.
>
> Currently, only signed configurations with hashed images are supported,
> as the other variants are less useful for verified boot. Compatible FIT
> images can be created using U-Boot's mkimage tool.

What about unsigned images?

I also get: unsupported algo crc32

Is it intended to be supported?

> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  arch/arm/lib/bootm.c |  74 +++++++
>  commands/Kconfig     |   8 +
>  common/Kconfig       |   6 +
>  common/Makefile      |   1 +
>  common/image-fit.c   | 590 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/image-fit.h  |  42 ++++
>  6 files changed, 721 insertions(+)
>  create mode 100644 common/image-fit.c
>  create mode 100644 include/image-fit.h
>
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 7bb9b436560c..22c2d6017e7c 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -553,6 +553,78 @@ BAREBOX_MAGICVAR(aimage_noverwrite_bootargs, "Disable overwrite of the bootargs
>  BAREBOX_MAGICVAR(aimage_noverwrite_tags, "Disable overwrite of the tags addr with the one present in aimage");
>  #endif
>
> +#include <image-fit.h>
> +
> +static int do_bootm_arm_fit(struct image_data *data)
> +{
> +       struct fit_handle *handle;
> +       int ret;
> +       unsigned long mem_free;
> +       unsigned long mem_start, mem_size;
> +
> +       handle = fit_open(data->os_file, data->os_num, data->verbose);
> +       if (!handle)
> +               return -EINVAL;
> +
> +       ret = sdram_start_and_size(&mem_start, &mem_size);
> +       if (ret)
> +               return ret;
> +
> +       /* no support for custom load address */
> +       data->os_address = mem_start + PAGE_ALIGN(handle->kernel_size * 4);
> +       data->os_res = request_sdram_region("fit-kernel", data->os_address, handle->kernel_size);
> +       if (!data->os_res) {
> +               pr_err("Cannot request region 0x%08lx - 0x%08lx\n",
> +                               data->os_address, handle->kernel_size);
> +               ret = -ENOMEM;
> +               goto err_out;
> +       }
> +       memcpy((void *)data->os_res->start, handle->kernel, handle->kernel_size);
> +
> +       /*
> +        * Put oftree/initrd close behind compressed kernel image to avoid
> +        * placing it outside of the kernels lowmem.
> +        */
> +       if (handle->initrd_size) {
> +               data->initrd_address = PAGE_ALIGN(data->os_res->end + SZ_1M);
> +               data->initrd_res = request_sdram_region("fit-initrd", data->initrd_address, handle->initrd_size);
> +               if (!data->initrd_res) {
> +                       ret = -ENOMEM;
> +                       goto err_out;
> +               }
> +               memcpy((void *)data->initrd_res->start, handle->initrd, handle->initrd_size);
> +       }
> +
> +       data->of_root_node = of_unflatten_dtb(handle->oftree);
> +       if (!data->of_root_node) {
> +               pr_err("unable to unflatten devicetree\n");
> +               ret = -EINVAL;
> +               goto err_out;
> +       }
> +
> +       /*
> +        * Put devicetree right after initrd if present or after the kernel
> +        * if not.
> +        */
> +       if (data->initrd_res)
> +               mem_free = PAGE_ALIGN(data->initrd_res->end);
> +       else
> +               mem_free = PAGE_ALIGN(data->os_res->end + SZ_1M);
> +
> +       return __do_bootm_linux(data, mem_free, 0);
> +
> +err_out:
> +       if (handle)
> +               fit_close(handle);
> +       return ret;
> +}
> +
> +static struct image_handler arm_fit_handler = {
> +        .name = "FIT image",
> +        .bootm = do_bootm_arm_fit,
> +        .filetype = filetype_oftree,
> +};
> +
>  static struct binfmt_hook binfmt_aimage_hook = {
>         .type = filetype_aimage,
>         .exec = "bootm",
> @@ -578,6 +650,8 @@ static int armlinux_register_image_handler(void)
>                 register_image_handler(&aimage_handler);
>                 binfmt_register(&binfmt_aimage_hook);
>         }
> +       if (IS_BUILTIN(CONFIG_CMD_BOOTM_FITIMAGE))
> +               register_image_handler(&arm_fit_handler);
>         binfmt_register(&binfmt_arm_zimage_hook);
>         binfmt_register(&binfmt_barebox_hook);
>
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 1743670ed33c..b89627209f5a 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -418,6 +418,14 @@ config CMD_BOOTM_AIMAGE
>         help
>           Support using Android Images.
>
> +config CMD_BOOTM_FITIMAGE
> +       bool
> +       prompt "FIT image support"
> +       select FITIMAGE
> +       depends on CMD_BOOTM && ARM
> +       help
> +         Support using FIT Images.
> +
>  config CMD_BOOTU
>         tristate
>         default y
> diff --git a/common/Kconfig b/common/Kconfig
> index 8e7950968c3e..d824b5e35f04 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -66,6 +66,12 @@ config UIMAGE
>         select CRC32
>         bool
>
> +config FITIMAGE
> +       bool
> +       select OFTREE
> +       select DIGEST
> +       select CRYPTO_RSA
> +
>  config LOGBUF
>         bool
>
> diff --git a/common/Makefile b/common/Makefile
> index 56e6becec078..ffaf8e7b42eb 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -46,6 +46,7 @@ obj-$(CONFIG_SHELL_HUSH)      += hush.o
>  obj-$(CONFIG_SHELL_SIMPLE)     += parser.o
>  obj-$(CONFIG_STATE)            += state.o
>  obj-$(CONFIG_UIMAGE)           += image.o uimage.o
> +obj-$(CONFIG_FITIMAGE)         += image-fit.o
>  obj-$(CONFIG_MENUTREE)         += menutree.o
>  obj-$(CONFIG_EFI_GUID)         += efi-guid.o
>  obj-$(CONFIG_EFI_DEVICEPATH)   += efi-devicepath.o
> diff --git a/common/image-fit.c b/common/image-fit.c
> new file mode 100644
> index 000000000000..fc95ed4d8118
> --- /dev/null
> +++ b/common/image-fit.c
> @@ -0,0 +1,590 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + * Copyright (C) Jan Lübbe, 2014
> + */
> +
> +#include <common.h>
> +#include <init.h>
> +#include <boot.h>
> +#include <libfile.h>
> +#include <fdt.h>
> +#include <digest.h>
> +#include <of.h>
> +#include <fs.h>
> +#include <malloc.h>
> +#include <linux/ctype.h>
> +#include <asm/byteorder.h>
> +#include <errno.h>
> +#include <linux/err.h>
> +#include <stringlist.h>
> +#include <rsa.h>
> +#include <image-fit.h>
> +
> +#define FDT_MAX_DEPTH 32
> +#define FDT_MAX_PATH_LEN 200
> +
> +#define CHECK_LEVEL_NONE 0
> +#define CHECK_LEVEL_HASH 1
> +#define CHECK_LEVEL_SIG 2
> +#define CHECK_LEVEL_MAX 3
> +
> +static uint32_t dt_struct_advance(struct fdt_header *f, uint32_t dt, int size)
> +{
> +       dt += size;
> +       dt = ALIGN(dt, 4);
> +
> +       if (dt > f->off_dt_struct + f->size_dt_struct)
> +               return 0;
> +
> +       return dt;
> +}
> +
> +static char *dt_string(struct fdt_header *f, char *strstart, uint32_t ofs)
> +{
> +       if (ofs > f->size_dt_strings)
> +               return NULL;
> +       else
> +               return strstart + ofs;
> +}
> +
> +static int of_read_string_list(struct device_node *np, const char *name, struct string_list *sl)
> +{
> +       struct property *prop;
> +       const char *s;
> +
> +       of_property_for_each_string(np, name, prop, s) {
> +               string_list_add(sl, s);
> +       }
> +
> +       return prop ? 0 : -EINVAL;
> +}
> +
> +static int fit_digest(void *fit, struct digest *digest,
> +                     struct string_list *inc_nodes, struct string_list *exc_props,
> +                     uint32_t hashed_strings_start, uint32_t hashed_strings_size)
> +{
> +       struct fdt_header *fdt = fit;
> +       uint32_t dt_struct;
> +       void *dt_strings;
> +       struct fdt_header f;
> +       int stack[FDT_MAX_DEPTH];
> +       char path[FDT_MAX_PATH_LEN];
> +       char *end;
> +       uint32_t tag;
> +       int start = -1;
> +       int depth = -1;
> +       int want = 0;
> +
> +       f.totalsize = fdt32_to_cpu(fdt->totalsize);
> +       f.off_dt_struct = fdt32_to_cpu(fdt->off_dt_struct);
> +       f.size_dt_struct = fdt32_to_cpu(fdt->size_dt_struct);
> +       f.off_dt_strings = fdt32_to_cpu(fdt->off_dt_strings);
> +       f.size_dt_strings = fdt32_to_cpu(fdt->size_dt_strings);
> +
> +       if (hashed_strings_start > f.size_dt_strings ||
> +           hashed_strings_size > f.size_dt_strings ||
> +           hashed_strings_start + hashed_strings_size > f.size_dt_strings) {
> +               pr_err("%s: hashed-strings too large\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       dt_struct = f.off_dt_struct;
> +       dt_strings = (void *)fdt + f.off_dt_strings;
> +
> +       end = path;
> +       *end = '\0';
> +
> +       do {
> +               const struct fdt_property *fdt_prop;
> +               const struct fdt_node_header *fnh;
> +               const char *name;
> +               int include = 0;
> +               int stop_at = 0;
> +               int offset = dt_struct;
> +               int maxlen, len;
> +
> +               tag = be32_to_cpu(*(uint32_t *)(fit + dt_struct));
> +
> +               switch (tag) {
> +               case FDT_BEGIN_NODE:
> +                       fnh = fit + dt_struct;
> +                       name = fnh->name;
> +                       maxlen = (unsigned long)fdt + f.off_dt_struct +
> +                               f.size_dt_struct - (unsigned long)name;
> +
> +                       len = strnlen(name, maxlen + 1);
> +                       if (len > maxlen)
> +                               return -ESPIPE;
> +
> +                       dt_struct = dt_struct_advance(&f, dt_struct,
> +                                                     sizeof(struct fdt_node_header) + len + 1);
> +
> +                       depth++;
> +                       if (depth == FDT_MAX_DEPTH)
> +                               return -ESPIPE;
> +                       if (end - path + 2 + len >= FDT_MAX_PATH_LEN)
> +                               return -ESPIPE;
> +                       if (end != path + 1)
> +                               *end++ = '/';
> +                       strcpy(end, name);
> +                       end += len;
> +                       stack[depth] = want;
> +                       if (want == 1)
> +                               stop_at = offset;
> +                       if (string_list_contains(inc_nodes, path))
> +                               want = 2;
> +                       else if (want)
> +                               want--;
> +                       else
> +                               stop_at = offset;
> +                       include = want;
> +
> +                       break;
> +
> +               case FDT_END_NODE:
> +                       dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE);
> +
> +                       include = want;
> +                       want = stack[depth--];
> +                       while (end > path && *--end != '/')
> +                               ;
> +                       *end = '\0';
> +
> +                       break;
> +
> +               case FDT_PROP:
> +                       fdt_prop = fit + dt_struct;
> +                       len = fdt32_to_cpu(fdt_prop->len);
> +
> +                       name = dt_string(&f, dt_strings, fdt32_to_cpu(fdt_prop->nameoff));
> +                       if (!name)
> +                               return -ESPIPE;
> +
> +                       dt_struct = dt_struct_advance(&f, dt_struct,
> +                                                     sizeof(struct fdt_property) + len);
> +
> +                       include = want >= 2;
> +                       stop_at = offset;
> +                       if (string_list_contains(exc_props, name))
> +                               include = 0;
> +
> +                       break;
> +
> +               case FDT_NOP:
> +                       dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE);
> +
> +                       include = want >= 2;
> +                       stop_at = offset;
> +
> +                       break;
> +
> +               case FDT_END:
> +                       dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE);
> +
> +                       include = 1;
> +
> +                       break;
> +
> +               default:
> +                       pr_err("%s: Unknown tag 0x%08X\n", __func__, tag);
> +                       return -EINVAL;
> +               }
> +
> +               if (!dt_struct)
> +                       return -ESPIPE;
> +
> +               pr_debug("%s: include %d, want %d, offset 0x%x, len 0x%x\n",
> +                        path, include, want, offset, dt_struct-offset);
> +
> +               if (include && start == -1)
> +                       start = offset;
> +
> +               if (!include && start != -1) {
> +                       pr_debug("region: 0x%p+0x%x\n", fit+start, offset-start);
> +                       digest_update(digest, fit+start, offset-start);
> +                       start = -1;
> +               }
> +       } while (tag != FDT_END);
> +
> +       pr_debug("region: 0x%p+0x%x\n", fit+start, dt_struct-start);
> +       digest_update(digest, fit+start, dt_struct-start);
> +
> +       pr_debug("strings: 0x%p+0x%x\n", dt_strings+hashed_strings_start, hashed_strings_size);
> +       digest_update(digest, dt_strings+hashed_strings_start, hashed_strings_size);
> +
> +       return 0;
> +}
> +
> +/*
> + * The consistency of the FTD structure was already checked by of_unflatten_dtb()
> + */
> +static int fit_verify_signature(struct device_node *sig_node, void *fit)
> +{
> +       uint32_t hashed_strings_start, hashed_strings_size;
> +       struct string_list inc_nodes, exc_props;
> +       struct rsa_public_key key = {};
> +       struct digest *digest;
> +       int sig_len;
> +       const char *algo_name, *key_name, *sig_value;
> +       char *key_path;
> +       struct device_node *key_node;
> +       enum hash_algo algo;
> +       void *hash;
> +       int ret;
> +
> +       if (of_property_read_string(sig_node, "algo", &algo_name)) {
> +               pr_err("algo not found\n");
> +               ret = -EINVAL;
> +               goto out;
> +       }
> +       if (strcmp(algo_name, "sha1,rsa2048") == 0) {
> +               algo = HASH_ALGO_SHA1;
> +       } else if (strcmp(algo_name, "sha256,rsa4096") == 0) {
> +               algo = HASH_ALGO_SHA256;
> +       } else  {
> +               pr_err("unknown algo %s\n", algo_name);
> +               ret = -EINVAL;
> +               goto out;
> +       }
> +       digest = digest_alloc_by_algo(algo);
> +       if (!digest) {
> +               pr_err("unsupported algo %s\n", algo_name);
> +               ret = -EINVAL;
> +               goto out;
> +       }
> +
> +       sig_value = of_get_property(sig_node, "value", &sig_len);
> +       if (!sig_value) {
> +               pr_err("signature value not found\n");
> +               ret = -EINVAL;
> +               goto out_free_digest;
> +       }
> +
> +       if (of_property_read_string(sig_node, "key-name-hint", &key_name)) {
> +               pr_err("key name not found\n");
> +               ret = -EINVAL;
> +               goto out_free_digest;
> +       }
> +       key_path = asprintf("/signature/key-%s", key_name);
> +       if (!key_name) {
> +               ret = -ENOMEM;
> +               goto out_free_digest;
> +       }
> +       key_node = of_find_node_by_path(key_path);
> +       free(key_path);
> +       if (!key_node) {
> +               pr_info("failed to find key node\n");
> +               ret = -ENOENT;
> +               goto out_free_digest;
> +       }
> +
> +       ret = rsa_of_read_key(key_node, &key);
> +       if (ret) {
> +               pr_info("failed to read key\n");
> +               ret = -ENOENT;
> +               goto out_free_digest;
> +       }
> +
> +       if (of_property_read_u32_index(sig_node, "hashed-strings", 0, &hashed_strings_start)) {
> +               pr_err("%s: hashed-strings start not found\n", __func__);
> +               ret = -EINVAL;
> +               goto out_free_digest;
> +       }
> +       if (of_property_read_u32_index(sig_node, "hashed-strings", 1, &hashed_strings_size)) {
> +               pr_err("%s: hashed-strings size not found\n", __func__);
> +               ret = -EINVAL;
> +               goto out_free_digest;
> +       }
> +
> +       string_list_init(&inc_nodes);
> +       string_list_init(&exc_props);
> +
> +       if (of_read_string_list(sig_node, "hashed-nodes", &inc_nodes)) {
> +               pr_err("%s: hashed-nodes invalid\n", __func__);
> +               ret = -EINVAL;
> +               goto out_sl;
> +       }
> +
> +       string_list_add(&exc_props, "data");
> +
> +       digest_init(digest);
> +       ret = fit_digest(fit, digest, &inc_nodes, &exc_props, hashed_strings_start, hashed_strings_size);
> +       hash = xzalloc(digest_length(digest));
> +       digest_final(digest, hash);
> +
> +       ret = rsa_verify(&key, sig_value, sig_len, hash, algo);
> +       if (ret) {
> +               pr_info("sig BAD\n");
> +               ret = CHECK_LEVEL_NONE;
> +       } else {
> +               pr_info("sig OK\n");
> +               ret = CHECK_LEVEL_SIG;
> +       }
> +
> +       free(hash);
> + out_sl:
> +       string_list_free(&inc_nodes);
> +       string_list_free(&exc_props);
> + out_free_digest:
> +       digest_free(digest);
> + out:
> +       return ret;
> +}
> +
> +static int fit_verify_hash(struct device_node *hash, const void *data, int data_len)
> +{
> +       struct digest *d;
> +       const char *algo;
> +       const char *value_read;
> +       char *value_calc;
> +       int hash_len;
> +
> +       value_read = of_get_property(hash, "value", &hash_len);
> +       if (!value_read) {
> +               pr_err("value not found\n");
> +               return CHECK_LEVEL_NONE;
> +       }
> +
> +       if (of_property_read_string(hash, "algo", &algo)) {
> +               pr_err("algo not found\n");
> +               return -EINVAL;
> +       }
> +
> +       d = digest_alloc(algo);
> +       if (!d) {
> +               pr_err("unsupported algo %s\n", algo);
> +               return -EINVAL;
> +       }
> +
> +       if (hash_len != digest_length(d)) {
> +               pr_err("invalid hash length %d\n", hash_len);
> +               digest_free(d);
> +               return -EINVAL;
> +       }
> +
> +       value_calc = xmalloc(hash_len);
> +
> +       digest_init(d);
> +       digest_update(d, data, data_len);
> +       digest_final(d, value_calc);
> +
> +       if (memcmp(value_read, value_calc, hash_len)) {
> +               pr_info("hash BAD\n");
> +               digest_free(d);
> +               return CHECK_LEVEL_NONE;
> +       } else {
> +               pr_info("hash OK\n");
> +               digest_free(d);
> +               return CHECK_LEVEL_HASH;
> +       }
> +}
> +
> +static int fit_open_image(struct fit_handle *handle, const char* unit)
> +{
> +       struct device_node *image = NULL, *hash;
> +       const char *type = NULL, *desc;
> +       const void *data;
> +       int data_len;
> +       int ret, level;
> +
> +       image = of_get_child_by_name(handle->root, "images");
> +       if (!image)
> +               return -ENOENT;
> +
> +       image = of_get_child_by_name(image, unit);
> +       if (!image)
> +               return -ENOENT;
> +
> +       if (of_property_read_string(image, "description", &desc)) {
> +               pr_info("FIT image '%s' (no description)\n", unit);
> +       } else {
> +               pr_info("FIT image '%s': '%s'\n", unit, desc);
> +       }
> +
> +       of_property_read_string(image, "type", &type);
> +       if (!type)
> +               return -EINVAL;
> +
> +       data = of_get_property(image, "data", &data_len);
> +       if (!data) {
> +               pr_err("data not found\n");
> +               return -EINVAL;
> +       }
> +
> +       level = CHECK_LEVEL_MAX;
> +       for_each_child_of_node(image, hash) {
> +               if (handle->verbose)
> +                       of_print_nodes(hash, 0);
> +               ret = fit_verify_hash(hash, data, data_len);
> +               if (ret < 0)
> +                       return ret;
> +               level = min(level, ret);
> +       }
> +       if (level == CHECK_LEVEL_MAX) {
> +               return -EINVAL;
> +       }
> +
> +       if (level == CHECK_LEVEL_HASH) {
> +               if (strcmp(type, "kernel") == 0 ||
> +                   strcmp(type, "kernel_noload") == 0) {
> +                       handle->kernel = data;
> +                       handle->kernel_size = data_len;
> +               } else if (strcmp(type, "flat_dt") == 0) {
> +                       handle->oftree = data;
> +                       handle->oftree_size = data_len;
> +               } else if (strcmp(type, "ramdisk") == 0) {
> +                       handle->initrd = data;
> +                       handle->initrd_size = data_len;
> +               } else {
> +                       pr_info("unknown image type %s, ignoring\n", type);
> +               }
> +       }
> +
> +       return level;
> +}
> +
> +static int fit_open_configuration(struct fit_handle *handle, int num)
> +{
> +       struct device_node *conf_node = NULL, *sig_node;
> +       char unit_name[10];
> +       const char *unit, *desc;
> +       int ret, level;
> +
> +       conf_node = of_get_child_by_name(handle->root, "configurations");
> +       if (!conf_node)
> +               return -ENOENT;
> +
> +       if (num) {
> +               snprintf(unit_name, sizeof(unit_name), "conf@%d", num);

This is not working for my *.its file:
https://github.com/visionsystemsgmbh/onrisc_br_bsp/blob/master/board/vscom/baltos/kernel-fit.its
U-Boot is working with bootm ${loadaddr}#conf${board_name}

For Barebox I've changed this line to

snprintf(unit_name, sizeof(unit_name), "conf%d@1", num)

This is how I start Linux: bootm /boot/kernel-fit.itb@$global.board.id

What is the standard for providing FIT configuration?

> +               unit = unit_name;
> +       } else if (of_property_read_string(conf_node, "default", &unit)) {
> +               unit = "conf@1";
> +       }
> +
> +       conf_node = of_get_child_by_name(conf_node, unit);
> +       if (!conf_node) {
> +               pr_err("FIT configuration '%s' not found\n", unit);
> +               return -ENOENT;
> +       }
> +
> +       if (of_property_read_string(conf_node, "description", &desc)) {
> +               pr_info("FIT configuration '%s' (no description)\n", unit);
> +       } else {
> +               pr_info("FIT configuration '%s': '%s'\n", unit, desc);
> +       }
> +
> +       level = CHECK_LEVEL_MAX;
> +       for_each_child_of_node(conf_node, sig_node) {
> +               if (handle->verbose)
> +                       of_print_nodes(sig_node, 0);
> +               ret = fit_verify_signature(sig_node, handle->fit);
> +               if (ret < 0)
> +                       return ret;
> +               level = min(level, ret);
> +       }
> +       if (level == CHECK_LEVEL_MAX)
> +               return -EINVAL;
> +
> +       if (level != CHECK_LEVEL_SIG)
> +               return -EINVAL;
> +
> +       if (of_property_read_string(conf_node, "kernel", &unit) == 0)
> +               level = min(level, fit_open_image(handle, unit));
> +       else
> +               return -ENOENT;
> +
> +       if (of_property_read_string(conf_node, "fdt", &unit) == 0)
> +               level = min(level, fit_open_image(handle, unit));
> +
> +       if (of_property_read_string(conf_node, "ramdisk", &unit) == 0)
> +               level = min(level, fit_open_image(handle, unit));
> +
> +       if (level != CHECK_LEVEL_HASH)
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
> +struct fit_handle *fit_open(const char *filename, int num, bool verbose)
> +{
> +       struct fit_handle *handle = NULL;
> +       const char *desc;
> +
> +       handle = xzalloc(sizeof(struct fit_handle));
> +
> +       handle->verbose = verbose;
> +
> +       handle->fit = read_file(filename, &handle->size);
> +       if (!handle->fit) {
> +               pr_err("unable to read %s: %s\n", filename, strerror(errno));
> +               goto err;
> +       }
> +
> +       handle->root = of_unflatten_dtb(handle->fit);
> +       if (IS_ERR(handle->root)) {
> +               goto err;
> +       }
> +
> +       if (of_property_read_string(handle->root, "description", &desc)) {
> +               pr_info("FIT '%s' (no description)\n", filename);
> +       } else {
> +               pr_info("FIT '%s': '%s'\n", filename, desc);
> +       }
> +
> +       if (fit_open_configuration(handle, num))
> +               goto err;
> +
> +       return handle;
> + err:
> +       if (handle->root)
> +               of_delete_node(handle->root);
> +       if (handle->fit)
> +               free(handle->fit);
> +       free(handle);
> +
> +       return NULL;
> +}
> +
> +void fit_close(struct fit_handle *handle)
> +{
> +       if (handle->root)
> +               of_delete_node(handle->root);
> +       if (handle->fit)
> +               free(handle->fit);
> +       free(handle);
> +}
> +
> +#ifdef CONFIG_SANDBOX
> +static int do_bootm_sandbox_fit(struct image_data *data)
> +{
> +       struct fit_handle *handle;
> +       handle = fit_open(data->os_file, data->os_num, data->verbose);
> +       if (handle)
> +               fit_close(handle);
> +       return 0;
> +}
> +
> +static struct image_handler sandbox_fit_handler = {
> +       .name = "FIT image",
> +       .bootm = do_bootm_sandbox_fit,
> +       .filetype = filetype_oftree,
> +};
> +
> +static int sandbox_fit_register(void)
> +{
> +       return register_image_handler(&sandbox_fit_handler);
> +}
> +late_initcall(sandbox_fit_register);
> +#endif
> diff --git a/include/image-fit.h b/include/image-fit.h
> new file mode 100644
> index 000000000000..bcbc859ead37
> --- /dev/null
> +++ b/include/image-fit.h
> @@ -0,0 +1,42 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + * Copyright (C) Jan Lübbe, 2014
> + */
> +
> +#ifndef __IMAGE_FIT_H__
> +#define __IMAGE_FIT_H__
> +
> +#include <linux/types.h>
> +
> +struct fit_handle {
> +       void *fit;
> +       size_t size;
> +
> +       bool verbose;
> +
> +       struct device_node *root;
> +
> +       const void *kernel;
> +       unsigned long kernel_size;
> +       const void *oftree;
> +       unsigned long oftree_size;
> +       const void *initrd;
> +       unsigned long initrd_size;
> +};
> +
> +struct fit_handle *fit_open(const char *filename, int num, bool verbose);
> +void fit_close(struct fit_handle *handle);
> +
> +#endif /* __IMAGE_FIT_H__ */
> --
> 2.6.2
>

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

Re: [PATCH 3/3] bootm: add initial FIT support

[Marc Kleine-Budde]

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

On 01/05/2016 11:28 AM, Yegor Yefremov wrote:
> Hi Marc,
> 
> thanks for reposting the patches.
> 
> On Tue, Jan 5, 2016 at 9:11 AM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>> From: Jan Luebbe <jlu@pengutronix.de>
>>
>> This implementation is inspired by U-Boot's FIT support. Instead of
>> using libfdt (which does not exist in barebox), configuration signatures
>> are verified by using a simplified DT parser based on barebox's own
>> code.
>>
>> Currently, only signed configurations with hashed images are supported,
>> as the other variants are less useful for verified boot. Compatible FIT
>> images can be created using U-Boot's mkimage tool.
> 
> What about unsigned images?

That's not our use case. We use plain zImages instead.

> I also get: unsupported algo crc32
> Is it intended to be supported?

Not for our usecase - feel free to add crc32 support.

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 3/3] bootm: add initial FIT support

[Yegor Yefremov]

On Tue, Jan 5, 2016 at 11:32 AM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 01/05/2016 11:28 AM, Yegor Yefremov wrote:
>> Hi Marc,
>>
>> thanks for reposting the patches.
>>
>> On Tue, Jan 5, 2016 at 9:11 AM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>>> From: Jan Luebbe <jlu@pengutronix.de>
>>>
>>> This implementation is inspired by U-Boot's FIT support. Instead of
>>> using libfdt (which does not exist in barebox), configuration signatures
>>> are verified by using a simplified DT parser based on barebox's own
>>> code.
>>>
>>> Currently, only signed configurations with hashed images are supported,
>>> as the other variants are less useful for verified boot. Compatible FIT
>>> images can be created using U-Boot's mkimage tool.
>>
>> What about unsigned images?
>
> That's not our use case. We use plain zImages instead.

The solution would be to introduce an option like in U-Boot?

CONFIG_FIT_SIGNATURE:

This option enables signature verification of FIT uImages,
using a hash signed and verified using RSA. If
CONFIG_SHA_PROG_HW_ACCEL is defined, i.e support for progressive
hashing is available using hardware, RSA library will use it.
See doc/uImage.FIT/signature.txt for more details.

>> I also get: unsupported algo crc32
>> Is it intended to be supported?
>
> Not for our usecase - feel free to add crc32 support.

OK.

But what about FIT configuration selection syntax?

Yegor

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

Re: [PATCH 3/3] bootm: add initial FIT support

[Marc Kleine-Budde]

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

On 01/05/2016 11:40 AM, Yegor Yefremov wrote:
> On Tue, Jan 5, 2016 at 11:32 AM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>> On 01/05/2016 11:28 AM, Yegor Yefremov wrote:
>>> Hi Marc,
>>>
>>> thanks for reposting the patches.
>>>
>>> On Tue, Jan 5, 2016 at 9:11 AM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>>>> From: Jan Luebbe <jlu@pengutronix.de>
>>>>
>>>> This implementation is inspired by U-Boot's FIT support. Instead of
>>>> using libfdt (which does not exist in barebox), configuration signatures
>>>> are verified by using a simplified DT parser based on barebox's own
>>>> code.
>>>>
>>>> Currently, only signed configurations with hashed images are supported,
>>>> as the other variants are less useful for verified boot. Compatible FIT
>>>> images can be created using U-Boot's mkimage tool.
>>>
>>> What about unsigned images?
>>
>> That's not our use case. We use plain zImages instead.
> 
> The solution would be to introduce an option like in U-Boot?
> 
> CONFIG_FIT_SIGNATURE:
> 
> This option enables signature verification of FIT uImages,
> using a hash signed and verified using RSA. If
> CONFIG_SHA_PROG_HW_ACCEL is defined, i.e support for progressive
> hashing is available using hardware, RSA library will use it.
> See doc/uImage.FIT/signature.txt for more details.

Technically possible, but I'm not sure what are the benefits of using
fit images, if you don't need signatures. barebox implements
freedesktop.org's bootspec and this is IMHO the way to go.

>>> I also get: unsupported algo crc32
>>> Is it intended to be supported?
>>
>> Not for our usecase - feel free to add crc32 support.
> 
> OK.
> 
> But what about FIT configuration selection syntax?

What's this?

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 3/3] bootm: add initial FIT support

[Yegor Yefremov]

On Tue, Jan 5, 2016 at 12:54 PM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 01/05/2016 11:40 AM, Yegor Yefremov wrote:
>> On Tue, Jan 5, 2016 at 11:32 AM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>>> On 01/05/2016 11:28 AM, Yegor Yefremov wrote:
>>>> Hi Marc,
>>>>
>>>> thanks for reposting the patches.
>>>>
>>>> On Tue, Jan 5, 2016 at 9:11 AM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>>>>> From: Jan Luebbe <jlu@pengutronix.de>
>>>>>
>>>>> This implementation is inspired by U-Boot's FIT support. Instead of
>>>>> using libfdt (which does not exist in barebox), configuration signatures
>>>>> are verified by using a simplified DT parser based on barebox's own
>>>>> code.
>>>>>
>>>>> Currently, only signed configurations with hashed images are supported,
>>>>> as the other variants are less useful for verified boot. Compatible FIT
>>>>> images can be created using U-Boot's mkimage tool.
>>>>
>>>> What about unsigned images?
>>>
>>> That's not our use case. We use plain zImages instead.
>>
>> The solution would be to introduce an option like in U-Boot?
>>
>> CONFIG_FIT_SIGNATURE:
>>
>> This option enables signature verification of FIT uImages,
>> using a hash signed and verified using RSA. If
>> CONFIG_SHA_PROG_HW_ACCEL is defined, i.e support for progressive
>> hashing is available using hardware, RSA library will use it.
>> See doc/uImage.FIT/signature.txt for more details.
>
> Technically possible, but I'm not sure what are the benefits of using
> fit images, if you don't need signatures. barebox implements
> freedesktop.org's bootspec and this is IMHO the way to go.

For me FIT is just a way to have a kernel and a bunch of device tree
blobs in one file. Signed or not signed is an option for me. Just like
U-Boot implements it. This is user responsibility.

In my use case I just read device ID from EEPROM, load my
kernel-fit.itb and select needed DTB via this ID. This way I have only
one SD card image, that can be run on more, than 10 different devices
using the same core module.

>>>> I also get: unsupported algo crc32
>>>> Is it intended to be supported?
>>>
>>> Not for our usecase - feel free to add crc32 support.
>>
>> OK.
>>
>> But what about FIT configuration selection syntax?
>
> What's this?

Have you seen my comments to this patch regarding
fit_open_configuration() routine?

http://lists.infradead.org/pipermail/barebox/2016-January/025718.html

Yegor

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

Re: [PATCH 3/3] bootm: add initial FIT support

[Marc Kleine-Budde]

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

On 01/05/2016 02:05 PM, Yegor Yefremov wrote:
>>>>> What about unsigned images?
>>>>
>>>> That's not our use case. We use plain zImages instead.
>>>
>>> The solution would be to introduce an option like in U-Boot?
>>>
>>> CONFIG_FIT_SIGNATURE:
>>>
>>> This option enables signature verification of FIT uImages,
>>> using a hash signed and verified using RSA. If
>>> CONFIG_SHA_PROG_HW_ACCEL is defined, i.e support for progressive
>>> hashing is available using hardware, RSA library will use it.
>>> See doc/uImage.FIT/signature.txt for more details.
>>
>> Technically possible, but I'm not sure what are the benefits of using
>> fit images, if you don't need signatures. barebox implements
>> freedesktop.org's bootspec and this is IMHO the way to go.
> 
> For me FIT is just a way to have a kernel and a bunch of device tree
> blobs in one file. Signed or not signed is an option for me. Just like
> U-Boot implements it. This is user responsibility.

Send patches. :D

> In my use case I just read device ID from EEPROM, load my
> kernel-fit.itb and select needed DTB via this ID. This way I have only
> one SD card image, that can be run on more, than 10 different devices
> using the same core module.

>>>>> I also get: unsupported algo crc32
>>>>> Is it intended to be supported?
>>>>
>>>> Not for our usecase - feel free to add crc32 support.
>>>
>>> OK.
>>>
>>> But what about FIT configuration selection syntax?
>>
>> What's this?
> 
> Have you seen my comments to this patch regarding
> fit_open_configuration() routine?

sorry - I've missed that. Too many quoted lines. :D

>> > +static int fit_open_configuration(struct fit_handle *handle, int num)
>> > +{
>> > +       struct device_node *conf_node = NULL, *sig_node;
>> > +       char unit_name[10];
>> > +       const char *unit, *desc;
>> > +       int ret, level;
>> > +
>> > +       conf_node = of_get_child_by_name(handle->root, "configurations");
>> > +       if (!conf_node)
>> > +               return -ENOENT;
>> > +
>> > +       if (num) {
>> > +               snprintf(unit_name, sizeof(unit_name), "conf@%d", num);
> 
> This is not working for my *.its file:
> https://github.com/visionsystemsgmbh/onrisc_br_bsp/blob/master/board/vscom/baltos/kernel-fit.its
> U-Boot is working with bootm ${loadaddr}#conf${board_name}
> 
> For Barebox I've changed this line to
> 
> snprintf(unit_name, sizeof(unit_name), "conf%d@1", num)
> 
> This is how I start Linux: bootm /boot/kernel-fit.itb@$global.board.id
> 
> What is the standard for providing FIT configuration?

Don't know. Is there a spec in the u-boot sources, otherwise use the code.

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 3/3] bootm: add initial FIT support

[Yegor Yefremov]

On Tue, Jan 5, 2016 at 2:50 PM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 01/05/2016 02:05 PM, Yegor Yefremov wrote:
>>>>>> What about unsigned images?
>>>>>
>>>>> That's not our use case. We use plain zImages instead.
>>>>
>>>> The solution would be to introduce an option like in U-Boot?
>>>>
>>>> CONFIG_FIT_SIGNATURE:
>>>>
>>>> This option enables signature verification of FIT uImages,
>>>> using a hash signed and verified using RSA. If
>>>> CONFIG_SHA_PROG_HW_ACCEL is defined, i.e support for progressive
>>>> hashing is available using hardware, RSA library will use it.
>>>> See doc/uImage.FIT/signature.txt for more details.
>>>
>>> Technically possible, but I'm not sure what are the benefits of using
>>> fit images, if you don't need signatures. barebox implements
>>> freedesktop.org's bootspec and this is IMHO the way to go.
>>
>> For me FIT is just a way to have a kernel and a bunch of device tree
>> blobs in one file. Signed or not signed is an option for me. Just like
>> U-Boot implements it. This is user responsibility.
>
> Send patches. :D

I'll prepare one on top of yours. So far I can boot into Linux on my
am335x based board, so

Tested-by: Yegor Yefremov <yegorslists@googlemail.com>

>> In my use case I just read device ID from EEPROM, load my
>> kernel-fit.itb and select needed DTB via this ID. This way I have only
>> one SD card image, that can be run on more, than 10 different devices
>> using the same core module.
>
>>>>>> I also get: unsupported algo crc32
>>>>>> Is it intended to be supported?
>>>>>
>>>>> Not for our usecase - feel free to add crc32 support.
>>>>
>>>> OK.
>>>>
>>>> But what about FIT configuration selection syntax?
>>>
>>> What's this?
>>
>> Have you seen my comments to this patch regarding
>> fit_open_configuration() routine?
>
> sorry - I've missed that. Too many quoted lines. :D
>
>>> > +static int fit_open_configuration(struct fit_handle *handle, int num)
>>> > +{
>>> > +       struct device_node *conf_node = NULL, *sig_node;
>>> > +       char unit_name[10];
>>> > +       const char *unit, *desc;
>>> > +       int ret, level;
>>> > +
>>> > +       conf_node = of_get_child_by_name(handle->root, "configurations");
>>> > +       if (!conf_node)
>>> > +               return -ENOENT;
>>> > +
>>> > +       if (num) {
>>> > +               snprintf(unit_name, sizeof(unit_name), "conf@%d", num);
>>
>> This is not working for my *.its file:
>> https://github.com/visionsystemsgmbh/onrisc_br_bsp/blob/master/board/vscom/baltos/kernel-fit.its
>> U-Boot is working with bootm ${loadaddr}#conf${board_name}
>>
>> For Barebox I've changed this line to
>>
>> snprintf(unit_name, sizeof(unit_name), "conf%d@1", num)
>>
>> This is how I start Linux: bootm /boot/kernel-fit.itb@$global.board.id
>>
>> What is the standard for providing FIT configuration?
>
> Don't know. Is there a spec in the u-boot sources, otherwise use the code.

Will look closer at U-Boot code and send a patch.

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

Re: [PATCH 1/3] crypto: add enum

[Sam Ravnborg]

On Tue, Jan 05, 2016 at 09:11:01AM +0100, Marc Kleine-Budde wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

The subject and the patch description could use some
more love and care.


> diff --git a/crypto/digest.c b/crypto/digest.c
> index a90e4ff79f89..46600f246ece 100644
> --- a/crypto/digest.c
> +++ b/crypto/digest.c
> @@ -116,7 +116,27 @@ static struct digest_algo *digest_algo_get_by_name(const char *name)
>  	list_for_each_entry(tmp, &digests, list) {
>  		if (strcmp(tmp->base.name, name) != 0)
>  			continue;
> -		
> +
> +		if (tmp->base.priority <= priority)
> +			continue;
> +
> +		d = tmp;
> +		priority = tmp->base.priority;
> +	}
> +
> +	return d;
> +}
> +
> +static struct digest_algo *digest_algo_get_by_algo(enum hash_algo algo)
> +{
> +	struct digest_algo *d = NULL;
> +	struct digest_algo *tmp;
> +	int priority = -1;
> +
> +	list_for_each_entry(tmp, &digests, list) {
> +		if (tmp->base.algo != algo)
> +			continue;
> +
>  		if (tmp->base.priority <= priority)
>  			continue;
>  
> @@ -160,6 +180,27 @@ struct digest *digest_alloc(const char *name)
>  }
>  EXPORT_SYMBOL_GPL(digest_alloc);
>  
> +struct digest *digest_alloc_by_algo(enum hash_algo hash_algo)
> +{
> +	struct digest *d;
> +	struct digest_algo *algo;
> +
> +	algo = digest_algo_get_by_algo(hash_algo);
> +	if (!algo)
> +		return NULL;
> +
> +	d = xzalloc(sizeof(*d));
> +	d->algo = algo;
> +	d->ctx = xzalloc(algo->ctx_length);

Neither allocations are checked for failure.

	Sam

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

Re: [PATCH 3/3] bootm: add initial FIT support

[Trent Piepho]

On Tue, 2016-01-05 at 09:11 +0100, Marc Kleine-Budde wrote:
> +static int do_bootm_arm_fit(struct image_data *data)
> +{
> +	struct fit_handle *handle;
> +	int ret;
> +	unsigned long mem_free;
> +	unsigned long mem_start, mem_size;
> +
> +	handle = fit_open(data->os_file, data->os_num, data->verbose);
> +	if (!handle)
> +		return -EINVAL;
> +
> +	ret = sdram_start_and_size(&mem_start, &mem_size);
> +	if (ret)
> +		return ret;
> +
> +	/* no support for custom load address */
> +	data->os_address = mem_start + PAGE_ALIGN(handle->kernel_size * 4);
> +	data->os_res = request_sdram_region("fit-kernel", data->os_address, handle->kernel_size);
> +	if (!data->os_res) {
> +		pr_err("Cannot request region 0x%08lx - 0x%08lx\n",
> +				data->os_address, handle->kernel_size);
> +		ret = -ENOMEM;
> +		goto err_out;
> +	}
> +	memcpy((void *)data->os_res->start, handle->kernel, handle->kernel_size);
> +
> +	/*
> +	 * Put oftree/initrd close behind compressed kernel image to avoid
> +	 * placing it outside of the kernels lowmem.
> +	 */
> +	if (handle->initrd_size) {
> +		data->initrd_address = PAGE_ALIGN(data->os_res->end + SZ_1M);
> +		data->initrd_res = request_sdram_region("fit-initrd", data->initrd_address, handle->initrd_size);
> +		if (!data->initrd_res) {
> +			ret = -ENOMEM;
> +			goto err_out;
> +		}
> +		memcpy((void *)data->initrd_res->start, handle->initrd, handle->initrd_size);
> +	}
> +
> +	data->of_root_node = of_unflatten_dtb(handle->oftree);
> +	if (!data->of_root_node) {
> +		pr_err("unable to unflatten devicetree\n");
> +		ret = -EINVAL;
> +		goto err_out;
> +	}
> +
> +	/*
> +	 * Put devicetree right after initrd if present or after the kernel
> +	 * if not.
> +	 */
> +	if (data->initrd_res)
> +		mem_free = PAGE_ALIGN(data->initrd_res->end);
> +	else
> +		mem_free = PAGE_ALIGN(data->os_res->end + SZ_1M);
Why the extra 1M?
> +
> +	return __do_bootm_linux(data, mem_free, 0);
> +
> +err_out:
> +	if (handle)
> +		fit_close(handle);

handle can't be NULL, it's been dereferenced in every path that gets
there.

> +	return ret;
> +}
> +
> +static struct image_handler arm_fit_handler = {

Can this be const?

> +        .name = "FIT image",
> +        .bootm = do_bootm_arm_fit,
> +        .filetype = filetype_oftree,
> +};
> +
>  static struct binfmt_hook binfmt_aimage_hook = {
>  	.type = filetype_aimage,
>  	.exec = "bootm",
> @@ -578,6 +650,8 @@ static int armlinux_register_image_handler(void)
>  		register_image_handler(&aimage_handler);
>  		binfmt_register(&binfmt_aimage_hook);
>  	}
> +	if (IS_BUILTIN(CONFIG_CMD_BOOTM_FITIMAGE))
> +	        register_image_handler(&arm_fit_handler);
>  	binfmt_register(&binfmt_arm_zimage_hook);
>  	binfmt_register(&binfmt_barebox_hook);
>  
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 1743670ed33c..b89627209f5a 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -418,6 +418,14 @@ config CMD_BOOTM_AIMAGE
>  	help
>  	  Support using Android Images.
>  
> +config CMD_BOOTM_FITIMAGE
> +	bool
> +	prompt "FIT image support"
> +	select FITIMAGE
> +	depends on CMD_BOOTM && ARM
> +	help
> +	  Support using FIT Images.

Perhaps a link about FIT images or a pointer to a file in Documentation
could go here?


> +/*
> + * The consistency of the FTD structure was already checked by of_unflatten_dtb()
> + */
> +static int fit_verify_signature(struct device_node *sig_node, void *fit)
> +{
> +	uint32_t hashed_strings_start, hashed_strings_size;
> +	struct string_list inc_nodes, exc_props;
> +	struct rsa_public_key key = {};
> +	struct digest *digest;
> +	int sig_len;
> +	const char *algo_name, *key_name, *sig_value;
> +	char *key_path;
> +	struct device_node *key_node;
> +	enum hash_algo algo;
> +	void *hash;
> +	int ret;
> +
> +	if (of_property_read_string(sig_node, "algo", &algo_name)) {
> +		pr_err("algo not found\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +	if (strcmp(algo_name, "sha1,rsa2048") == 0) {
> +		algo = HASH_ALGO_SHA1;
> +	} else if (strcmp(algo_name, "sha256,rsa4096") == 0) {
> +		algo = HASH_ALGO_SHA256;
> +	} else	{
> +		pr_err("unknown algo %s\n", algo_name);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +	digest = digest_alloc_by_algo(algo);
> +	if (!digest) {
> +		pr_err("unsupported algo %s\n", algo_name);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	sig_value = of_get_property(sig_node, "value", &sig_len);
> +	if (!sig_value) {
> +		pr_err("signature value not found\n");
> +		ret = -EINVAL;
> +		goto out_free_digest;
> +	}
> +
> +	if (of_property_read_string(sig_node, "key-name-hint", &key_name)) {
> +		pr_err("key name not found\n");
> +		ret = -EINVAL;
> +		goto out_free_digest;
> +	}
> +	key_path = asprintf("/signature/key-%s", key_name);

If I understand this correctly, one computes a hash of part of the
device tree and then verifies it against a signature, also in the device
tree, using an RSA key, also in the device tree.

What's the point of that?  Isn't it basically a complex CRC check for
internal consistency?

> +	if (!key_name) {
> +		ret = -ENOMEM;
> +		goto out_free_digest;
> +	}
> +	key_node = of_find_node_by_path(key_path);
> +	free(key_path);
> +	if (!key_node) {
> +		pr_info("failed to find key node\n");
> +		ret = -ENOENT;
> +		goto out_free_digest;
> +	}
> +
> +	ret = rsa_of_read_key(key_node, &key);
> +	if (ret) {
> +		pr_info("failed to read key\n");
> +		ret = -ENOENT;
> +		goto out_free_digest;
> +	}
> +
> +	if (of_property_read_u32_index(sig_node, "hashed-strings", 0, &hashed_strings_start)) {
> +		pr_err("%s: hashed-strings start not found\n", __func__);
> +		ret = -EINVAL;
> +		goto out_free_digest;
> +	}
> +	if (of_property_read_u32_index(sig_node, "hashed-strings", 1, &hashed_strings_size)) {
> +		pr_err("%s: hashed-strings size not found\n", __func__);
> +		ret = -EINVAL;
> +		goto out_free_digest;
> +	}
> +
> +	string_list_init(&inc_nodes);
> +	string_list_init(&exc_props);
> +
> +	if (of_read_string_list(sig_node, "hashed-nodes", &inc_nodes)) {
> +		pr_err("%s: hashed-nodes invalid\n", __func__);
> +		ret = -EINVAL;
> +		goto out_sl;
> +	}
> +
> +	string_list_add(&exc_props, "data");
> +
> +	digest_init(digest);
> +	ret = fit_digest(fit, digest, &inc_nodes, &exc_props, hashed_strings_start, hashed_strings_size);
> +	hash = xzalloc(digest_length(digest));
> +	digest_final(digest, hash);
> +
> +	ret = rsa_verify(&key, sig_value, sig_len, hash, algo);
> +	if (ret) {
> +		pr_info("sig BAD\n");
> +		ret = CHECK_LEVEL_NONE;

Info level message should maybe be a bit more descriptive.

> +	} else {
> +		pr_info("sig OK\n");
> +		ret = CHECK_LEVEL_SIG;
> +	}
> +
> +	free(hash);
> + out_sl:
> +	string_list_free(&inc_nodes);
> +	string_list_free(&exc_props);
> + out_free_digest:
> +	digest_free(digest);
> + out:
> +	return ret;
> +}
> +
> +static int fit_verify_hash(struct device_node *hash, const void *data, int data_len)
> +{
> +	struct digest *d;
> +	const char *algo;
> +	const char *value_read;
> +	char *value_calc;
> +	int hash_len;
> +
> +	value_read = of_get_property(hash, "value", &hash_len);
> +	if (!value_read) {
> +		pr_err("value not found\n");
> +		return CHECK_LEVEL_NONE;

Suggest adding node path to error messages.  "value not found" really
doesn't tell anyone anything.

This is an error message, but returns CHECK_LEVEL_NONE.  While later...

> +
> +	if (memcmp(value_read, value_calc, hash_len)) {
> +		pr_info("hash BAD\n");
> +		digest_free(d);
> +		return CHECK_LEVEL_NONE;

Now it's an info message and returns CHECK_LEVEL_NONE.  Seems wrong to
return the same value for an error and a non-error.

> +	} else {
> +		pr_info("hash OK\n");
> +		digest_free(d);
> +		return CHECK_LEVEL_HASH;
> +	}
> +}
> +
> +static int fit_open_image(struct fit_handle *handle, const char* unit)
> +{
> +	struct device_node *image = NULL, *hash;
> +	const char *type = NULL, *desc;
> +	const void *data;
> +	int data_len;
> +	int ret, level;
> +
> +	image = of_get_child_by_name(handle->root, "images");
> +	if (!image)
> +		return -ENOENT;
> +
> +	image = of_get_child_by_name(image, unit);
> +	if (!image)
> +		return -ENOENT;
> +
> +	if (of_property_read_string(image, "description", &desc)) {
Here you check the return value of of_property_read_string()
> +		pr_info("FIT image '%s' (no description)\n", unit);
> +	} else {
> +		pr_info("FIT image '%s': '%s'\n", unit, desc);
> +	}

Suggest:
         desc = "(no description)";
         of_property_read_string(image, "description", &desc);
         pr_info("FIT image '%s': '%s'\n", unit, desc);
         /* desc has valid value if anyone uses it again, instead of
            being uninitialized */
> +
> +	of_property_read_string(image, "type", &type);
> +	if (!type)
> +		return -EINVAL;

But here you check that type is non-NULL.

> +
> +	data = of_get_property(image, "data", &data_len);
> +	if (!data) {
> +		pr_err("data not found\n");
> +		return -EINVAL;
> +	}
> +
> +	level = CHECK_LEVEL_MAX;
> +	for_each_child_of_node(image, hash) {
> +		if (handle->verbose)
> +			of_print_nodes(hash, 0);
> +		ret = fit_verify_hash(hash, data, data_len);
> +		if (ret < 0)
> +			return ret;
> +		level = min(level, ret);
> +	}
> +	if (level == CHECK_LEVEL_MAX) {
> +		return -EINVAL;
> +	}
Inconsistent use of {} for one statement then clause.

> +
> +	if (level == CHECK_LEVEL_HASH) {
> +		if (strcmp(type, "kernel") == 0 ||
> +		    strcmp(type, "kernel_noload") == 0) {
> +			handle->kernel = data;
> +			handle->kernel_size = data_len;
> +		} else if (strcmp(type, "flat_dt") == 0) {
> +			handle->oftree = data;
> +			handle->oftree_size = data_len;
> +		} else if (strcmp(type, "ramdisk") == 0) {
> +			handle->initrd = data;
> +			handle->initrd_size = data_len;
> +		} else {
> +			pr_info("unknown image type %s, ignoring\n", type);
> +		}
> +	}
> +
> +	return level;
> +}
> +
> +static int fit_open_configuration(struct fit_handle *handle, int num)
> +{
> +	struct device_node *conf_node = NULL, *sig_node;
> +	char unit_name[10];
> +	const char *unit, *desc;
> +	int ret, level;
> +
> +	conf_node = of_get_child_by_name(handle->root, "configurations");
> +	if (!conf_node)
> +		return -ENOENT;
> +
> +	if (num) {
> +		snprintf(unit_name, sizeof(unit_name), "conf@%d", num);
> +		unit = unit_name;
> +	} else if (of_property_read_string(conf_node, "default", &unit)) {
> +		unit = "conf@1";
> +	}
> +
> +	conf_node = of_get_child_by_name(conf_node, unit);
> +	if (!conf_node) {
> +		pr_err("FIT configuration '%s' not found\n", unit);
> +		return -ENOENT;
> +	}
> +
> +	if (of_property_read_string(conf_node, "description", &desc)) {
> +		pr_info("FIT configuration '%s' (no description)\n", unit);
> +	} else {
> +		pr_info("FIT configuration '%s': '%s'\n", unit, desc);
> +	}
> +
> +	level = CHECK_LEVEL_MAX;
> +	for_each_child_of_node(conf_node, sig_node) {
> +		if (handle->verbose)
> +			of_print_nodes(sig_node, 0);
> +		ret = fit_verify_signature(sig_node, handle->fit);
> +		if (ret < 0)
> +			return ret;
> +		level = min(level, ret);
> +	}
> +	if (level == CHECK_LEVEL_MAX)
> +		return -EINVAL;

This function up to here seems very similar to fit_open_image().  Could
they be refactored, Ie. _fit_open(handle, "images", unit) vs
_fit_open(handle, "configurations", "conf@1").

> +
> +	if (level != CHECK_LEVEL_SIG)
> +		return -EINVAL;
> +
> +	if (of_property_read_string(conf_node, "kernel", &unit) == 0)
> +		level = min(level, fit_open_image(handle, unit));
> +	else
> +		return -ENOENT;
> +
> +	if (of_property_read_string(conf_node, "fdt", &unit) == 0)
> +		level = min(level, fit_open_image(handle, unit));
> +
> +	if (of_property_read_string(conf_node, "ramdisk", &unit) == 0)
> +		level = min(level, fit_open_image(handle, unit));
> +
> +	if (level != CHECK_LEVEL_HASH)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +struct fit_handle *fit_open(const char *filename, int num, bool verbose)
> +{
> +	struct fit_handle *handle = NULL;
> +	const char *desc;
> +
> +	handle = xzalloc(sizeof(struct fit_handle));
> +
> +	handle->verbose = verbose;
> +
> +	handle->fit = read_file(filename, &handle->size);
> +	if (!handle->fit) {
> +		pr_err("unable to read %s: %s\n", filename, strerror(errno));
> +		goto err;
> +	}
> +
> +	handle->root = of_unflatten_dtb(handle->fit);
> +	if (IS_ERR(handle->root)) {
> +		goto err;
> +	}
Use of {} for one statement if clauses is inconsistent in this function.

> +
> +	if (of_property_read_string(handle->root, "description", &desc)) {
> +		pr_info("FIT '%s' (no description)\n", filename);
> +	} else {
> +		pr_info("FIT '%s': '%s'\n", filename, desc);
> +	}
> +
> +	if (fit_open_configuration(handle, num))
> +		goto err;
> +
> +	return handle;
> + err:
> +	if (handle->root)
> +		of_delete_node(handle->root);
> +	if (handle->fit)
> +		free(handle->fit);
> +	free(handle);
> +
> +	return NULL;
> +}
> +
> +void fit_close(struct fit_handle *handle)
> +{
> +	if (handle->root)
> +		of_delete_node(handle->root);
> +	if (handle->fit)
> +		free(handle->fit);

Isn't free(NULL) allowed in Barebox?  In the kernel it's defined that
kfree() will check for NULL and code should not check before calling it.

> +	free(handle);
> +}
> +
> +#ifdef CONFIG_SANDBOX
> +static int do_bootm_sandbox_fit(struct image_data *data)
> +{
> +	struct fit_handle *handle;
> +	handle = fit_open(data->os_file, data->os_num, data->verbose);
> +	if (handle)
> +		fit_close(handle);
> +	return 0;
> +}
> +
> +static struct image_handler sandbox_fit_handler = {
> +	.name = "FIT image",
> +	.bootm = do_bootm_sandbox_fit,
> +	.filetype = filetype_oftree,
> +};
> +
> +static int sandbox_fit_register(void)
> +{
> +	return register_image_handler(&sandbox_fit_handler);
> +}
> +late_initcall(sandbox_fit_register);
> +#endif
> diff --git a/include/image-fit.h b/include/image-fit.h
> new file mode 100644
> index 000000000000..bcbc859ead37
> --- /dev/null
> +++ b/include/image-fit.h
> @@ -0,0 +1,42 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + * Copyright (C) Jan Lübbe, 2014
> + */
> +
> +#ifndef __IMAGE_FIT_H__
> +#define __IMAGE_FIT_H__
> +
> +#include <linux/types.h>
> +
> +struct fit_handle {
> +	void *fit;
> +	size_t size;
> +
> +	bool verbose;
> +
> +	struct device_node *root;
> +
> +	const void *kernel;
> +	unsigned long kernel_size;
> +	const void *oftree;
> +	unsigned long oftree_size;
> +	const void *initrd;
> +	unsigned long initrd_size;
> +};
> +
> +struct fit_handle *fit_open(const char *filename, int num, bool verbose);
> +void fit_close(struct fit_handle *handle);
> +
> +#endif	/* __IMAGE_FIT_H__ */

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

Re: [PATCH 1/3] crypto: add enum

[Marc Kleine-Budde]

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

On 01/05/2016 05:54 PM, Sam Ravnborg wrote:
> On Tue, Jan 05, 2016 at 09:11:01AM +0100, Marc Kleine-Budde wrote:
>> From: Sascha Hauer <s.hauer@pengutronix.de>
>>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> The subject and the patch description could use some
> more love and care.

Good idea.

>> diff --git a/crypto/digest.c b/crypto/digest.c
>> index a90e4ff79f89..46600f246ece 100644
>> --- a/crypto/digest.c
>> +++ b/crypto/digest.c
>> @@ -116,7 +116,27 @@ static struct digest_algo *digest_algo_get_by_name(const char *name)
>>  	list_for_each_entry(tmp, &digests, list) {
>>  		if (strcmp(tmp->base.name, name) != 0)
>>  			continue;
>> -		
>> +
>> +		if (tmp->base.priority <= priority)
>> +			continue;
>> +
>> +		d = tmp;
>> +		priority = tmp->base.priority;
>> +	}
>> +
>> +	return d;
>> +}
>> +
>> +static struct digest_algo *digest_algo_get_by_algo(enum hash_algo algo)
>> +{
>> +	struct digest_algo *d = NULL;
>> +	struct digest_algo *tmp;
>> +	int priority = -1;
>> +
>> +	list_for_each_entry(tmp, &digests, list) {
>> +		if (tmp->base.algo != algo)
>> +			continue;
>> +
>>  		if (tmp->base.priority <= priority)
>>  			continue;
>>  
>> @@ -160,6 +180,27 @@ struct digest *digest_alloc(const char *name)
>>  }
>>  EXPORT_SYMBOL_GPL(digest_alloc);
>>  
>> +struct digest *digest_alloc_by_algo(enum hash_algo hash_algo)
>> +{
>> +	struct digest *d;
>> +	struct digest_algo *algo;
>> +
>> +	algo = digest_algo_get_by_algo(hash_algo);
>> +	if (!algo)
>> +		return NULL;
>> +
>> +	d = xzalloc(sizeof(*d));
>> +	d->algo = algo;
>> +	d->ctx = xzalloc(algo->ctx_length);
> 
> Neither allocations are checked for failure.

If xzalloc fails barebox will go into OOM and throw a backtrace.

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 3/3] bootm: add initial FIT support

[Marc Kleine-Budde]

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

On 01/05/2016 09:28 PM, Trent Piepho wrote:

Thanks for the review. Comments inline.

> On Tue, 2016-01-05 at 09:11 +0100, Marc Kleine-Budde wrote:
>> +static int do_bootm_arm_fit(struct image_data *data)
>> +{
>> +	struct fit_handle *handle;
>> +	int ret;
>> +	unsigned long mem_free;
>> +	unsigned long mem_start, mem_size;
>> +
>> +	handle = fit_open(data->os_file, data->os_num, data->verbose);
>> +	if (!handle)
>> +		return -EINVAL;
>> +
>> +	ret = sdram_start_and_size(&mem_start, &mem_size);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* no support for custom load address */
>> +	data->os_address = mem_start + PAGE_ALIGN(handle->kernel_size * 4);
>> +	data->os_res = request_sdram_region("fit-kernel", data->os_address, handle->kernel_size);
>> +	if (!data->os_res) {
>> +		pr_err("Cannot request region 0x%08lx - 0x%08lx\n",
>> +				data->os_address, handle->kernel_size);
>> +		ret = -ENOMEM;
>> +		goto err_out;
>> +	}
>> +	memcpy((void *)data->os_res->start, handle->kernel, handle->kernel_size);
>> +
>> +	/*
>> +	 * Put oftree/initrd close behind compressed kernel image to avoid
>> +	 * placing it outside of the kernels lowmem.
>> +	 */
>> +	if (handle->initrd_size) {
>> +		data->initrd_address = PAGE_ALIGN(data->os_res->end + SZ_1M);
>> +		data->initrd_res = request_sdram_region("fit-initrd", data->initrd_address, handle->initrd_size);
>> +		if (!data->initrd_res) {
>> +			ret = -ENOMEM;
>> +			goto err_out;
>> +		}
>> +		memcpy((void *)data->initrd_res->start, handle->initrd, handle->initrd_size);
>> +	}
>> +
>> +	data->of_root_node = of_unflatten_dtb(handle->oftree);
>> +	if (!data->of_root_node) {
>> +		pr_err("unable to unflatten devicetree\n");
>> +		ret = -EINVAL;
>> +		goto err_out;
>> +	}
>> +
>> +	/*
>> +	 * Put devicetree right after initrd if present or after the kernel
>> +	 * if not.
>> +	 */
>> +	if (data->initrd_res)
>> +		mem_free = PAGE_ALIGN(data->initrd_res->end);
>> +	else
>> +		mem_free = PAGE_ALIGN(data->os_res->end + SZ_1M);
> Why the extra 1M?

Let's see if Jan remembers, he has coded this.

>> +
>> +	return __do_bootm_linux(data, mem_free, 0);
>> +
>> +err_out:
>> +	if (handle)
>> +		fit_close(handle);
> 
> handle can't be NULL, it's been dereferenced in every path that gets
> there.

correct. tnx.

> 
>> +	return ret;
>> +}
>> +
>> +static struct image_handler arm_fit_handler = {
> 
> Can this be const?

No, because it has a struct list_head embedded.

>> +        .name = "FIT image",
>> +        .bootm = do_bootm_arm_fit,
>> +        .filetype = filetype_oftree,
>> +};
>> +
>>  static struct binfmt_hook binfmt_aimage_hook = {
>>  	.type = filetype_aimage,
>>  	.exec = "bootm",
>> @@ -578,6 +650,8 @@ static int armlinux_register_image_handler(void)
>>  		register_image_handler(&aimage_handler);
>>  		binfmt_register(&binfmt_aimage_hook);
>>  	}
>> +	if (IS_BUILTIN(CONFIG_CMD_BOOTM_FITIMAGE))
>> +	        register_image_handler(&arm_fit_handler);
>>  	binfmt_register(&binfmt_arm_zimage_hook);
>>  	binfmt_register(&binfmt_barebox_hook);
>>  
>> diff --git a/commands/Kconfig b/commands/Kconfig
>> index 1743670ed33c..b89627209f5a 100644
>> --- a/commands/Kconfig
>> +++ b/commands/Kconfig
>> @@ -418,6 +418,14 @@ config CMD_BOOTM_AIMAGE
>>  	help
>>  	  Support using Android Images.
>>  
>> +config CMD_BOOTM_FITIMAGE
>> +	bool
>> +	prompt "FIT image support"
>> +	select FITIMAGE
>> +	depends on CMD_BOOTM && ARM
>> +	help
>> +	  Support using FIT Images.
> 
> Perhaps a link about FIT images or a pointer to a file in Documentation
> could go here?

OK.

>> +/*
>> + * The consistency of the FTD structure was already checked by of_unflatten_dtb()
>> + */
>> +static int fit_verify_signature(struct device_node *sig_node, void *fit)
>> +{
>> +	uint32_t hashed_strings_start, hashed_strings_size;
>> +	struct string_list inc_nodes, exc_props;
>> +	struct rsa_public_key key = {};
>> +	struct digest *digest;
>> +	int sig_len;
>> +	const char *algo_name, *key_name, *sig_value;
>> +	char *key_path;
>> +	struct device_node *key_node;
>> +	enum hash_algo algo;
>> +	void *hash;
>> +	int ret;
>> +
>> +	if (of_property_read_string(sig_node, "algo", &algo_name)) {
>> +		pr_err("algo not found\n");
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +	if (strcmp(algo_name, "sha1,rsa2048") == 0) {
>> +		algo = HASH_ALGO_SHA1;
>> +	} else if (strcmp(algo_name, "sha256,rsa4096") == 0) {
>> +		algo = HASH_ALGO_SHA256;
>> +	} else	{
>> +		pr_err("unknown algo %s\n", algo_name);
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +	digest = digest_alloc_by_algo(algo);
>> +	if (!digest) {
>> +		pr_err("unsupported algo %s\n", algo_name);
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	sig_value = of_get_property(sig_node, "value", &sig_len);
>> +	if (!sig_value) {
>> +		pr_err("signature value not found\n");
>> +		ret = -EINVAL;
>> +		goto out_free_digest;
>> +	}
>> +
>> +	if (of_property_read_string(sig_node, "key-name-hint", &key_name)) {
>> +		pr_err("key name not found\n");
>> +		ret = -EINVAL;
>> +		goto out_free_digest;
>> +	}
>> +	key_path = asprintf("/signature/key-%s", key_name);
> 
> If I understand this correctly, one computes a hash of part of the
> device tree and then verifies it against a signature, also in the device
> tree, using an RSA key, also in the device tree.
> 
> What's the point of that?  Isn't it basically a complex CRC check for
> internal consistency?

No, as the public key comes from the device tree that's already active
in barebox. In my use case it's linked into the barebox binary which
itself is signed again (and tested by the ROM code before executed).

>> +	if (!key_name) {
>> +		ret = -ENOMEM;
>> +		goto out_free_digest;
>> +	}
>> +	key_node = of_find_node_by_path(key_path);
>> +	free(key_path);
>> +	if (!key_node) {
>> +		pr_info("failed to find key node\n");
>> +		ret = -ENOENT;
>> +		goto out_free_digest;
>> +	}
>> +
>> +	ret = rsa_of_read_key(key_node, &key);
>> +	if (ret) {
>> +		pr_info("failed to read key\n");
>> +		ret = -ENOENT;
>> +		goto out_free_digest;
>> +	}
>> +
>> +	if (of_property_read_u32_index(sig_node, "hashed-strings", 0, &hashed_strings_start)) {
>> +		pr_err("%s: hashed-strings start not found\n", __func__);
>> +		ret = -EINVAL;
>> +		goto out_free_digest;
>> +	}
>> +	if (of_property_read_u32_index(sig_node, "hashed-strings", 1, &hashed_strings_size)) {
>> +		pr_err("%s: hashed-strings size not found\n", __func__);
>> +		ret = -EINVAL;
>> +		goto out_free_digest;
>> +	}
>> +
>> +	string_list_init(&inc_nodes);
>> +	string_list_init(&exc_props);
>> +
>> +	if (of_read_string_list(sig_node, "hashed-nodes", &inc_nodes)) {
>> +		pr_err("%s: hashed-nodes invalid\n", __func__);
>> +		ret = -EINVAL;
>> +		goto out_sl;
>> +	}
>> +
>> +	string_list_add(&exc_props, "data");
>> +
>> +	digest_init(digest);
>> +	ret = fit_digest(fit, digest, &inc_nodes, &exc_props, hashed_strings_start, hashed_strings_size);
>> +	hash = xzalloc(digest_length(digest));
>> +	digest_final(digest, hash);
>> +
>> +	ret = rsa_verify(&key, sig_value, sig_len, hash, algo);
>> +	if (ret) {
>> +		pr_info("sig BAD\n");
>> +		ret = CHECK_LEVEL_NONE;
> 
> Info level message should maybe be a bit more descriptive.

OK

> 
>> +	} else {
>> +		pr_info("sig OK\n");
>> +		ret = CHECK_LEVEL_SIG;
>> +	}
>> +
>> +	free(hash);
>> + out_sl:
>> +	string_list_free(&inc_nodes);
>> +	string_list_free(&exc_props);
>> + out_free_digest:
>> +	digest_free(digest);
>> + out:
>> +	return ret;
>> +}
>> +
>> +static int fit_verify_hash(struct device_node *hash, const void *data, int data_len)
>> +{
>> +	struct digest *d;
>> +	const char *algo;
>> +	const char *value_read;
>> +	char *value_calc;
>> +	int hash_len;
>> +
>> +	value_read = of_get_property(hash, "value", &hash_len);
>> +	if (!value_read) {
>> +		pr_err("value not found\n");
>> +		return CHECK_LEVEL_NONE;
> 
> Suggest adding node path to error messages.  "value not found" really
> doesn't tell anyone anything.

OK

> 
> This is an error message, but returns CHECK_LEVEL_NONE.  While later...
> 
>> +
>> +	if (memcmp(value_read, value_calc, hash_len)) {
>> +		pr_info("hash BAD\n");
>> +		digest_free(d);
>> +		return CHECK_LEVEL_NONE;
> 
> Now it's an info message and returns CHECK_LEVEL_NONE.  Seems wrong to
> return the same value for an error and a non-error.

I've kept the LEVEL_NONE here, but return with -EINVAL if no hash value
is found.

> 
>> +	} else {
>> +		pr_info("hash OK\n");
>> +		digest_free(d);
>> +		return CHECK_LEVEL_HASH;
>> +	}
>> +}
>> +
>> +static int fit_open_image(struct fit_handle *handle, const char* unit)
>> +{
>> +	struct device_node *image = NULL, *hash;
>> +	const char *type = NULL, *desc;
>> +	const void *data;
>> +	int data_len;
>> +	int ret, level;
>> +
>> +	image = of_get_child_by_name(handle->root, "images");
>> +	if (!image)
>> +		return -ENOENT;
>> +
>> +	image = of_get_child_by_name(image, unit);
>> +	if (!image)
>> +		return -ENOENT;
>> +
>> +	if (of_property_read_string(image, "description", &desc)) {
> Here you check the return value of of_property_read_string()
>> +		pr_info("FIT image '%s' (no description)\n", unit);
>> +	} else {
>> +		pr_info("FIT image '%s': '%s'\n", unit, desc);
>> +	}
> 
> Suggest:
>          desc = "(no description)";
>          of_property_read_string(image, "description", &desc);
>          pr_info("FIT image '%s': '%s'\n", unit, desc);
>          /* desc has valid value if anyone uses it again, instead of
>             being uninitialized */

tnx

>> +
>> +	of_property_read_string(image, "type", &type);
>> +	if (!type)
>> +		return -EINVAL;
> 
> But here you check that type is non-NULL.

It's later used in strcmp().

>> +
>> +	data = of_get_property(image, "data", &data_len);
>> +	if (!data) {
>> +		pr_err("data not found\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	level = CHECK_LEVEL_MAX;
>> +	for_each_child_of_node(image, hash) {
>> +		if (handle->verbose)
>> +			of_print_nodes(hash, 0);
>> +		ret = fit_verify_hash(hash, data, data_len);
>> +		if (ret < 0)
>> +			return ret;
>> +		level = min(level, ret);
>> +	}
>> +	if (level == CHECK_LEVEL_MAX) {
>> +		return -EINVAL;
>> +	}
> Inconsistent use of {} for one statement then clause.

ok

>> +
>> +	if (level == CHECK_LEVEL_HASH) {
>> +		if (strcmp(type, "kernel") == 0 ||
>> +		    strcmp(type, "kernel_noload") == 0) {
>> +			handle->kernel = data;
>> +			handle->kernel_size = data_len;
>> +		} else if (strcmp(type, "flat_dt") == 0) {
>> +			handle->oftree = data;
>> +			handle->oftree_size = data_len;
>> +		} else if (strcmp(type, "ramdisk") == 0) {
>> +			handle->initrd = data;
>> +			handle->initrd_size = data_len;
>> +		} else {
>> +			pr_info("unknown image type %s, ignoring\n", type);
>> +		}
>> +	}
>> +
>> +	return level;
>> +}
>> +
>> +static int fit_open_configuration(struct fit_handle *handle, int num)
>> +{
>> +	struct device_node *conf_node = NULL, *sig_node;
>> +	char unit_name[10];
>> +	const char *unit, *desc;
>> +	int ret, level;
>> +
>> +	conf_node = of_get_child_by_name(handle->root, "configurations");
>> +	if (!conf_node)
>> +		return -ENOENT;
>> +
>> +	if (num) {
>> +		snprintf(unit_name, sizeof(unit_name), "conf@%d", num);
>> +		unit = unit_name;
>> +	} else if (of_property_read_string(conf_node, "default", &unit)) {
>> +		unit = "conf@1";
>> +	}
>> +
>> +	conf_node = of_get_child_by_name(conf_node, unit);
>> +	if (!conf_node) {
>> +		pr_err("FIT configuration '%s' not found\n", unit);
>> +		return -ENOENT;
>> +	}
>> +
>> +	if (of_property_read_string(conf_node, "description", &desc)) {
>> +		pr_info("FIT configuration '%s' (no description)\n", unit);
>> +	} else {
>> +		pr_info("FIT configuration '%s': '%s'\n", unit, desc);
>> +	}
>> +
>> +	level = CHECK_LEVEL_MAX;
>> +	for_each_child_of_node(conf_node, sig_node) {
>> +		if (handle->verbose)
>> +			of_print_nodes(sig_node, 0);
>> +		ret = fit_verify_signature(sig_node, handle->fit);
>> +		if (ret < 0)
>> +			return ret;
>> +		level = min(level, ret);
>> +	}
>> +	if (level == CHECK_LEVEL_MAX)
>> +		return -EINVAL;
> 
> This function up to here seems very similar to fit_open_image().  Could
> they be refactored, Ie. _fit_open(handle, "images", unit) vs
> _fit_open(handle, "configurations", "conf@1").

Given the fact that Yegor wants to improve the configuration selection
anyways I'd postpone this until he's finished.

> 
>> +
>> +	if (level != CHECK_LEVEL_SIG)
>> +		return -EINVAL;
>> +
>> +	if (of_property_read_string(conf_node, "kernel", &unit) == 0)
>> +		level = min(level, fit_open_image(handle, unit));
>> +	else
>> +		return -ENOENT;
>> +
>> +	if (of_property_read_string(conf_node, "fdt", &unit) == 0)
>> +		level = min(level, fit_open_image(handle, unit));
>> +
>> +	if (of_property_read_string(conf_node, "ramdisk", &unit) == 0)
>> +		level = min(level, fit_open_image(handle, unit));
>> +
>> +	if (level != CHECK_LEVEL_HASH)
>> +		return -EINVAL;
>> +
>> +	return 0;
>> +}
>> +
>> +struct fit_handle *fit_open(const char *filename, int num, bool verbose)
>> +{
>> +	struct fit_handle *handle = NULL;
>> +	const char *desc;
>> +
>> +	handle = xzalloc(sizeof(struct fit_handle));
>> +
>> +	handle->verbose = verbose;
>> +
>> +	handle->fit = read_file(filename, &handle->size);
>> +	if (!handle->fit) {
>> +		pr_err("unable to read %s: %s\n", filename, strerror(errno));
>> +		goto err;
>> +	}
>> +
>> +	handle->root = of_unflatten_dtb(handle->fit);
>> +	if (IS_ERR(handle->root)) {
>> +		goto err;
>> +	}
> Use of {} for one statement if clauses is inconsistent in this function.

tnx

>> +
>> +	if (of_property_read_string(handle->root, "description", &desc)) {
>> +		pr_info("FIT '%s' (no description)\n", filename);
>> +	} else {
>> +		pr_info("FIT '%s': '%s'\n", filename, desc);
>> +	}
>> +
>> +	if (fit_open_configuration(handle, num))
>> +		goto err;
>> +
>> +	return handle;
>> + err:
>> +	if (handle->root)
>> +		of_delete_node(handle->root);
>> +	if (handle->fit)
>> +		free(handle->fit);
>> +	free(handle);
>> +
>> +	return NULL;
>> +}
>> +
>> +void fit_close(struct fit_handle *handle)
>> +{
>> +	if (handle->root)
>> +		of_delete_node(handle->root);
>> +	if (handle->fit)
>> +		free(handle->fit);
> 
> Isn't free(NULL) allowed in Barebox?  In the kernel it's defined that
> kfree() will check for NULL and code should not check before calling it.

tnx, should be no problem.

> 
>> +	free(handle);
>> +}
>> +
>> +#ifdef CONFIG_SANDBOX
>> +static int do_bootm_sandbox_fit(struct image_data *data)
>> +{
>> +	struct fit_handle *handle;
>> +	handle = fit_open(data->os_file, data->os_num, data->verbose);
>> +	if (handle)
>> +		fit_close(handle);
>> +	return 0;
>> +}
>> +
>> +static struct image_handler sandbox_fit_handler = {
>> +	.name = "FIT image",
>> +	.bootm = do_bootm_sandbox_fit,
>> +	.filetype = filetype_oftree,
>> +};
>> +
>> +static int sandbox_fit_register(void)
>> +{
>> +	return register_image_handler(&sandbox_fit_handler);
>> +}
>> +late_initcall(sandbox_fit_register);
>> +#endif
>> diff --git a/include/image-fit.h b/include/image-fit.h
>> new file mode 100644
>> index 000000000000..bcbc859ead37
>> --- /dev/null
>> +++ b/include/image-fit.h
>> @@ -0,0 +1,42 @@
>> +/*
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
>> + *
>> + * Copyright (C) Jan Lübbe, 2014
>> + */
>> +
>> +#ifndef __IMAGE_FIT_H__
>> +#define __IMAGE_FIT_H__
>> +
>> +#include <linux/types.h>
>> +
>> +struct fit_handle {
>> +	void *fit;
>> +	size_t size;
>> +
>> +	bool verbose;
>> +
>> +	struct device_node *root;
>> +
>> +	const void *kernel;
>> +	unsigned long kernel_size;
>> +	const void *oftree;
>> +	unsigned long oftree_size;
>> +	const void *initrd;
>> +	unsigned long initrd_size;
>> +};
>> +
>> +struct fit_handle *fit_open(const char *filename, int num, bool verbose);
>> +void fit_close(struct fit_handle *handle);
>> +
>> +#endif	/* __IMAGE_FIT_H__ */
> 

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 1/3] crypto: add enum

[Sam Ravnborg]

> >>  
> >> +struct digest *digest_alloc_by_algo(enum hash_algo hash_algo)
> >> +{
> >> +	struct digest *d;
> >> +	struct digest_algo *algo;
> >> +
> >> +	algo = digest_algo_get_by_algo(hash_algo);
> >> +	if (!algo)
> >> +		return NULL;
> >> +
> >> +	d = xzalloc(sizeof(*d));
> >> +	d->algo = algo;
> >> +	d->ctx = xzalloc(algo->ctx_length);
> > 
> > Neither allocations are checked for failure.
> 
> If xzalloc fails barebox will go into OOM and throw a backtrace.
Makes sense. I thought about this, but failed to go back
and check this myself.

	Sam

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

Re: [PATCH 3/3] bootm: add initial FIT support

[Jan Lübbe]

On Mi, 2016-01-06 at 17:09 +0100, Marc Kleine-Budde wrote:
[...]
> >> +	/*
> >> +	 * Put oftree/initrd close behind compressed kernel image to avoid
> >> +	 * placing it outside of the kernels lowmem.
> >> +	 */
> >> +	if (handle->initrd_size) {
> >> +		data->initrd_address = PAGE_ALIGN(data->os_res->end + SZ_1M);
> >> +		data->initrd_res = request_sdram_region("fit-initrd", data->initrd_address, handle->initrd_size);
> >> +		if (!data->initrd_res) {
> >> +			ret = -ENOMEM;
> >> +			goto err_out;
> >> +		}
> >> +		memcpy((void *)data->initrd_res->start, handle->initrd, handle->initrd_size);
> >> +	}
> >> +
> >> +	data->of_root_node = of_unflatten_dtb(handle->oftree);
> >> +	if (!data->of_root_node) {
> >> +		pr_err("unable to unflatten devicetree\n");
> >> +		ret = -EINVAL;
> >> +		goto err_out;
> >> +	}
> >> +
> >> +	/*
> >> +	 * Put devicetree right after initrd if present or after the kernel
> >> +	 * if not.
> >> +	 */
> >> +	if (data->initrd_res)
> >> +		mem_free = PAGE_ALIGN(data->initrd_res->end);
> >> +	else
> >> +		mem_free = PAGE_ALIGN(data->os_res->end + SZ_1M);
> > Why the extra 1M?
> 
> Let's see if Jan remembers, he has coded this.

This is the same as in arch/arm/lib/bootm.c. I wanted to avoid having
different behaviors.

Regards,
Jan
-- 
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 3/3] bootm: add initial FIT support

[Jan Lübbe]

On Di, 2016-01-05 at 14:50 +0100, Marc Kleine-Budde wrote:
> >> > +static int fit_open_configuration(struct fit_handle *handle, int
> num)
> >> > +{
> >> > +       struct device_node *conf_node = NULL, *sig_node;
> >> > +       char unit_name[10];
> >> > +       const char *unit, *desc;
> >> > +       int ret, level;
> >> > +
> >> > +       conf_node = of_get_child_by_name(handle->root,
> "configurations");
> >> > +       if (!conf_node)
> >> > +               return -ENOENT;
> >> > +
> >> > +       if (num) {
> >> > +               snprintf(unit_name, sizeof(unit_name), "conf@%d",
> num);
> > 
> > This is not working for my *.its file:
> >
> https://github.com/visionsystemsgmbh/onrisc_br_bsp/blob/master/board/vscom/baltos/kernel-fit.its
> > U-Boot is working with bootm ${loadaddr}#conf${board_name}
> > 
> > For Barebox I've changed this line to
> > 
> > snprintf(unit_name, sizeof(unit_name), "conf%d@1", num)
> > 
> > This is how I start Linux: bootm /boot/kernel-fit.itb@
> $global.board.id
> > 
> > What is the standard for providing FIT configuration?
> 
> Don't know. Is there a spec in the u-boot sources, otherwise use the
> code.

I used the u-boot example ITS for reference (doc/uImage.FIT/multi.its).
The have the number after the @, so I used the same. Barebox's bootm
currently only supports a number to select between different "subimages"
as a legacy from uImages.

Your ITS has a @1 for every config/fdt. Why?

For selecting between different configurations depending on the board,
it would be nice to be able to reuse the DT board compatibles.

Regards,
Jan

-- 
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 3/3] bootm: add initial FIT support

[Yegor Yefremov]

On Thu, Jan 7, 2016 at 6:09 PM, Jan Lübbe <jlu@pengutronix.de> wrote:
> On Di, 2016-01-05 at 14:50 +0100, Marc Kleine-Budde wrote:
>> >> > +static int fit_open_configuration(struct fit_handle *handle, int
>> num)
>> >> > +{
>> >> > +       struct device_node *conf_node = NULL, *sig_node;
>> >> > +       char unit_name[10];
>> >> > +       const char *unit, *desc;
>> >> > +       int ret, level;
>> >> > +
>> >> > +       conf_node = of_get_child_by_name(handle->root,
>> "configurations");
>> >> > +       if (!conf_node)
>> >> > +               return -ENOENT;
>> >> > +
>> >> > +       if (num) {
>> >> > +               snprintf(unit_name, sizeof(unit_name), "conf@%d",
>> num);
>> >
>> > This is not working for my *.its file:
>> >
>> https://github.com/visionsystemsgmbh/onrisc_br_bsp/blob/master/board/vscom/baltos/kernel-fit.its
>> > U-Boot is working with bootm ${loadaddr}#conf${board_name}
>> >
>> > For Barebox I've changed this line to
>> >
>> > snprintf(unit_name, sizeof(unit_name), "conf%d@1", num)
>> >
>> > This is how I start Linux: bootm /boot/kernel-fit.itb@
>> $global.board.id
>> >
>> > What is the standard for providing FIT configuration?
>>
>> Don't know. Is there a spec in the u-boot sources, otherwise use the
>> code.
>
> I used the u-boot example ITS for reference (doc/uImage.FIT/multi.its).
> The have the number after the @, so I used the same. Barebox's bootm
> currently only supports a number to select between different "subimages"
> as a legacy from uImages.

But to be compatible with U-Boot Barebox should also accept strings
after bootm. See Joel's slides.

> Your ITS has a @1 for every config/fdt. Why?

I used http://elinux.org/images/f/f4/Elc2013_Fernandes.pdf creating my
*.its file. I don't know why, but Joel used @1 everywhere. I've
removed '@1' and it is working. As far as I understand you need this
really only to create unique nodes of the same "type".

> For selecting between different configurations depending on the board,
> it would be nice to be able to reuse the DT board compatibles.
>
> Regards,
> Jan
>
> --
> 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