bareboxenv -s: output depends on filesystem

bareboxenv -s: output depends on filesystem

[Baeuerle, Florian]

Hi,

I'm currently trying to get reproducible barebox binaries.

One problem I'm facing is, that the barebox defaultenv generated during build
depends on the filesystem used on the build machine. That is, because
envfs_save() uses recursive_action(), which in turn uses readdir() without
sorting the entries afterwards.

quoting man readdir:
  The order in which filenames are read by successive calls to readdir() depends
on the filesystem implementation; it is unlikely that the names will be sorted
in any fashion.

In fact, on ext4 I get a different barebox binary as on XFS.


Is this considered something worth being fixed?

It should be sufficient to build a list of directories and sort it before
recursing. It is, however, shared code which is also used by the saveenv command
and I'm not sure if malloc'ing recursively is a good idea on all targets where
saveenv is used.

I would go ahead and implement it if no one opposes.

Should I introduce a flag for recursive_action() that allows taking an
"unsorted" (non-malloc) code path for use with the saveenv command?


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

Re: bareboxenv -s: output depends on filesystem

[Sascha Hauer]

Hi Florian,

On Thu, Dec 13, 2018 at 03:47:20PM +0000, Baeuerle, Florian wrote:
> Hi,
> 
> I'm currently trying to get reproducible barebox binaries.
> 
> One problem I'm facing is, that the barebox defaultenv generated during build
> depends on the filesystem used on the build machine. That is, because
> envfs_save() uses recursive_action(), which in turn uses readdir() without
> sorting the entries afterwards.
> 
> quoting man readdir:
>   The order in which filenames are read by successive calls to readdir() depends
> on the filesystem implementation; it is unlikely that the names will be sorted
> in any fashion.
> 
> In fact, on ext4 I get a different barebox binary as on XFS.
> 
> 
> Is this considered something worth being fixed?

Yes, sure.

> 
> It should be sufficient to build a list of directories and sort it before
> recursing. It is, however, shared code which is also used by the saveenv command
> and I'm not sure if malloc'ing recursively is a good idea on all targets where
> saveenv is used.
> 
> I would go ahead and implement it if no one opposes.
> 
> Should I introduce a flag for recursive_action() that allows taking an
> "unsorted" (non-malloc) code path for use with the saveenv command?

Yes, that would be good. I guess there's no point in letting barebox
sort the entries.

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

[PATCH 1/2] recursive_action: add ACTION_SORT flag

[Baeuerle, Florian]

Add a flag to sort directory entries before recursing into them.

Since this part of lib/ is used inside barebox as well as in
scripts/bareboxenv.c, we cannot easily use stringlists from lib/, which
would have made the code a bit nicer.

Further, add poison.h from kernel 4.5-rc1, which was the base for
importing linux headers under scripts/ in a883d9a. This is required for
actually using kernel linked lists within scripts/ tools.

Signed-off-by: Florian Bäuerle <florian.baeuerle@allegion.com>
---
 include/libbb.h                   |  1 +
 lib/list_sort.c                   |  3 +-
 lib/recursive_action.c            | 61 +++++++++++++++++---
 scripts/bareboxenv.c              |  6 ++
 scripts/include/linux/list_sort.h |  1 +
 scripts/include/linux/poison.h    | 93 ++++++++++++++++++++++++++++++-
 6 files changed, 155 insertions(+), 10 deletions(-)
 create mode 100644 scripts/include/linux/list_sort.h

diff --git a/include/libbb.h b/include/libbb.h
index a362bd32d..1f6afaa27 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -17,6 +17,7 @@ enum {
 	ACTION_FOLLOWLINKS    = (1 << 1),
 	ACTION_DEPTHFIRST  = (1 << 2),
 	/*ACTION_REVERSE   = (1 << 3), - unused */
+	ACTION_SORT        = (1 << 4),
 };
 
 int recursive_action(const char *fileName, unsigned flags,
diff --git a/lib/list_sort.c b/lib/list_sort.c
index b7e74f260..84c6f6465 100644
--- a/lib/list_sort.c
+++ b/lib/list_sort.c
@@ -1,7 +1,6 @@
 #ifndef __BAREBOX__
 #include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/slab.h>
+#define EXPORT_SYMBOL(x)
 #else
 #include <common.h>
 #include <malloc.h>
diff --git a/lib/recursive_action.c b/lib/recursive_action.c
index 345d3db0c..0d7ddbd8e 100644
--- a/lib/recursive_action.c
+++ b/lib/recursive_action.c
@@ -16,6 +16,14 @@
 
 #endif
 
+#include <linux/list.h>
+#include <linux/list_sort.h>
+
+struct dirlist {
+	char *dirname;
+	struct list_head list;
+};
+
 /*
  * Walk down all the directories under the specified
  * location, and do something (something specified
@@ -33,6 +41,19 @@ static int true_action(const char *fileName, struct stat *statbuf,
 	return 1;
 }
 
+static int cmp_dirlist(void *priv, struct list_head *a, struct list_head *b)
+{
+	struct dirlist *ra, *rb;
+
+	if (a == b)
+		return 0;
+
+	ra = list_entry(a, struct dirlist, list);
+	rb = list_entry(b, struct dirlist, list);
+
+	return strcmp(ra->dirname, rb->dirname);
+}
+
 /* fileAction return value of 0 on any file in directory will make
  * recursive_action() return 0, but it doesn't stop directory traversal
  * (fileAction/dirAction will be called on each file).
@@ -55,9 +76,12 @@ int recursive_action(const char *fileName,
 {
 	struct stat statbuf;
 	unsigned follow;
+	unsigned sort;
 	int status;
 	DIR *dir;
 	struct dirent *next;
+	struct dirlist *entry, *entry_tmp;
+	LIST_HEAD(dirs);
 
 	if (!fileAction) fileAction = true_action;
 	if (!dirAction) dirAction = true_action;
@@ -106,20 +130,43 @@ int recursive_action(const char *fileName,
 		/* To trigger: "find -exec rm -rf {} \;" */
 		goto done_nak_warn;
 	}
