libfdt: Ensure fdt_add_property frees allocated name string on failure

If fdt_add_property or fdt_property_placeholder fail after allocating
a string for the name, they return without freeing that string. This
does not change the structure of the tree, but in very specific cases
it could lead to undesirable space consumption.

Fix this by rolling back the string allocation in this situation.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Message-Id: <20190509094122.834-2-npiggin@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

4 files changed, 100 insertions, 1 deletions

diff --git a/tests/.gitignore b/tests/.gitignore
index 0bc78aa..d3f1434 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -56,6 +56,7 @@ tmp.*
 /references
 /root_node
 /rw_tree1
+/rw_oom
 /set_name
 /setprop
 /setprop_inplace
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index 1f8feed..5093aaa 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -15,7 +15,7 @@ LIB_TESTS_L = get_mem_rsv \
 	setprop_inplace nop_property nop_node \
 	sw_tree1 sw_states \
 	move_and_save mangle-layout nopulate \
-	open_pack rw_tree1 set_name setprop del_property del_node \
+	open_pack rw_tree1 rw_oom set_name setprop del_property del_node \
 	appendprop1 appendprop2 propname_escapes \
 	string_escapes references path-references phandle_format \
 	boot-cpuid incbin \
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 489a3d2..d350c3d 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -421,6 +421,8 @@ libfdt_tests () {
 	tree1_tests_rw noppy.$basetree
     done
 
+    run_test rw_oom
+
     run_dtc_test -I dts -O dtb -o subnode_iterate.dtb subnode_iterate.dts
     run_test subnode_iterate subnode_iterate.dtb
 
diff --git a/tests/rw_oom.c b/tests/rw_oom.c
new file mode 100644
index 0000000..a787dc2
--- /dev/null
+++ b/tests/rw_oom.c
@@ -0,0 +1,96 @@
+/*
+ * libfdt - Flat Device Tree manipulation
+ *	Testcase for fdt_nop_node()
+ * Copyright (C) 2006 David Gibson, IBM Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <ctype.h>
+#include <stdint.h>
+
+#include <libfdt.h>
+
+#include "tests.h"
+#include "testdata.h"
+
+/* This is not derived programatically. May require adjustment to changes. */
+#define SPACE	285
+
+#define CHECK(code) \
+	{ \
+		err = (code); \
+		if (err) \
+			FAIL(#code ": %s", fdt_strerror(err)); \
+	}
+
+#define OFF_CHECK(off, code) \
+	{ \
+		(off) = (code); \
+		if (off < 0) \
+			FAIL(#code ": %s", fdt_strerror(off)); \
+	}
+
+int main(int argc, char *argv[])
+{
+	void *fdt;
+	int err;
+	int offset, s1;
+	int strsize1, strsize2;
+
+	/*
+	 * Check OOM path, and check that property is cleaned up if it fails
+	 * with OOM, rather than adding an orphan name.
+	 *
+	 * SW OOM is tested with the realloc/resize strategies.
+	 */
+	test_init(argc, argv);
+
+	fdt = xmalloc(SPACE);
+
+	/* First create empty tree with SW */
+	CHECK(fdt_create_empty_tree(fdt, SPACE));
+
+	CHECK(fdt_add_mem_rsv(fdt, TEST_ADDR_1, TEST_SIZE_1));
+	CHECK(fdt_add_mem_rsv(fdt, TEST_ADDR_2, TEST_SIZE_2));
+
+	CHECK(fdt_setprop_string(fdt, 0, "compatible", "test_oom"));
+	CHECK(fdt_setprop_u32(fdt, 0, "prop-int", TEST_VALUE_1));
+	CHECK(fdt_setprop_u64(fdt, 0, "prop-int64", TEST_VALUE64_1));
+	CHECK(fdt_setprop_string(fdt, 0, "prop-str", TEST_STRING_1));
+
+	OFF_CHECK(offset, fdt_add_subnode(fdt, 0, "subnode@1"));
+	s1 = offset;
+
+	strsize1 = fdt_size_dt_strings(fdt);
+	err = fdt_setprop_string(fdt, s1, "unique", "subnode1");
+	if (err != -FDT_ERR_NOSPACE)
+		FAIL("fdt_setprop_string(fdt, s1, \"compatible\", \"subnode1\"): %s", fdt_strerror(err));
+	strsize2 = fdt_size_dt_strings(fdt);
+
+	if (strsize1 != strsize2)
+		FAIL("fdt_setprop NOSPACE error failed to clean up allocated string\n");
+	err = 0;
+
+	/* Ensure we failed in the right place */
+	CHECK(fdt_setprop_string(fdt, s1, "unique", ""));
+
+	CHECK(fdt_pack(fdt));
+
+	PASS();
+}