[PATCH 1/7] imd: use struct imd_header * as argument

[PATCH 1/7] imd: use struct imd_header * as argument

[Sascha Hauer]

imd_concat_strings() and imd_string_data() are easier to handle when
they take a struct imd_header * instead of a struct imd_entry_string *.
Change this.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/imd.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/common/imd.c b/common/imd.c
index f84e344..acaa23f 100644
--- a/common/imd.c
+++ b/common/imd.c
@@ -193,11 +193,14 @@ static uint32_t imd_name_to_type(const char *name)
 	return IMD_TYPE_INVALID;
 }
 
-static char *imd_string_data(struct imd_entry_string *imd_string, int index)
+static char *imd_string_data(struct imd_header *imd, int index)
 {
 	int i, total = 0, l = 0;
-	int len = imd_read_length(&imd_string->header);
-	char *p = imd_string->data;
+	int len = imd_read_length(imd);
+	char *p = (char *)(imd + 1);
+
+	if (!imd_is_string(imd->type))
+		return NULL;
 
 	for (i = 0; total < len; total += l, p += l) {
 		l = strlen(p) + 1;
@@ -208,16 +211,20 @@ static char *imd_string_data(struct imd_entry_string *imd_string, int index)
 	return NULL;
 }
 
-static char *imd_concat_strings(struct imd_entry_string *imd_string)
+static char *imd_concat_strings(struct imd_header *imd)
 {
-	int i, len = imd_read_length(&imd_string->header);
+	int i, len = imd_read_length(imd);
 	char *str;
+	char *data = (char *)(imd + 1);
+
+	if (!imd_is_string(imd->type))
+		return NULL;
 
 	str = malloc(len);
 	if (!str)
 		return NULL;
 
-	memcpy(str, imd_string->data, len);
+	memcpy(str, data, len);
 
 	for (i = 0; i < len - 1; i++)
 		if (str[i] == 0)
@@ -284,10 +291,7 @@ int imd_command(int argc, char *argv[])
 			uint32_t type = imd_read_type(imd);
 
 			if (imd_is_string(type)) {
-				struct imd_entry_string *imd_string =
-					(struct imd_entry_string *)imd;
-
-				str = imd_concat_strings(imd_string);
+				str = imd_concat_strings(imd);
 
 				printf("%s: %s\n", imd_type_to_name(type), str);
 			} else {
@@ -302,13 +306,10 @@ int imd_command(int argc, char *argv[])
 		}
 
 		if (imd_is_string(type)) {
-			struct imd_entry_string *imd_string =
-				(struct imd_entry_string *)imd;
-
 			if (strno >= 0)
-				str = imd_string_data(imd_string, strno);
+				str = imd_string_data(imd, strno);
 			else
-				str = imd_concat_strings(imd_string);
+				str = imd_concat_strings(imd);
 
 			if (!str)
 				return -ENODATA;
-- 
2.7.0


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

[PATCH 2/7] imd: string returned from imd_string_data should be const

[Sascha Hauer]

imd_string_data() returns the original data, so the string should be
const.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/imd.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/common/imd.c b/common/imd.c
index acaa23f..9bd1563 100644
--- a/common/imd.c
+++ b/common/imd.c
@@ -193,7 +193,7 @@ static uint32_t imd_name_to_type(const char *name)
 	return IMD_TYPE_INVALID;
 }
 
-static char *imd_string_data(struct imd_header *imd, int index)
+static const char *imd_string_data(struct imd_header *imd, int index)
 {
 	int i, total = 0, l = 0;
 	int len = imd_read_length(imd);
@@ -306,10 +306,15 @@ int imd_command(int argc, char *argv[])
 		}
 
 		if (imd_is_string(type)) {
-			if (strno >= 0)
-				str = imd_string_data(imd, strno);
-			else
+			if (strno >= 0) {
+				const char *s = imd_string_data(imd, strno);
+				if (s)
+					str = strdup(s);
+				else
+					str = NULL;
+			} else {
 				str = imd_concat_strings(imd);
+			}
 
 			if (!str)
 				return -ENODATA;
-- 
2.7.0


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

[PATCH 3/7] scripts: Add scripts/include/ to include path for target programs

[Sascha Hauer]

Programs compiled for the target need -I $(srctree)/scripts/include/ to
be able to include for example linux/err.h.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 scripts/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile b/scripts/Makefile
index aeedcac..a5c16b2 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -45,12 +45,13 @@ targetprogs-$(CONFIG_IMD_TARGET) += bareboximd-target
 subdir-	+= basic kconfig setupmbr
 
 quiet_cmd_csingle	= CC      $@
-      cmd_csingle	= $(CC) -Wp,-MD,$(depfile) $(CFLAGS) -o $@ $<
+      cmd_csingle	= $(CC) -Wp,-MD,$(depfile) $(TARGETCFLAGS) $(CFLAGS) -o $@ $<
 
 __targetprogs	:= $(sort $(targetprogs-y) $(targetprogs-m))
 target-csingle	:= $(foreach m,$(__targetprogs),$(if $($(m)-objs),,$(m)))
 __targetprogs	:= $(addprefix $(obj)/,$(__targetprogs))
 target-csingle	:= $(addprefix $(obj)/,$(target-csingle))
+TARGETCFLAGS += -I$(srctree)/scripts/include/
 
 always		:= $(hostprogs-y) $(hostprogs-m) $(targetprogs-y)
 
-- 
2.7.0


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

[PATCH 4/7] imd: rename imd_search_validate to imd_get

[Sascha Hauer]

The name is more suitable for what the function does. Also let the
function return a pointer to the imd data found in the buffer.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/imd.c         | 24 +++++++++++-------------
 scripts/bareboximd.c |  1 +
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/common/imd.c b/common/imd.c
index 9bd1563..318980b 100644
--- a/common/imd.c
+++ b/common/imd.c
@@ -112,17 +112,17 @@ static int imd_validate_tags(void *buf, int bufsize, int start_ofs)
 }
 
 /*
- * imd_search_validate - find valid metadata in a buffer
+ * imd_get - find valid metadata in a buffer
  * @buf		the buffer
  * @size	buffer size
- * @start	The returned pointer to the metadata
  *
  * This iterates over a buffer and searches for metadata. The metadata
  * is checked for consistency (length fields not exceeding buffer and
- * presence of end header) and returned in @start. The returned pointer
- * is only valid when 0 is returned. The returned data may be unaligned.
+ * presence of end header) and returned.
+ *
+ * Return: a pointer to the image metadata or a ERR_PTR
  */
-static int imd_search_validate(void *buf, int size, struct imd_header **start)
+struct imd_header *imd_get(void *buf, int size)
 {
 	int start_ofs = 0;
 	int i, ret;
@@ -138,13 +138,11 @@ static int imd_search_validate(void *buf, int size, struct imd_header **start)
 		debug("Start tag found at offset %d\n", i);
 
 		ret = imd_validate_tags(buf, size, i);
-		if (!ret) {
-			*start = buf + i;
-			return 0;
-		}
+		if (!ret)
+			return buf + i;
 	}
 
-	return -EINVAL;
+	return ERR_PTR(-EINVAL);
 }
 
 struct imd_type_names {
@@ -282,9 +280,9 @@ int imd_command(int argc, char *argv[])
 	if (ret && ret != -EFBIG)
 		return -errno;
 
-	ret = imd_search_validate(buf, size, &imd_start);
-	if (ret)
-		return ret;
+	imd_start = imd_get(buf, size);
+	if (IS_ERR(imd_start))
+		return PTR_ERR(imd_start);
 
 	if (type == IMD_TYPE_INVALID) {
 		imd_for_each(imd_start, imd) {
diff --git a/scripts/bareboximd.c b/scripts/bareboximd.c
index a3622af..d4da681 100644
--- a/scripts/bareboximd.c
+++ b/scripts/bareboximd.c
@@ -33,6 +33,7 @@
 #include <string.h>
 #include <errno.h>
 #include <stdarg.h>
+#include <linux/err.h>
 
 #include "../include/image-metadata.h"
 
-- 
2.7.0


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

[PATCH 5/7] imd: export functions

[Sascha Hauer]

To make the image metadata API usable for external users export
some functions.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/imd.c             | 30 +++++++++++++++++++++++++++---
 include/image-metadata.h |  5 +++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/common/imd.c b/common/imd.c
index 318980b..06822bb 100644
--- a/common/imd.c
+++ b/common/imd.c
@@ -169,7 +169,13 @@ static struct imd_type_names imd_types[] = {
 	},
 };
 
-static const char *imd_type_to_name(uint32_t type)
+/**
+ * imd_type_to_name - convert a imd type to a name
+ * @type: The imd type
+ *
+ * This function returns a string representation of the imd type
+ */
+const char *imd_type_to_name(uint32_t type)
 {
 	int i;
 
@@ -191,7 +197,16 @@ static uint32_t imd_name_to_type(const char *name)
 	return IMD_TYPE_INVALID;
 }
 
-static const char *imd_string_data(struct imd_header *imd, int index)
+/**
+ * imd_string_data - get string data
+ * @imd: The IMD entry
+ * @index: The index of the string
+ *
+ * This function returns the string in @imd indexed by @index.
+ *
+ * Return: A pointer to the string or NULL if the string is not found
+ */
+const char *imd_string_data(struct imd_header *imd, int index)
 {
 	int i, total = 0, l = 0;
 	int len = imd_read_length(imd);
@@ -209,7 +224,16 @@ static const char *imd_string_data(struct imd_header *imd, int index)
 	return NULL;
 }
 
-static char *imd_concat_strings(struct imd_header *imd)
+/**
+ * imd_concat_strings - get string data
+ * @imd: The IMD entry
+ *
+ * This function returns the concatenated strings in @imd. The string
+ * returned is allocated with malloc() and the caller has to free() it.
+ *
+ * Return: A pointer to the string or NULL if the string is not found
+ */
+char *imd_concat_strings(struct imd_header *imd)
 {
 	int i, len = imd_read_length(imd);
 	char *str;
diff --git a/include/image-metadata.h b/include/image-metadata.h
index 34dae5c..33ca9c6 100644
--- a/include/image-metadata.h
+++ b/include/image-metadata.h
@@ -80,6 +80,11 @@ static inline uint32_t imd_read_length(struct imd_header *imd)
 
 struct imd_header *imd_find_type(struct imd_header *imd, uint32_t type);
 
+struct imd_header *imd_get(void *buf, int size);
+const char *imd_string_data(struct imd_header *imd, int index);
+const char *imd_type_to_name(uint32_t type);
+char *imd_concat_strings(struct imd_header *imd);
+
 extern int imd_command_verbose;
 int imd_command_setenv(const char *variable_name, const char *value);
 int imd_command(int argc, char *argv[]);
-- 
2.7.0


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

[PATCH 6/7] imd: Add function to read parameters

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/imd.c             | 31 +++++++++++++++++++++++++++++++
 include/image-metadata.h |  1 +
 2 files changed, 32 insertions(+)

diff --git a/common/imd.c b/common/imd.c
index 06822bb..4b22f41 100644
--- a/common/imd.c
+++ b/common/imd.c
@@ -255,6 +255,37 @@ char *imd_concat_strings(struct imd_header *imd)
 	return str;
 }
 
+/**
+ * imd_get_param - get a parameter
+ * @imd: The IMD entry
+ * @name: The name of the parameter.
+ *
+ * Parameters have the IMD type IMD_TYPE_PARAMETER and the form
+ * "key=value". This function iterates over the IMD entries and when
+ * it finds a parameter with name "key" it returns the value found.
+ *
+ * Return: A pointer to the value or NULL if the string is not found
+ */
+const char *imd_get_param(struct imd_header *imd, const char *name)
+{
+	struct imd_header *cur;
+	int namelen = strlen(name);
+
+	imd_for_each(imd, cur) {
+		char *data = (char *)(cur + 1);
+
+		if (cur->type != IMD_TYPE_PARAMETER)
+			continue;
+		if (strncmp(name, data, namelen))
+			continue;
+		if (data[namelen] != '=')
+			continue;
+		return data + namelen + 1;
+	}
+
+	return NULL;
+}
+
 int imd_command_verbose;
 
 int imd_command(int argc, char *argv[])
diff --git a/include/image-metadata.h b/include/image-metadata.h
index 33ca9c6..0ba9246 100644
--- a/include/image-metadata.h
+++ b/include/image-metadata.h
@@ -84,6 +84,7 @@ struct imd_header *imd_get(void *buf, int size);
 const char *imd_string_data(struct imd_header *imd, int index);
 const char *imd_type_to_name(uint32_t type);
 char *imd_concat_strings(struct imd_header *imd);
+const char *imd_get_param(struct imd_header *imd, const char *name);
 
 extern int imd_command_verbose;
 int imd_command_setenv(const char *variable_name, const char *value);
-- 
2.7.0


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

[PATCH 7/7] bbu: print and evaluate image Metadata

[Sascha Hauer]

With imd we can store metadata in barebox images. Let's use this
information to further verify that the image that is to be flashed
is the correct one. This patch extracts the device tree compatible
from the image and compares it with the one from the currently
running barebox. If it doesn't match the update is aborted with a
warning.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/bbu.c  | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/bbu.h |  1 +
 2 files changed, 74 insertions(+)

diff --git a/common/bbu.c b/common/bbu.c
index 68812a7..b49fbe6 100644
--- a/common/bbu.c
+++ b/common/bbu.c
@@ -26,6 +26,7 @@
 #include <fcntl.h>
 #include <malloc.h>
 #include <linux/stat.h>
+#include <image-metadata.h>
 
 static LIST_HEAD(bbu_image_handlers);
 
@@ -125,6 +126,76 @@ bool barebox_update_handler_exists(struct bbu_data *data)
 	return !bbu_find_handler(data->handler_name);
 }
 
+static int bbu_check_of_compat(struct bbu_data *data)
+{
+	struct device_node *root_node;
+	const char *machine, *str;
+	int ret;
+	struct imd_header *of_compat;
+
+	if (!IS_ENABLED(CONFIG_OFDEVICE) || !IS_ENABLED(CONFIG_IMD))
+		return 0;
+
+	of_compat = imd_find_type(data->imd_data, IMD_TYPE_OF_COMPATIBLE);
+	if (!of_compat)
+		return 0;
+
+	root_node = of_get_root_node();
+	if (!root_node)
+		return 0;
+
+	str = imd_string_data(of_compat, 0);
+
+	if (of_machine_is_compatible(str)) {
+		pr_info("Devicetree compatible \"%s\" matches current machine\n", str);
+		return 0;
+	}
+
+	ret = of_property_read_string(root_node, "compatible", &machine);
+	if (ret)
+		return 0;
+
+	if (!bbu_force(data, "machine is incompatible with \"%s\", have \"%s\"\n", str, machine))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int bbu_check_metadata(struct bbu_data *data)
+{
+	struct imd_header *imd;
+	int ret;
+	char *str;
+
+	if (!data->image)
+		return 0;
+
+	data->imd_data = imd_get(data->image, data->len);
+	if (IS_ERR(data->imd_data)) {
+		data->imd_data = NULL;
+		return 0;
+	}
+
+	printf("Image Metadata:\n");
+	imd_for_each(data->imd_data, imd) {
+		uint32_t type = imd_read_type(imd);
+
+		if (!imd_is_string(type))
+			continue;
+
+		str = imd_concat_strings(imd);
+
+		printf("  %s: %s\n", imd_type_to_name(type), str);
+		free(str);
+	}
+
+	ret = bbu_check_of_compat(data);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 /*
  * do a barebox update with data from *data
  */
@@ -147,6 +218,8 @@ int barebox_update(struct bbu_data *data)
 	if (!data->devicefile)
 		data->devicefile = handler->devicefile;
 
+	bbu_check_metadata(data);
+
 	ret = handler->handler(handler, data);
 	if (ret == -EINTR)
 		printf("update aborted\n");
diff --git a/include/bbu.h b/include/bbu.h
index 0fe7a1a..701d7f6 100644
--- a/include/bbu.h
+++ b/include/bbu.h
@@ -16,6 +16,7 @@ struct bbu_data {
 	const char *devicefile;
 	size_t len;
 	const char *handler_name;
+	struct imd_header *imd_data;
 };
 
 struct bbu_handler {
-- 
2.7.0


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

Re: [PATCH 7/7] bbu: print and evaluate image Metadata

[Stefan Christ]

Hi Sascha,

> @@ -147,6 +218,8 @@ int barebox_update(struct bbu_data *data)
>  	if (!data->devicefile)
>  		data->devicefile = handler->devicefile;
>  
> +	bbu_check_metadata(data);
> +
>  	ret = handler->handler(handler, data);
>  	if (ret == -EINTR)
>  		printf("update aborted\n");

hmm. I think the code should be 

     ret = bbu_check_metadata(data);
     if (ret)
          return ret;

or something. Otherwise the verification of the compatible doesn't abort the
update process. The function bbu_check_metadata returns -EINVAL if the
compatible is wrong.

The user has still the option to use "-f" or "-l" to flash a new barebox image,
if the already running barebox has the wrong compatible for the device. I just
noticed these options while looking at your patches. The force level is quite a
nice implementation to gradually overwrite any safety locks.

Mit freundlichen Grüßen / Kind regards,
	Stefan Christ

On Tue, Mar 29, 2016 at 10:50:54AM +0200, Sascha Hauer wrote:
> With imd we can store metadata in barebox images. Let's use this
> information to further verify that the image that is to be flashed
> is the correct one. This patch extracts the device tree compatible
> from the image and compares it with the one from the currently
> running barebox. If it doesn't match the update is aborted with a
> warning.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  common/bbu.c  | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/bbu.h |  1 +
>  2 files changed, 74 insertions(+)
> 
> diff --git a/common/bbu.c b/common/bbu.c
> index 68812a7..b49fbe6 100644
> --- a/common/bbu.c
> +++ b/common/bbu.c
> @@ -26,6 +26,7 @@
>  #include <fcntl.h>
>  #include <malloc.h>
>  #include <linux/stat.h>
> +#include <image-metadata.h>
>  
>  static LIST_HEAD(bbu_image_handlers);
>  
> @@ -125,6 +126,76 @@ bool barebox_update_handler_exists(struct bbu_data *data)
>  	return !bbu_find_handler(data->handler_name);
>  }
>  
> +static int bbu_check_of_compat(struct bbu_data *data)
> +{
> +	struct device_node *root_node;
> +	const char *machine, *str;
> +	int ret;
> +	struct imd_header *of_compat;
> +
> +	if (!IS_ENABLED(CONFIG_OFDEVICE) || !IS_ENABLED(CONFIG_IMD))
> +		return 0;
> +
> +	of_compat = imd_find_type(data->imd_data, IMD_TYPE_OF_COMPATIBLE);
> +	if (!of_compat)
> +		return 0;
> +
> +	root_node = of_get_root_node();
> +	if (!root_node)
> +		return 0;
> +
> +	str = imd_string_data(of_compat, 0);
> +
> +	if (of_machine_is_compatible(str)) {
> +		pr_info("Devicetree compatible \"%s\" matches current machine\n", str);
> +		return 0;
> +	}
> +
> +	ret = of_property_read_string(root_node, "compatible", &machine);
> +	if (ret)
> +		return 0;
> +
> +	if (!bbu_force(data, "machine is incompatible with \"%s\", have \"%s\"\n", str, machine))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int bbu_check_metadata(struct bbu_data *data)
> +{
> +	struct imd_header *imd;
> +	int ret;
> +	char *str;
> +
> +	if (!data->image)
> +		return 0;
> +
> +	data->imd_data = imd_get(data->image, data->len);
> +	if (IS_ERR(data->imd_data)) {
> +		data->imd_data = NULL;
> +		return 0;
> +	}
> +
> +	printf("Image Metadata:\n");
> +	imd_for_each(data->imd_data, imd) {
> +		uint32_t type = imd_read_type(imd);
> +
> +		if (!imd_is_string(type))
> +			continue;
> +
> +		str = imd_concat_strings(imd);
> +
> +		printf("  %s: %s\n", imd_type_to_name(type), str);
> +		free(str);
> +	}
> +
> +	ret = bbu_check_of_compat(data);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  /*
>   * do a barebox update with data from *data
>   */
> @@ -147,6 +218,8 @@ int barebox_update(struct bbu_data *data)
>  	if (!data->devicefile)
>  		data->devicefile = handler->devicefile;
>  
> +	bbu_check_metadata(data);
> +
>  	ret = handler->handler(handler, data);
>  	if (ret == -EINTR)
>  		printf("update aborted\n");
> diff --git a/include/bbu.h b/include/bbu.h
> index 0fe7a1a..701d7f6 100644
> --- a/include/bbu.h
> +++ b/include/bbu.h
> @@ -16,6 +16,7 @@ struct bbu_data {
>  	const char *devicefile;
>  	size_t len;
>  	const char *handler_name;
> +	struct imd_header *imd_data;
>  };
>  
>  struct bbu_handler {
> -- 
> 2.7.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

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

Re: [PATCH 2/7] imd: string returned from imd_string_data should be const

[Stefan Christ]

Hi Sascha,

On Tue, Mar 29, 2016 at 10:50:49AM +0200, Sascha Hauer wrote:
> imd_string_data() returns the original data, so the string should be
> const.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  common/imd.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/common/imd.c b/common/imd.c
> index acaa23f..9bd1563 100644
> --- a/common/imd.c
> +++ b/common/imd.c
> @@ -193,7 +193,7 @@ static uint32_t imd_name_to_type(const char *name)
>  	return IMD_TYPE_INVALID;
>  }
>  
> -static char *imd_string_data(struct imd_header *imd, int index)
> +static const char *imd_string_data(struct imd_header *imd, int index)
>  {
>  	int i, total = 0, l = 0;
>  	int len = imd_read_length(imd);
> @@ -306,10 +306,15 @@ int imd_command(int argc, char *argv[])
>  		}
>  
>  		if (imd_is_string(type)) {
> -			if (strno >= 0)
> -				str = imd_string_data(imd, strno);
> -			else
> +			if (strno >= 0) {
> +				const char *s = imd_string_data(imd, strno);
> +				if (s)
> +					str = strdup(s);

The following code in the function doesn't free the string 'str' correctly. The
code is

                        if (strno < 0)
                                free(str);


> +				else
> +					str = NULL;
> +			} else {
>  				str = imd_concat_strings(imd);
> +			}
>  
>  			if (!str)
>  				return -ENODATA;
> -- 
> 2.7.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

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

Re: [PATCH 7/7] bbu: print and evaluate image Metadata

[Sascha Hauer]

Hi Stefan,

On Tue, Mar 29, 2016 at 11:29:45AM +0200, Stefan Christ wrote:
> Hi Sascha,
> 
> > @@ -147,6 +218,8 @@ int barebox_update(struct bbu_data *data)
> >  	if (!data->devicefile)
> >  		data->devicefile = handler->devicefile;
> >  
> > +	bbu_check_metadata(data);
> > +
> >  	ret = handler->handler(handler, data);
> >  	if (ret == -EINTR)
> >  		printf("update aborted\n");
> 
> hmm. I think the code should be 
> 
>      ret = bbu_check_metadata(data);
>      if (ret)
>           return ret;
> 
> or something. Otherwise the verification of the compatible doesn't abort the
> update process. The function bbu_check_metadata returns -EINVAL if the
> compatible is wrong.

Indeed, fixed this.

Sascha

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

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

Re: [PATCH 2/7] imd: string returned from imd_string_data should be const

[Sascha Hauer]

On Tue, Mar 29, 2016 at 11:34:06AM +0200, Stefan Christ wrote:
> Hi Sascha,
> 
> On Tue, Mar 29, 2016 at 10:50:49AM +0200, Sascha Hauer wrote:
> > imd_string_data() returns the original data, so the string should be
> > const.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  common/imd.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/common/imd.c b/common/imd.c
> > index acaa23f..9bd1563 100644
> > --- a/common/imd.c
> > +++ b/common/imd.c
> > @@ -193,7 +193,7 @@ static uint32_t imd_name_to_type(const char *name)
> >  	return IMD_TYPE_INVALID;
> >  }
> >  
> > -static char *imd_string_data(struct imd_header *imd, int index)
> > +static const char *imd_string_data(struct imd_header *imd, int index)
> >  {
> >  	int i, total = 0, l = 0;
> >  	int len = imd_read_length(imd);
> > @@ -306,10 +306,15 @@ int imd_command(int argc, char *argv[])
> >  		}
> >  
> >  		if (imd_is_string(type)) {
> > -			if (strno >= 0)
> > -				str = imd_string_data(imd, strno);
> > -			else
> > +			if (strno >= 0) {
> > +				const char *s = imd_string_data(imd, strno);
> > +				if (s)
> > +					str = strdup(s);
> 
> The following code in the function doesn't free the string 'str' correctly. The
> code is
> 
>                         if (strno < 0)
>                                 free(str);

Ok, if I see it correctly str should be freed unconditionally.

Sascha


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

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