+	sort = flags & ACTION_SORT;
 	status = 1;
 	while ((next = readdir(dir)) != NULL) {
-		char *nextFile;
-
-		nextFile = concat_subpath_file(fileName, next->d_name);
+		char *nextFile = concat_subpath_file(fileName, next->d_name);
 		if (nextFile == NULL)
 			continue;
-		/* now descend into it, forcing recursion. */
-		if (!recursive_action(nextFile, flags | ACTION_RECURSE,
-				fileAction, dirAction, userData, depth+1)) {
-			status = 0;
+
+		if (sort) {
+			struct dirlist *e = malloc(sizeof(*e));
+			e->dirname = strdup(next->d_name);
+			list_add(&e->list, &dirs);
+		} else {
+			/* descend into it, forcing recursion. */
+			if (!recursive_action(nextFile, flags | ACTION_RECURSE,
+						fileAction, dirAction, userData, depth+1)) {
+				status = 0;
+			}
 		}
+
 		free(nextFile);
 	}
+
+	if (sort) {
+		list_sort(NULL, &dirs, &cmp_dirlist);
+
+		list_for_each_entry_safe(entry, entry_tmp, &dirs, list)
+		{
+			/* descend into it, forcing recursion. */
+			if (!recursive_action(entry->dirname, flags | ACTION_RECURSE,
+						fileAction, dirAction, userData, depth+1)) {
+				status = 0;
+			}
+
+			list_del(&entry->list);
+			free(entry->dirname);
+		}
+	}
 	closedir(dir);
 	if ((flags & ACTION_DEPTHFIRST) &&
 		!dirAction(fileName, &statbuf, userData, depth)) {
diff --git a/scripts/bareboxenv.c b/scripts/bareboxenv.c
index 249e65b25..de57c2fce 100644
--- a/scripts/bareboxenv.c
+++ b/scripts/bareboxenv.c
@@ -34,6 +34,7 @@
 #include "compiler.h"
 
 #define debug(...)
+#define printk_once(...)
 
 /* Find out if the last character of a string matches the one given.
  * Don't underrun the buffer if the string length is 0.
@@ -54,6 +55,7 @@ enum {
 	ACTION_FOLLOWLINKS = (1 << 1),
 	ACTION_DEPTHFIRST  = (1 << 2),
 	/*ACTION_REVERSE   = (1 << 3), - unused */
+	ACTION_SORT        = (1 << 4),
 };
 
 int recursive_action(const char *fileName, unsigned flags,
@@ -95,6 +97,10 @@ static char *concat_subpath_file(const char *path, const char *f)
 	return concat_path_file(path, f);
 }
 
+#include <linux/list.h>
+#include <linux/list_sort.h>
+#include <linux/compiler.h>
+#include "../lib/list_sort.c"
 #include "../lib/recursive_action.c"
 #include "../include/envfs.h"
 #include "../crypto/crc32.c"
diff --git a/scripts/include/linux/list_sort.h b/scripts/include/linux/list_sort.h
new file mode 100644
index 000000000..be889c9ae
--- /dev/null
+++ b/scripts/include/linux/list_sort.h
@@ -0,0 +1 @@
+#include <../../include/linux/list_sort.h>
diff --git a/scripts/include/linux/poison.h b/scripts/include/linux/poison.h
index 0c27bdf14..15927ebc2 100644
--- a/scripts/include/linux/poison.h
+++ b/scripts/include/linux/poison.h
@@ -1 +1,92 @@
-#include "../../../include/linux/poison.h"
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_POISON_H
+#define _LINUX_POISON_H
+
+/********** include/linux/list.h **********/
+
+/*
+ * Architectures might want to move the poison pointer offset
+ * into some well-recognized area such as 0xdead000000000000,
+ * that is also not mappable by user-space exploits:
+ */
+#ifdef CONFIG_ILLEGAL_POINTER_VALUE
+# define POISON_POINTER_DELTA _AC(CONFIG_ILLEGAL_POINTER_VALUE, UL)
+#else
+# define POISON_POINTER_DELTA 0
+#endif
+
+/*
+ * These are non-NULL pointers that will result in page faults
+ * under normal circumstances, used to verify that nobody uses
+ * non-initialized list entries.
+ */
+#define LIST_POISON1  ((void *) 0x100 + POISON_POINTER_DELTA)
+#define LIST_POISON2  ((void *) 0x200 + POISON_POINTER_DELTA)
+
+/********** include/linux/timer.h **********/
+/*
+ * Magic number "tsta" to indicate a static timer initializer
+ * for the object debugging code.
+ */
+#define TIMER_ENTRY_STATIC	((void *) 0x300 + POISON_POINTER_DELTA)
+
+/********** mm/debug-pagealloc.c **********/
+#ifdef CONFIG_PAGE_POISONING_ZERO
+#define PAGE_POISON 0x00
+#else
+#define PAGE_POISON 0xaa
+#endif
+
+/********** mm/page_alloc.c ************/
+
+#define TAIL_MAPPING	((void *) 0x400 + POISON_POINTER_DELTA)
+
+/********** mm/slab.c **********/
+/*
+ * Magic nums for obj red zoning.
+ * Placed in the first word before and the first word after an obj.
+ */
+#define	RED_INACTIVE	0x09F911029D74E35BULL	/* when obj is inactive */
+#define	RED_ACTIVE	0xD84156C5635688C0ULL	/* when obj is active */
+
+#define SLUB_RED_INACTIVE	0xbb
+#define SLUB_RED_ACTIVE		0xcc
+
+/* ...and for poisoning */
+#define	POISON_INUSE	0x5a	/* for use-uninitialised poisoning */
+#define POISON_FREE	0x6b	/* for use-after-free poisoning */
+#define	POISON_END	0xa5	/* end-byte of poisoning */
+
+/********** arch/$ARCH/mm/init.c **********/
+#define POISON_FREE_INITMEM	0xcc
+
+/********** arch/ia64/hp/common/sba_iommu.c **********/
+/*
+ * arch/ia64/hp/common/sba_iommu.c uses a 16-byte poison string with a
+ * value of "SBAIOMMU POISON\0" for spill-over poisoning.
+ */
+
+/********** fs/jbd/journal.c **********/
+#define JBD_POISON_FREE		0x5b
+#define JBD2_POISON_FREE	0x5c
+
+/********** drivers/base/dmapool.c **********/
+#define	POOL_POISON_FREED	0xa7	/* !inuse */
+#define	POOL_POISON_ALLOCATED	0xa9	/* !initted */
+
+/********** drivers/atm/ **********/
+#define ATM_POISON_FREE		0x12
+#define ATM_POISON		0xdeadbeef
+
+/********** kernel/mutexes **********/
+#define MUTEX_DEBUG_INIT	0x11
+#define MUTEX_DEBUG_FREE	0x22
+#define MUTEX_POISON_WW_CTX	((void *) 0x500 + POISON_POINTER_DELTA)
+
+/********** lib/flex_array.c **********/
+#define FLEX_ARRAY_FREE	0x6c	/* for use-after-free poisoning */
+
+/********** security/ **********/
+#define KEY_DESTROY		0xbd
+
+#endif
-- 
2.19.2

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

[PATCH 2/2] envfs: new flag for sorting env before saving

[Baeuerle, Florian]

The resulting environment was dependend of the build machines'
filesystem, i.e. the order in which readdir returns dirents depends on
the filesystem implementation.

Use the new flag in scripts/bareboxenv.c for generating a reproducible
built-in environment.

Signed-off-by: Florian Bäuerle <florian.baeuerle@allegion.com>
---
 common/environment.c | 8 ++++++--
 include/envfs.h      | 1 +
 scripts/bareboxenv.c | 2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/common/environment.c b/common/environment.c
index 56a030eda..cea55f313 100644
--- a/common/environment.c
+++ b/common/environment.c
@@ -257,6 +257,7 @@ int envfs_save(const char *filename, const char *dirname, unsigned flags)
 	void *buf = NULL, *wbuf;
 	struct envfs_entry *env;
 	const char *defenv_path = default_environment_path_get();
+	int recursive_flags = ACTION_RECURSE;
 
 	if (!filename)
 		filename = defenv_path;
@@ -276,10 +277,13 @@ int envfs_save(const char *filename, const char *dirname, unsigned flags)
 	if (flags & ENVFS_FLAGS_FORCE_BUILT_IN) {
 		size = 0; /* force no content */
 	} else {
+		if (flags & ENVFS_FLAGS_SORTED)
+			recursive_flags |= ACTION_SORT;
+
 		/* first pass: calculate size */
-		recursive_action(dirname, ACTION_RECURSE, file_action,
+		recursive_action(dirname, recursive_flags, file_action,
 				NULL, &data, 0);
-		recursive_action("/.defaultenv", ACTION_RECURSE,
+		recursive_action("/.defaultenv", recursive_flags,
 				file_remove_action, NULL, &data, 0);
 		size = 0;
 
diff --git a/include/envfs.h b/include/envfs.h
index 27c4b42c6..fa12c60a8 100644
--- a/include/envfs.h
+++ b/include/envfs.h
@@ -45,6 +45,7 @@ struct envfs_super {
 	uint16_t future;		/* reserved for future use */
 	uint32_t flags;			/* feature flags */
 #define ENVFS_FLAGS_FORCE_BUILT_IN	(1 << 0)
+#define ENVFS_FLAGS_SORTED		(1 << 1)
 	uint32_t sb_crc;		/* crc for the superblock */
 };
 
diff --git a/scripts/bareboxenv.c b/scripts/bareboxenv.c
index de57c2fce..3e7c736b9 100644
--- a/scripts/bareboxenv.c
+++ b/scripts/bareboxenv.c
@@ -127,7 +127,7 @@ int main(int argc, char *argv[])
 	int opt;
 	int save = 0, load = 0, pad = 0, err = 0, fd;
 	char *filename = NULL, *dirname = NULL;
-	unsigned envfs_flags = 0;
+	unsigned envfs_flags = ENVFS_FLAGS_SORTED;
 	int verbose = 0;
 
 	while((opt = getopt(argc, argv, "slp:vz")) != -1) {
-- 
2.19.2

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

Re: [PATCH 1/2] recursive_action: add ACTION_SORT flag

[Baeuerle, Florian]

Am Dienstag, den 18.12.2018, 13:22 +0000 schrieb Baeuerle, Florian:
> Add a flag to sort directory entries before recursing into them.
> 
> Since this part of lib/ is used inside barebox as well as in
> scripts/bareboxenv.c, we cannot easily use stringlists from lib/, which
> would have made the code a bit nicer.
> 
> Further, add poison.h from kernel 4.5-rc1, which was the base for
> importing linux headers under scripts/ in a883d9a. This is required for
> actually using kernel linked lists within scripts/ tools.
> 
> Signed-off-by: Florian Bäuerle <florian.baeuerle@allegion.com>
> ---
>  include/libbb.h                   |  1 +
>  lib/list_sort.c                   |  3 +-
>  lib/recursive_action.c            | 61 +++++++++++++++++---
>  scripts/bareboxenv.c              |  6 ++
>  scripts/include/linux/list_sort.h |  1 +
>  scripts/include/linux/poison.h    | 93 ++++++++++++++++++++++++++++++-
>  6 files changed, 155 insertions(+), 10 deletions(-)
>  create mode 100644 scripts/include/linux/list_sort.h
> 
> diff --git a/include/libbb.h b/include/libbb.h
> index a362bd32d..1f6afaa27 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
> @@ -17,6 +17,7 @@ enum {
>  	ACTION_FOLLOWLINKS    = (1 << 1),
>  	ACTION_DEPTHFIRST  = (1 << 2),
>  	/*ACTION_REVERSE   = (1 << 3), - unused */
> +	ACTION_SORT        = (1 << 4),
>  };
>  
>  int recursive_action(const char *fileName, unsigned flags,
> diff --git a/lib/list_sort.c b/lib/list_sort.c
> index b7e74f260..84c6f6465 100644
> --- a/lib/list_sort.c
> +++ b/lib/list_sort.c
> @@ -1,7 +1,6 @@
>  #ifndef __BAREBOX__
>  #include <linux/kernel.h>
> -#include <linux/module.h>
> -#include <linux/slab.h>
> +#define EXPORT_SYMBOL(x)
>  #else
>  #include <common.h>
>  #include <malloc.h>
> diff --git a/lib/recursive_action.c b/lib/recursive_action.c
> index 345d3db0c..0d7ddbd8e 100644
> --- a/lib/recursive_action.c
> +++ b/lib/recursive_action.c
> @@ -16,6 +16,14 @@
>  
>  #endif
>  
> +#include <linux/list.h>
> +#include <linux/list_sort.h>
> +
> +struct dirlist {
> +	char *dirname;
> +	struct list_head list;
> +};
> +
>  /*
>   * Walk down all the directories under the specified
>   * location, and do something (something specified
> @@ -33,6 +41,19 @@ static int true_action(const char *fileName, struct stat
> *statbuf,
>  	return 1;
>  }
>  
> +static int cmp_dirlist(void *priv, struct list_head *a, struct list_head *b)
> +{
> +	struct dirlist *ra, *rb;
> +
> +	if (a == b)
> +		return 0;
> +
> +	ra = list_entry(a, struct dirlist, list);
> +	rb = list_entry(b, struct dirlist, list);
> +
> +	return strcmp(ra->dirname, rb->dirname);
> +}
> +
>  /* fileAction return value of 0 on any file in directory will make
>   * recursive_action() return 0, but it doesn't stop directory traversal
>   * (fileAction/dirAction will be called on each file).
> @@ -55,9 +76,12 @@ int recursive_action(const char *fileName,
>  {
>  	struct stat statbuf;
>  	unsigned follow;
> +	unsigned sort;
>  	int status;
>  	DIR *dir;
>  	struct dirent *next;
> +	struct dirlist *entry, *entry_tmp;
> +	LIST_HEAD(dirs);
>  
>  	if (!fileAction) fileAction = true_action;
>  	if (!dirAction) dirAction = true_action;
> @@ -106,20 +130,43 @@ int recursive_action(const char *fileName,
>  		/* To trigger: "find -exec rm -rf {} \;" */
>  		goto done_nak_warn;
>  	}
> +	sort = flags & ACTION_SORT;
>  	status = 1;
>  	while ((next = readdir(dir)) != NULL) {
> -		char *nextFile;
> -
> -		nextFile = concat_subpath_file(fileName, next->d_name);
> +		char *nextFile = concat_subpath_file(fileName, next->d_name);
>  		if (nextFile == NULL)
>  			continue;
> -		/* now descend into it, forcing recursion. */
> -		if (!recursive_action(nextFile, flags | ACTION_RECURSE,
> -				fileAction, dirAction, userData, depth+1)) {
> -			status = 0;
> +
> +		if (sort) {
> +			struct dirlist *e = malloc(sizeof(*e));
> +			e->dirname = strdup(next->d_name);

I think this should handle NULLs.

> +			list_add(&e->list, &dirs);
> +		} else {
> +			/* descend into it, forcing recursion. */
> +			if (!recursive_action(nextFile, flags | ACTION_RECURSE,
> +						fileAction, dirAction, userData,
> depth+1)) {
> +				status = 0;
> +			}
>  		}
> +
>  		free(nextFile);
>  	}
> +
> +	if (sort) {
> +		list_sort(NULL, &dirs, &cmp_dirlist);
> +
> +		list_for_each_entry_safe(entry, entry_tmp, &dirs, list)
> +		{
> +			/* descend into it, forcing recursion. */
> +			if (!recursive_action(entry->dirname, flags |
> ACTION_RECURSE,
> +						fileAction, dirAction, userData,
> depth+1)) {
> +				status = 0;
> +			}
> +
> +			list_del(&entry->list);
> +			free(entry->dirname);
> +		}
> +	}
>  	closedir(dir);
>  	if ((flags & ACTION_DEPTHFIRST) &&
>  		!dirAction(fileName, &statbuf, userData, depth)) {
> diff --git a/scripts/bareboxenv.c b/scripts/bareboxenv.c
> index 249e65b25..de57c2fce 100644
> --- a/scripts/bareboxenv.c
> +++ b/scripts/bareboxenv.c
> @@ -34,6 +34,7 @@
>  #include "compiler.h"
>  
>  #define debug(...)
> +#define printk_once(...)
>  
>  /* Find out if the last character of a string matches the one given.
>   * Don't underrun the buffer if the string length is 0.
> @@ -54,6 +55,7 @@ enum {
>  	ACTION_FOLLOWLINKS = (1 << 1),
>  	ACTION_DEPTHFIRST  = (1 << 2),
>  	/*ACTION_REVERSE   = (1 << 3), - unused */
> +	ACTION_SORT        = (1 << 4),
>  };
>  
>  int recursive_action(const char *fileName, unsigned flags,
> @@ -95,6 +97,10 @@ static char *concat_subpath_file(const char *path, const
> char *f)
>  	return concat_path_file(path, f);
>  }
>  
> +#include <linux/list.h>
> +#include <linux/list_sort.h>
> +#include <linux/compiler.h>
> +#include "../lib/list_sort.c"
>  #include "../lib/recursive_action.c"
>  #include "../include/envfs.h"
>  #include "../crypto/crc32.c"
> diff --git a/scripts/include/linux/list_sort.h
> b/scripts/include/linux/list_sort.h
> new file mode 100644
> index 000000000..be889c9ae
> --- /dev/null
> +++ b/scripts/include/linux/list_sort.h
> @@ -0,0 +1 @@
> +#include <../../include/linux/list_sort.h>
> diff --git a/scripts/include/linux/poison.h b/scripts/include/linux/poison.h
> index 0c27bdf14..15927ebc2 100644
> --- a/scripts/include/linux/poison.h
> +++ b/scripts/include/linux/poison.h
> @@ -1 +1,92 @@
> -#include "../../../include/linux/poison.h"
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_POISON_H
> +#define _LINUX_POISON_H
> +
> +/********** include/linux/list.h **********/
> +
> +/*
> + * Architectures might want to move the poison pointer offset
> + * into some well-recognized area such as 0xdead000000000000,
> + * that is also not mappable by user-space exploits:
> + */
> +#ifdef CONFIG_ILLEGAL_POINTER_VALUE
> +# define POISON_POINTER_DELTA _AC(CONFIG_ILLEGAL_POINTER_VALUE, UL)
> +#else
> +# define POISON_POINTER_DELTA 0
> +#endif
> +
> +/*
> + * These are non-NULL pointers that will result in page faults
> + * under normal circumstances, used to verify that nobody uses
> + * non-initialized list entries.
> + */
> +#define LIST_POISON1  ((void *) 0x100 + POISON_POINTER_DELTA)
> +#define LIST_POISON2  ((void *) 0x200 + POISON_POINTER_DELTA)
> +
> +/********** include/linux/timer.h **********/
> +/*
> + * Magic number "tsta" to indicate a static timer initializer
> + * for the object debugging code.
> + */
> +#define TIMER_ENTRY_STATIC	((void *) 0x300 + POISON_POINTER_DELTA)
> +
> +/********** mm/debug-pagealloc.c **********/
> +#ifdef CONFIG_PAGE_POISONING_ZERO
> +#define PAGE_POISON 0x00
> +#else
> +#define PAGE_POISON 0xaa
> +#endif
> +
> +/********** mm/page_alloc.c ************/
> +
> +#define TAIL_MAPPING	((void *) 0x400 + POISON_POINTER_DELTA)
> +
> +/********** mm/slab.c **********/
> +/*
> + * Magic nums for obj red zoning.
> + * Placed in the first word before and the first word after an obj.
> + */
> +#define	RED_INACTIVE	0x09F911029D74E35BULL	/* when obj is
> inactive */
> +#define	RED_ACTIVE	0xD84156C5635688C0ULL	/* when obj is active
> */
> +
> +#define SLUB_RED_INACTIVE	0xbb
> +#define SLUB_RED_ACTIVE		0xcc
> +
> +/* ...and for poisoning */
> +#define	POISON_INUSE	0x5a	/* for use-uninitialised poisoning */
> +#define POISON_FREE	0x6b	/* for use-after-free poisoning */
> +#define	POISON_END	0xa5	/* end-byte of poisoning */
> +
> +/********** arch/$ARCH/mm/init.c **********/
> +#define POISON_FREE_INITMEM	0xcc
> +
> +/********** arch/ia64/hp/common/sba_iommu.c **********/
> +/*
> + * arch/ia64/hp/common/sba_iommu.c uses a 16-byte poison string with a
> + * value of "SBAIOMMU POISON\0" for spill-over poisoning.
> + */
> +
> +/********** fs/jbd/journal.c **********/
> +#define JBD_POISON_FREE		0x5b
> +#define JBD2_POISON_FREE	0x5c
> +
> +/********** drivers/base/dmapool.c **********/
> +#define	POOL_POISON_FREED	0xa7	/* !inuse */
> +#define	POOL_POISON_ALLOCATED	0xa9	/* !initted */
> +
> +/********** drivers/atm/ **********/
> +#define ATM_POISON_FREE		0x12
> +#define ATM_POISON		0xdeadbeef
> +
> +/********** kernel/mutexes **********/
> +#define MUTEX_DEBUG_INIT	0x11
> +#define MUTEX_DEBUG_FREE	0x22
> +#define MUTEX_POISON_WW_CTX	((void *) 0x500 + POISON_POINTER_DELTA)
> +
> +/********** lib/flex_array.c **********/
> +#define FLEX_ARRAY_FREE	0x6c	/* for use-after-free poisoning */
> +
> +/********** security/ **********/
> +#define KEY_DESTROY		0xbd
> +
> +#endif
> -- 
> 2.19.2
> 
> _______________________________________________
> 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

[PATCH v2 1/2] recursive_action: add ACTION_SORT flag

[Baeuerle, Florian]

Add a flag to sort directory entries before recursing into them.

Since this part of lib/ is used inside barebox as well as in
scripts/bareboxenv.c, we cannot easily use stringlists from lib/, which
would have made the code a bit nicer.

Further, add poison.h from kernel 4.5-rc1, which was the base for
importing linux headers under scripts/ in a883d9a. This is required for
actually using kernel linked lists within scripts/ tools.

Signed-off-by: Florian Bäuerle <florian.baeuerle@allegion.com>
---
 include/libbb.h                   |  1 +
 lib/list_sort.c                   |  3 +-
 lib/recursive_action.c            | 61 +++++++++++++++++---
 scripts/bareboxenv.c              | 15 +++++
 scripts/include/linux/list_sort.h |  1 +
 scripts/include/linux/poison.h    | 93 ++++++++++++++++++++++++++++++-
 6 files changed, 164 insertions(+), 10 deletions(-)
 create mode 100644 scripts/include/linux/list_sort.h

diff --git a/include/libbb.h b/include/libbb.h
index a362bd32d..1f6afaa27 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -17,6 +17,7 @@ enum {
 	ACTION_FOLLOWLINKS    = (1 << 1),
 	ACTION_DEPTHFIRST  = (1 << 2),
 	/*ACTION_REVERSE   = (1 << 3), - unused */
+	ACTION_SORT        = (1 << 4),
 };
 
 int recursive_action(const char *fileName, unsigned flags,
diff --git a/lib/list_sort.c b/lib/list_sort.c
index b7e74f260..84c6f6465 100644
--- a/lib/list_sort.c
+++ b/lib/list_sort.c
@@ -1,7 +1,6 @@
 #ifndef __BAREBOX__
 #include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/slab.h>
+#define EXPORT_SYMBOL(x)
 #else
 #include <common.h>
 #include <malloc.h>
diff --git a/lib/recursive_action.c b/lib/recursive_action.c
index 345d3db0c..19e661ef1 100644
--- a/lib/recursive_action.c
+++ b/lib/recursive_action.c
@@ -13,9 +13,18 @@
 #include <linux/stat.h>
 #include <malloc.h>
 #include <libbb.h>
+#include <xfuncs.h>
 
 #endif
 
+#include <linux/list.h>
+#include <linux/list_sort.h>
+
+struct dirlist {
+	char *dirname;
+	struct list_head list;
+};
+
 /*
  * Walk down all the directories under the specified
  * location, and do something (something specified
@@ -33,6 +42,19 @@ static int true_action(const char *fileName, struct stat *statbuf,
 	return 1;
 }
 
+static int cmp_dirlist(void *priv, struct list_head *a, struct list_head *b)
+{
+	struct dirlist *ra, *rb;
+
+	if (a == b)
+		return 0;
+
+	ra = list_entry(a, struct dirlist, list);
+	rb = list_entry(b, struct dirlist, list);
+
+	return strcmp(ra->dirname, rb->dirname);
+}
+
 /* fileAction return value of 0 on any file in directory will make
  * recursive_action() return 0, but it doesn't stop directory traversal
  * (fileAction/dirAction will be called on each file).
@@ -55,9 +77,12 @@ int recursive_action(const char *fileName,
 {
 	struct stat statbuf;
 	unsigned follow;
+	unsigned sort;
 	int status;
 	DIR *dir;
 	struct dirent *next;
+	struct dirlist *entry, *entry_tmp;
+	LIST_HEAD(dirs);
 
 	if (!fileAction) fileAction = true_action;
 	if (!dirAction) dirAction = true_action;
@@ -106,20 +131,42 @@ int recursive_action(const char *fileName,
 		/* To trigger: "find -exec rm -rf {} \;" */
 		goto done_nak_warn;
 	}
+	sort = flags & ACTION_SORT;
 	status = 1;
 	while ((next = readdir(dir)) != NULL) {
-		char *nextFile;
-
-		nextFile = concat_subpath_file(fileName, next->d_name);
+		char *nextFile = concat_subpath_file(fileName, next->d_name);
 		if (nextFile == NULL)
 			continue;
-		/* now descend into it, forcing recursion. */
-		if (!recursive_action(nextFile, flags | ACTION_RECURSE,
-				fileAction, dirAction, userData, depth+1)) {
-			status = 0;
+
+		if (sort) {
+			struct dirlist *e = xmalloc(sizeof(*e));
+			e->dirname = xstrdup(next->d_name);
+			list_add(&e->list, &dirs);
+		} else {
+			/* descend into it, forcing recursion. */
+			if (!recursive_action(nextFile, flags | ACTION_RECURSE,
+						fileAction, dirAction, userData, depth+1)) {
+				status = 0;
+			}
 		}
 		free(nextFile);
 	}
+
+	if (sort) {
+		list_sort(NULL, &dirs, &cmp_dirlist);
+
+		list_for_each_entry_safe(entry, entry_tmp, &dirs, list)
+		{
+			/* descend into it, forcing recursion. */
+			if (!recursive_action(entry->dirname, flags | ACTION_RECURSE,
+						fileAction, dirAction, userData, depth+1)) {
+				status = 0;
+			}
+
+			list_del(&entry->list);
+			free(entry->dirname);
+		}
+	}
 	closedir(dir);
 	if ((flags & ACTION_DEPTHFIRST) &&
 		!dirAction(fileName, &statbuf, userData, depth)) {
diff --git a/scripts/bareboxenv.c b/scripts/bareboxenv.c
index 249e65b25..e95bdeaa5 100644
--- a/scripts/bareboxenv.c
+++ b/scripts/bareboxenv.c
@@ -34,6 +34,7 @@
 #include "compiler.h"
 
 #define debug(...)
+#define printk_once(...)
 
 /* Find out if the last character of a string matches the one given.
  * Don't underrun the buffer if the string length is 0.
@@ -54,6 +55,7 @@ enum {
 	ACTION_FOLLOWLINKS = (1 << 1),
 	ACTION_DEPTHFIRST  = (1 << 2),
 	/*ACTION_REVERSE   = (1 << 3), - unused */
+	ACTION_SORT        = (1 << 4),
 };
 
 int recursive_action(const char *fileName, unsigned flags,
@@ -95,6 +97,19 @@ static char *concat_subpath_file(const char *path, const char *f)
 	return concat_path_file(path, f);
 }
 
+static char *xstrdup(const char *s)
+{
+	int len = strlen(s) + 1;
+	char *d = xmalloc(len);
+
+	memcpy(d, s, len);
+
+	return d;
+}
+
+#include <linux/list.h>
+#include <linux/list_sort.h>
+#include "../lib/list_sort.c"
 #include "../lib/recursive_action.c"
 #include "../include/envfs.h"
 #include "../crypto/crc32.c"
diff --git a/scripts/include/linux/list_sort.h b/scripts/include/linux/list_sort.h
new file mode 100644
index 000000000..be889c9ae
--- /dev/null
+++ b/scripts/include/linux/list_sort.h
@@ -0,0 +1 @@
+#include <../../include/linux/list_sort.h>
diff --git a/scripts/include/linux/poison.h b/scripts/include/linux/poison.h
index 0c27bdf14..15927ebc2 100644
--- a/scripts/include/linux/poison.h
+++ b/scripts/include/linux/poison.h
@@ -1 +1,92 @@
-#include "../../../include/linux/poison.h"
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_POISON_H
+#define _LINUX_POISON_H
+
+/********** include/linux/list.h **********/
+
+/*
+ * Architectures might want to move the poison pointer offset
+ * into some well-recognized area such as 0xdead000000000000,
+ * that is also not mappable by user-space exploits:
+ */
+#ifdef CONFIG_ILLEGAL_POINTER_VALUE
+# define POISON_POINTER_DELTA _AC(CONFIG_ILLEGAL_POINTER_VALUE, UL)
+#else
+# define POISON_POINTER_DELTA 0
+#endif
+
+/*
+ * These are non-NULL pointers that will result in page faults
+ * under normal circumstances, used to verify that nobody uses
+ * non-initialized list entries.
+ */
+#define LIST_POISON1  ((void *) 0x100 + POISON_POINTER_DELTA)
+#define LIST_POISON2  ((void *) 0x200 + POISON_POINTER_DELTA)
+
+/********** include/linux/timer.h **********/
+/*
+ * Magic number "tsta" to indicate a static timer initializer
+ * for the object debugging code.
+ */
+#define TIMER_ENTRY_STATIC	((void *) 0x300 + POISON_POINTER_DELTA)
+
+/********** mm/debug-pagealloc.c **********/
+#ifdef CONFIG_PAGE_POISONING_ZERO
+#define PAGE_POISON 0x00
+#else
+#define PAGE_POISON 0xaa
+#endif
+
+/********** mm/page_alloc.c ************/
+
+#define TAIL_MAPPING	((void *) 0x400 + POISON_POINTER_DELTA)
+
+/********** mm/slab.c **********/
+/*
+ * Magic nums for obj red zoning.
+ * Placed in the first word before and the first word after an obj.
+ */
+#define	RED_INACTIVE	0x09F911029D74E35BULL	/* when obj is inactive */
+#define	RED_ACTIVE	0xD84156C5635688C0ULL	/* when obj is active */
+
+#define SLUB_RED_INACTIVE	0xbb
+#define SLUB_RED_ACTIVE		0xcc
+
+/* ...and for poisoning */
+#define	POISON_INUSE	0x5a	/* for use-uninitialised poisoning */
+#define POISON_FREE	0x6b	/* for use-after-free poisoning */
+#define	POISON_END	0xa5	/* end-byte of poisoning */
+
+/********** arch/$ARCH/mm/init.c **********/
+#define POISON_FREE_INITMEM	0xcc
+
+/********** arch/ia64/hp/common/sba_iommu.c **********/
+/*
+ * arch/ia64/hp/common/sba_iommu.c uses a 16-byte poison string with a
+ * value of "SBAIOMMU POISON\0" for spill-over poisoning.
+ */
+
+/********** fs/jbd/journal.c **********/
+#define JBD_POISON_FREE		0x5b
+#define JBD2_POISON_FREE	0x5c
+
+/********** drivers/base/dmapool.c **********/
+#define	POOL_POISON_FREED	0xa7	/* !inuse */
+#define	POOL_POISON_ALLOCATED	0xa9	/* !initted */
+
+/********** drivers/atm/ **********/
+#define ATM_POISON_FREE		0x12
+#define ATM_POISON		0xdeadbeef
+
+/********** kernel/mutexes **********/
+#define MUTEX_DEBUG_INIT	0x11
+#define MUTEX_DEBUG_FREE	0x22
+#define MUTEX_POISON_WW_CTX	((void *) 0x500 + POISON_POINTER_DELTA)
+
+/********** lib/flex_array.c **********/
+#define FLEX_ARRAY_FREE	0x6c	/* for use-after-free poisoning */
+
+/********** security/ **********/
+#define KEY_DESTROY		0xbd
+
+#endif
-- 
2.19.2

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

[PATCH v2 2/2] envfs: new flag for sorting env before saving

[Baeuerle, Florian]

The resulting environment was dependend of the build machines'
filesystem, i.e. the order in which readdir returns dirents depends on
the filesystem implementation.

Use the new flag in scripts/bareboxenv.c for generating a reproducible
built-in environment.

Signed-off-by: Florian Bäuerle <florian.baeuerle@allegion.com>
---
 common/environment.c | 8 ++++++--
 include/envfs.h      | 1 +
 scripts/bareboxenv.c | 2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/common/environment.c b/common/environment.c
index 56a030eda..cea55f313 100644
--- a/common/environment.c
+++ b/common/environment.c
@@ -257,6 +257,7 @@ int envfs_save(const char *filename, const char *dirname, unsigned flags)
 	void *buf = NULL, *wbuf;
 	struct envfs_entry *env;
 	const char *defenv_path = default_environment_path_get();
+	int recursive_flags = ACTION_RECURSE;
 
 	if (!filename)
 		filename = defenv_path;
@@ -276,10 +277,13 @@ int envfs_save(const char *filename, const char *dirname, unsigned flags)
 	if (flags & ENVFS_FLAGS_FORCE_BUILT_IN) {
 		size = 0; /* force no content */
 	} else {
+		if (flags & ENVFS_FLAGS_SORTED)
+			recursive_flags |= ACTION_SORT;
+
 		/* first pass: calculate size */
-		recursive_action(dirname, ACTION_RECURSE, file_action,
+		recursive_action(dirname, recursive_flags, file_action,
 				NULL, &data, 0);
-		recursive_action("/.defaultenv", ACTION_RECURSE,
+		recursive_action("/.defaultenv", recursive_flags,
 				file_remove_action, NULL, &data, 0);
 		size = 0;
 
diff --git a/include/envfs.h b/include/envfs.h
index 27c4b42c6..fa12c60a8 100644
--- a/include/envfs.h
+++ b/include/envfs.h
@@ -45,6 +45,7 @@ struct envfs_super {
 	uint16_t future;		/* reserved for future use */
 	uint32_t flags;			/* feature flags */
 #define ENVFS_FLAGS_FORCE_BUILT_IN	(1 << 0)
+#define ENVFS_FLAGS_SORTED		(1 << 1)
 	uint32_t sb_crc;		/* crc for the superblock */
 };
 
diff --git a/scripts/bareboxenv.c b/scripts/bareboxenv.c
index e95bdeaa5..c27e78e94 100644
--- a/scripts/bareboxenv.c
+++ b/scripts/bareboxenv.c
@@ -136,7 +136,7 @@ int main(int argc, char *argv[])
 	int opt;
 	int save = 0, load = 0, pad = 0, err = 0, fd;
 	char *filename = NULL, *dirname = NULL;
-	unsigned envfs_flags = 0;
+	unsigned envfs_flags = ENVFS_FLAGS_SORTED;
 	int verbose = 0;
 
 	while((opt = getopt(argc, argv, "slp:vz")) != -1) {
-- 
2.19.2

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

Re: [PATCH v2 1/2] recursive_action: add ACTION_SORT flag

[Sascha Hauer]

Hi Florian,

On Wed, Dec 19, 2018 at 02:02:03PM +0000, Baeuerle, Florian wrote:
> Add a flag to sort directory entries before recursing into them.
> 
> Since this part of lib/ is used inside barebox as well as in
> scripts/bareboxenv.c, we cannot easily use stringlists from lib/, which
> would have made the code a bit nicer.
> 
> Further, add poison.h from kernel 4.5-rc1, which was the base for
> importing linux headers under scripts/ in a883d9a. This is required for
> actually using kernel linked lists within scripts/ tools.

I applied this with some changes. It was important for me that the sort
support does not leak into the barebox image so we do not increase the
binary size. Please check the result in the -next branch, it should do
what you need.

Sascha

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

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

Re: [PATCH v2 1/2] recursive_action: add ACTION_SORT flag

[Baeuerle, Florian]

Hi Sascha,

thanks a lot!

There's two things I noticed when reviewing the patches:

The commit message of 47866ba5 is a bit misleading since poison.h is no longer
being added in the commit.

861ce6d looks a bit weird, I guess that's a typo (ACTION_SORT | ACTION_SORT). 


- Florian

Am Donnerstag, den 03.01.2019, 11:59 +0100 schrieb Sascha Hauer:
> Hi Florian,
> 
> On Wed, Dec 19, 2018 at 02:02:03PM +0000, Baeuerle, Florian wrote:
> > Add a flag to sort directory entries before recursing into them.
> > 
> > Since this part of lib/ is used inside barebox as well as in
> > scripts/bareboxenv.c, we cannot easily use stringlists from lib/, which
> > would have made the code a bit nicer.
> > 
> > Further, add poison.h from kernel 4.5-rc1, which was the base for
> > importing linux headers under scripts/ in a883d9a. This is required for
> > actually using kernel linked lists within scripts/ tools.
> 
> I applied this with some changes. It was important for me that the sort
> support does not leak into the barebox image so we do not increase the
> binary size. Please check the result in the -next branch, it should do
> what you need.
> 
> Sascha
> 
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 1/2] recursive_action: add ACTION_SORT flag

[s.hauer]

On Thu, Jan 03, 2019 at 11:53:27AM +0000, Baeuerle, Florian wrote:
> Hi Sascha,
> 
> thanks a lot!
> 
> There's two things I noticed when reviewing the patches:
> 
> The commit message of 47866ba5 is a bit misleading since poison.h is no longer
> being added in the commit.
> 
> 861ce6d looks a bit weird, I guess that's a typo (ACTION_SORT | ACTION_SORT). 

Ups, indeed. I fixed that.

Thanks,
Sascha

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

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

Re: [PATCH v2 1/2] recursive_action: add ACTION_SORT flag

[Baeuerle, Florian]

NAK! NAK!

There are two terrible mistakes - indeed the patches produce a very reproducible
environment. Independent of what environment you want, you'd get an empty one.

Problem 1: a2f3b3d
it requires ACTION_RECURSE | ACTION_SORT

Problem 2: f5c0978
+			e->dirname = xstrdup(next->d_name);
should be much more like:
+			e->dirname = xstrdup(fileName);

Sorry for that, problem 2 should really *not* have happened.


- Florian

Am Montag, den 07.01.2019, 08:58 +0100 schrieb s.hauer@pengutronix.de:
> On Thu, Jan 03, 2019 at 11:53:27AM +0000, Baeuerle, Florian wrote:
> > Hi Sascha,
> > 
> > thanks a lot!
> > 
> > There's two things I noticed when reviewing the patches:
> > 
> > The commit message of 47866ba5 is a bit misleading since poison.h is no
> > longer
> > being added in the commit.
> > 
> > 861ce6d looks a bit weird, I guess that's a typo (ACTION_SORT |
> > ACTION_SORT). 
> 
> Ups, indeed. I fixed that.
> 
> Thanks,
> Sascha
> 
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] envfs: fix problem #1

[Baeuerle, Florian]

Signed-off-by: Florian Bäuerle <florian.baeuerle@allegion.com>
---
 common/environment.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/environment.c b/common/environment.c
index 19df74290..aba6dcde4 100644
--- a/common/environment.c
+++ b/common/environment.c
@@ -277,9 +277,9 @@ int envfs_save(const char *filename, const char *dirname, unsigned flags)
 		size = 0; /* force no content */
 	} else {
 		/* first pass: calculate size */
-		recursive_action(dirname, ACTION_SORT, file_action,
+		recursive_action(dirname, ACTION_RECURSE | ACTION_SORT, file_action,
 				NULL, &data, 0);
-		recursive_action("/.defaultenv", ACTION_SORT,
+		recursive_action("/.defaultenv", ACTION_RECURSE | ACTION_SORT,
 				file_remove_action, NULL, &data, 0);
 		size = 0;
 
-- 
2.19.2

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

[PATCH 2/2] envfs: fix problem #2

[Baeuerle, Florian]

Signed-off-by: Florian Bäuerle <florian.baeuerle@allegion.com>
---
 lib/recursive_action.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/recursive_action.c b/lib/recursive_action.c
index 4ccddc39b..9505c8628 100644
--- a/lib/recursive_action.c
+++ b/lib/recursive_action.c
@@ -149,7 +149,7 @@ int recursive_action(const char *fileName,
 
 		if (DO_SORT(flags)) {
 			struct dirlist *e = xmalloc(sizeof(*e));
-			e->dirname = xstrdup(next->d_name);
+			e->dirname = nextFile;
 			list_add(&e->list, &dirs);
 		} else {
 			/* descend into it, forcing recursion. */
@@ -157,8 +157,8 @@ int recursive_action(const char *fileName,
 						fileAction, dirAction, userData, depth+1)) {
 				status = 0;
 			}
+			free(nextFile);
 		}
-		free(nextFile);
 	}
 
 	if (DO_SORT(flags)) {
-- 
2.19.2

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

Re: [PATCH v2 1/2] recursive_action: add ACTION_SORT flag

[s.hauer]

On Tue, Jan 08, 2019 at 09:47:04AM +0000, Baeuerle, Florian wrote:
> NAK! NAK!
> 
> There are two terrible mistakes - indeed the patches produce a very reproducible
> environment. Independent of what environment you want, you'd get an empty one.
> 
> Problem 1: a2f3b3d
> it requires ACTION_RECURSE | ACTION_SORT
> 
> Problem 2: f5c0978
> +			e->dirname = xstrdup(next->d_name);
> should be much more like:
> +			e->dirname = xstrdup(fileName);
> 
> Sorry for that, problem 2 should really *not* have happened.

Don't worry, it's still in -next and not yet in master.
I squashed your fixups into the broken commits.

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