From 9d2d2c49f053018724bcc6e37029da10b7c3d60d Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Mon, 17 Dec 2018 10:46:32 +1030
Subject: [PATCH 1/1] tal: handle take() pointers more carefully.

take() applies to the literal pointer value, so tal_free() means it may
randomly apply to a future allocation.  Similarly, tal_resize() should
preserve it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 ccan/tal/tal.c                       |  6 ++++
 ccan/tal/tal.h                       |  3 ++
 ccan/tal/test/run-take-free-assert.c | 44 ++++++++++++++++++++++++++++
 ccan/tal/test/run-take.c             |  8 ++++-
 4 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 ccan/tal/test/run-take-free-assert.c

diff --git a/ccan/tal/tal.c b/ccan/tal/tal.c
index f6e2ee79..dff0dcfb 100644
--- a/ccan/tal/tal.c
+++ b/ccan/tal/tal.c
@@ -385,6 +385,8 @@ static void del_tree(struct tal_hdr *t, const tal_t *orig, int saved_errno)
 {
 	struct prop_hdr **prop, *p, *next;
 
+	assert(!taken(from_tal_hdr(t)));
+
         /* Already being destroyed?  Don't loop. */
         if (unlikely(get_destroying_bit(t->parent_child)))
                 return;
@@ -710,6 +712,10 @@ bool tal_resize_(tal_t **ctxp, size_t size, size_t count, bool clear)
 		/* Fix up linked list pointers. */
 		t->list.next->prev = t->list.prev->next = &t->list;
 
+		/* Copy take() property. */
+		if (taken(from_tal_hdr(old_t)))
+			take(from_tal_hdr(t));
+
 		/* Fix up child property's parent pointer. */
 		child = find_property(t, CHILDREN);
 		if (child) {
diff --git a/ccan/tal/tal.h b/ccan/tal/tal.h
index 865a6967..e25dcb92 100644
--- a/ccan/tal/tal.h
+++ b/ccan/tal/tal.h
@@ -106,6 +106,9 @@ void *tal_free(const tal_t *p);
  * This returns true on success (and may move *@p), or false on failure.
  * On success, tal_count() of *@p will be @count.
  *
+ * Note: if *p is take(), it will still be take() upon return, even if it
+ * has been moved.
+ *
  * Example:
  *	tal_resize(&p, 100);
  */
diff --git a/ccan/tal/test/run-take-free-assert.c b/ccan/tal/test/run-take-free-assert.c
new file mode 100644
index 00000000..062a778a
--- /dev/null
+++ b/ccan/tal/test/run-take-free-assert.c
@@ -0,0 +1,44 @@
+#include <ccan/tal/tal.h>
+#include <ccan/tal/tal.c>
+#include <ccan/tap/tap.h>
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+int main(void)
+{
+	int status;
+	char *p;
+
+	plan_tests(4);
+
+	/* Test direct free. */
+	switch (fork()) {
+	case 0:
+		tal_free(take(tal(NULL, char)));
+		exit(2);
+	case -1:
+		exit(1);
+	default:
+		wait(&status);
+		ok1(WIFSIGNALED(status));
+		ok1(WTERMSIG(status) == SIGABRT);
+	}
+
+	/* Test indirect free. */
+	switch (fork()) {
+	case 0:
+		p = tal(NULL, char);
+		take(tal(p, char));
+		tal_free(p);
+		exit(2);
+	case -1:
+		exit(1);
+	default:
+		wait(&status);
+		ok1(WIFSIGNALED(status));
+		ok1(WTERMSIG(status) == SIGABRT);
+	}
+	return exit_status();
+}
diff --git a/ccan/tal/test/run-take.c b/ccan/tal/test/run-take.c
index d93304e2..4a7f0c74 100644
--- a/ccan/tal/test/run-take.c
+++ b/ccan/tal/test/run-take.c
@@ -6,7 +6,7 @@ int main(void)
 {
 	char *parent, *c;
 
-	plan_tests(21);
+	plan_tests(22);
 
 	/* We can take NULL. */
 	ok1(take(NULL) == NULL);
@@ -44,6 +44,12 @@ int main(void)
 	tal_free(c);
 	ok1(tal_first(parent) == NULL);
 
+	/* tal_resize should return a taken pointer. */
+	c = take(tal_arr(parent, char, 5));
+	tal_resize(&c, 100);
+	ok1(taken(c));
+	tal_free(c);
+
 	tal_free(parent);
 	ok1(!taken_any());
 
-- 
2.39.2