Force manifest booleans to have empty values.
Allowing values such as `<0>` or `"false"` be considered true has the
potential to cause confusion so only accept empty properties as booleans.
Change-Id: I8a9db6a97fc8687ac336138b7b8fc10eee69ae91
diff --git a/inc/hf/manifest.h b/inc/hf/manifest.h
index cfedc61..0e628e5 100644
--- a/inc/hf/manifest.h
+++ b/inc/hf/manifest.h
@@ -66,6 +66,7 @@
MANIFEST_ERROR_MALFORMED_INTEGER,
MANIFEST_ERROR_INTEGER_OVERFLOW,
MANIFEST_ERROR_MALFORMED_INTEGER_LIST,
+ MANIFEST_ERROR_MALFORMED_BOOLEAN,
};
enum manifest_return_code manifest_init(struct manifest *manifest,
diff --git a/src/manifest.c b/src/manifest.c
index 1933d10..c3b5566 100644
--- a/src/manifest.c
+++ b/src/manifest.c
@@ -55,21 +55,22 @@
}
/**
- * Read a boolean property: true if present; false if not. The value of the
- * property is ignored.
- *
- * This is the convention used by Linux but beware of things like the following
- * that will actually be considered as `true`.
- *
- * true-property0 = <0>;
- * true-property1 = "false";
+ * Read a boolean property: true if present; false if not. If present, the value
+ * of the property must be empty else it is considered malformed.
*/
-static bool read_bool(const struct fdt_node *node, const char *property)
+static enum manifest_return_code read_bool(const struct fdt_node *node,
+ const char *property, bool *out)
{
const char *data;
uint32_t size;
+ bool present = fdt_read_property(node, property, &data, &size);
- return fdt_read_property(node, property, &data, &size);
+ if (present && size != 0) {
+ return MANIFEST_ERROR_MALFORMED_BOOLEAN;
+ }
+
+ *out = present;
+ return MANIFEST_SUCCESS;
}
static enum manifest_return_code read_string(const struct fdt_node *node,
@@ -286,8 +287,8 @@
dlog("%s SMC whitelist too long.\n", vm->debug_name);
}
- vm->smc_whitelist.permissive =
- read_bool(node, "smc_whitelist_permissive");
+ TRY(read_bool(node, "smc_whitelist_permissive",
+ &vm->smc_whitelist.permissive));
if (vm_id == HF_PRIMARY_VM_ID) {
TRY(read_optional_string(node, "ramdisk_filename",
@@ -396,6 +397,8 @@
return "Integer overflow";
case MANIFEST_ERROR_MALFORMED_INTEGER_LIST:
return "Malformed integer list property";
+ case MANIFEST_ERROR_MALFORMED_BOOLEAN:
+ return "Malformed boolean property";
}
panic("Unexpected manifest return code.");
diff --git a/src/manifest_test.cc b/src/manifest_test.cc
index 03b6a56..fdfd7f5 100644
--- a/src/manifest_test.cc
+++ b/src/manifest_test.cc
@@ -473,50 +473,47 @@
ASSERT_STREQ(string_data(&m.vm[0].primary.ramdisk_filename), "");
}
-TEST(manifest, true_booleans_with_values)
+static std::vector<char> gen_malformed_boolean_dtb(
+ const std::string_view &value)
{
- struct manifest m;
- struct fdt_node fdt_root;
-
/* clang-format off */
- std::vector<char> dtb = ManifestDtBuilder()
+ return ManifestDtBuilder()
.StartChild("hypervisor")
.Compatible()
.StartChild("vm1")
.DebugName("primary_vm")
- .Property("smc_whitelist_permissive", "\"false\"")
- .EndChild()
- .StartChild("vm2")
- .DebugName("first_secondary_vm")
- .VcpuCount(42)
- .MemSize(12345)
- .Property("smc_whitelist_permissive", "<0>")
- .EndChild()
- .StartChild("vm3")
- .DebugName("second_secondary_vm")
- .VcpuCount(43)
- .MemSize(0x12345)
- .Property("smc_whitelist_permissive", "\"true\"")
- .EndChild()
- .StartChild("vm4")
- .DebugName("tertiary_secondary_vm")
- .VcpuCount(44)
- .MemSize(0x55)
- .Property("smc_whitelist_permissive", "<1>")
+ .Property("smc_whitelist_permissive", value)
.EndChild()
.EndChild()
.Build();
/* clang-format on */
+}
- ASSERT_TRUE(get_fdt_root(dtb, &fdt_root));
+TEST(manifest, malformed_booleans)
+{
+ struct manifest m;
+ struct fdt_node fdt_root;
- ASSERT_EQ(manifest_init(&m, &fdt_root), MANIFEST_SUCCESS);
- ASSERT_EQ(m.vm_count, 4);
+ std::vector<char> dtb_false = gen_malformed_boolean_dtb("\"false\"");
+ std::vector<char> dtb_true = gen_malformed_boolean_dtb("\"true\"");
+ std::vector<char> dtb_0 = gen_malformed_boolean_dtb("\"<0>\"");
+ std::vector<char> dtb_1 = gen_malformed_boolean_dtb("\"<1>\"");
- ASSERT_TRUE(m.vm[0].smc_whitelist.permissive);
- ASSERT_TRUE(m.vm[1].smc_whitelist.permissive);
- ASSERT_TRUE(m.vm[2].smc_whitelist.permissive);
- ASSERT_TRUE(m.vm[3].smc_whitelist.permissive);
+ ASSERT_TRUE(get_fdt_root(dtb_false, &fdt_root));
+ ASSERT_EQ(manifest_init(&m, &fdt_root),
+ MANIFEST_ERROR_MALFORMED_BOOLEAN);
+
+ ASSERT_TRUE(get_fdt_root(dtb_true, &fdt_root));
+ ASSERT_EQ(manifest_init(&m, &fdt_root),
+ MANIFEST_ERROR_MALFORMED_BOOLEAN);
+
+ ASSERT_TRUE(get_fdt_root(dtb_0, &fdt_root));
+ ASSERT_EQ(manifest_init(&m, &fdt_root),
+ MANIFEST_ERROR_MALFORMED_BOOLEAN);
+
+ ASSERT_TRUE(get_fdt_root(dtb_1, &fdt_root));
+ ASSERT_EQ(manifest_init(&m, &fdt_root),
+ MANIFEST_ERROR_MALFORMED_BOOLEAN);
}
TEST(manifest, valid)
