From 144552c786925314c1e7cb8f91a71dae1aca8798 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Thu, 4 Oct 2018 20:24:17 -0700 Subject: of: overlay: add tests to validate kfrees from overlay removal Add checks: - attempted kfree due to refcount reaching zero before overlay is removed - properties linked to an overlay node when the node is removed - node refcount > one during node removal in a changeset destroy, if the node was created by the changeset After applying this patch, several validation warnings will be reported from the devicetree unittest during boot due to pre-existing devicetree bugs. The warnings will be similar to: OF: ERROR: of_node_release(), unexpected properties in /testcase-data/overlay-node/test-bus/test-unittest11 OF: ERROR: memory leak, expected refcount 1 instead of 2, of_node_get()/of_node_put() unbalanced - destroy cset entry: attach overlay node /testcase-data-2/substation@100/ hvac-medium-2 Tested-by: Alan Tull Signed-off-by: Frank Rowand --- include/linux/of.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'include/linux') diff --git a/include/linux/of.h b/include/linux/of.h index a5aee3c438ad..664cd5573ae2 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -138,11 +138,16 @@ extern struct device_node *of_aliases; extern struct device_node *of_stdout; extern raw_spinlock_t devtree_lock; -/* flag descriptions (need to be visible even when !CONFIG_OF) */ -#define OF_DYNAMIC 1 /* node and properties were allocated via kmalloc */ -#define OF_DETACHED 2 /* node has been detached from the device tree */ -#define OF_POPULATED 3 /* device already created for the node */ -#define OF_POPULATED_BUS 4 /* of_platform_populate recursed to children of this node */ +/* + * struct device_node flag descriptions + * (need to be visible even when !CONFIG_OF) + */ +#define OF_DYNAMIC 1 /* (and properties) allocated via kmalloc */ +#define OF_DETACHED 2 /* detached from the device tree */ +#define OF_POPULATED 3 /* device already created */ +#define OF_POPULATED_BUS 4 /* platform bus created for children */ +#define OF_OVERLAY 5 /* allocated for an overlay */ +#define OF_OVERLAY_FREE_CSET 6 /* in overlay cset being freed */ #define OF_BAD_ADDR ((u64)-1) -- cgit v1.2.3 From 6f75118800acf77f8ad6afec61ca1b2349ade371 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Thu, 4 Oct 2018 20:32:04 -0700 Subject: of: overlay: validate overlay properties #address-cells and #size-cells If overlay properties #address-cells or #size-cells are already in the live devicetree for any given node, then the values in the overlay must match the values in the live tree. If the properties are already in the live tree then there is no need to create a changeset entry to add them since they must have the same value. This reduces the memory used by the changeset and eliminates a possible memory leak. Tested-by: Alan Tull Signed-off-by: Frank Rowand --- drivers/of/overlay.c | 32 +++++++++++++++++++++++++++++--- include/linux/of.h | 6 ++++++ 2 files changed, 35 insertions(+), 3 deletions(-) (limited to 'include/linux') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 15be3da34fef..72bf00adb9c8 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -287,7 +287,12 @@ err_free_target_path: * @target may be either in the live devicetree or in a new subtree that * is contained in the changeset. * - * Some special properties are not updated (no error returned). + * Some special properties are not added or updated (no error returned): + * "name", "phandle", "linux,phandle". + * + * Properties "#address-cells" and "#size-cells" are not updated if they + * are already in the live tree, but if present in the live tree, the values + * in the overlay must match the values in the live tree. * * Update of property in symbols node is not allowed. * @@ -300,6 +305,7 @@ static int add_changeset_property(struct overlay_changeset *ovcs, { struct property *new_prop = NULL, *prop; int ret = 0; + bool check_for_non_overlay_node = false; if (!of_prop_cmp(overlay_prop->name, "name") || !of_prop_cmp(overlay_prop->name, "phandle") || @@ -322,12 +328,32 @@ static int add_changeset_property(struct overlay_changeset *ovcs, if (!new_prop) return -ENOMEM; - if (!prop) + if (!prop) { + check_for_non_overlay_node = true; ret = of_changeset_add_property(&ovcs->cset, target->np, new_prop); - else + } else if (!of_prop_cmp(prop->name, "#address-cells")) { + if (!of_prop_val_eq(prop, new_prop)) { + pr_err("ERROR: changing value of #address-cells is not allowed in %pOF\n", + target->np); + ret = -EINVAL; + } + } else if (!of_prop_cmp(prop->name, "#size-cells")) { + if (!of_prop_val_eq(prop, new_prop)) { + pr_err("ERROR: changing value of #size-cells is not allowed in %pOF\n", + target->np); + ret = -EINVAL; + } + } else { + check_for_non_overlay_node = true; ret = of_changeset_update_property(&ovcs->cset, target->np, new_prop); + } + + if (check_for_non_overlay_node && + !of_node_check_flag(target->np, OF_OVERLAY)) + pr_err("WARNING: memory leak will occur if overlay removed, property: %pOF/%s\n", + target->np, new_prop->name); if (ret) { kfree(new_prop->name); diff --git a/include/linux/of.h b/include/linux/of.h index 664cd5573ae2..18ac8921e90c 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -990,6 +990,12 @@ static inline int of_map_rid(struct device_node *np, u32 rid, #define of_node_cmp(s1, s2) strcasecmp((s1), (s2)) #endif +static inline int of_prop_val_eq(struct property *p1, struct property *p2) +{ + return p1->length == p2->length && + !memcmp(p1->value, p2->value, (size_t)p1->length); +} + #if defined(CONFIG_OF) && defined(CONFIG_NUMA) extern int of_node_to_nid(struct device_node *np); #else -- cgit v1.2.3 From 1ae367a2451e0b249074461d2d8ac76d8e929a53 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Tue, 6 Nov 2018 18:07:37 -0600 Subject: of/pdt: Remove unused of_pdt_build_more function ptr There are no users of of_pdt_build_more since 2012, so remove it. Cc: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/pdt.c | 5 ----- include/linux/of_pdt.h | 2 -- 2 files changed, 7 deletions(-) (limited to 'include/linux') diff --git a/drivers/of/pdt.c b/drivers/of/pdt.c index 013e65de074a..4fc0fd96ed04 100644 --- a/drivers/of/pdt.c +++ b/drivers/of/pdt.c @@ -21,8 +21,6 @@ static struct of_pdt_ops *of_pdt_prom_ops __initdata; -void __initdata (*of_pdt_build_more)(struct device_node *dp); - #if defined(CONFIG_SPARC) unsigned int of_pdt_unique_id __initdata; @@ -208,9 +206,6 @@ static struct device_node * __init of_pdt_build_tree(struct device_node *parent, dp->child = of_pdt_build_tree(dp, of_pdt_prom_ops->getchild(node)); - if (of_pdt_build_more) - of_pdt_build_more(dp); - node = of_pdt_prom_ops->getsibling(node); } diff --git a/include/linux/of_pdt.h b/include/linux/of_pdt.h index d0b183ab65c6..89e4eb076a01 100644 --- a/include/linux/of_pdt.h +++ b/include/linux/of_pdt.h @@ -35,6 +35,4 @@ extern void *prom_early_alloc(unsigned long size); /* for building the device tree */ extern void of_pdt_build_devicetree(phandle root_node, struct of_pdt_ops *ops); -extern void (*of_pdt_build_more)(struct device_node *dp); - #endif /* _LINUX_OF_PDT_H */ -- cgit v1.2.3 From b1ab95c63622e9d9bd0ce685e149034d393afc2e Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Mon, 5 Nov 2018 14:54:27 -0800 Subject: arch: Make phys_initrd_start and phys_initrd_size global variables Make phys_initrd_start and phys_initrd_size global variables declared in init/do_mounts_initrd.c such that we can later have generic code in drivers/of/fdt.c populate those variables for us. This requires both the ARM and unicore32 implementations to be properly guarded against CONFIG_BLK_DEV_INITRD, and also initialize the variables to the expected default values (unicore32). Signed-off-by: Florian Fainelli Reviewed-by: Mike Rapoport Signed-off-by: Rob Herring --- arch/arm/mm/init.c | 5 ++--- arch/unicore32/mm/init.c | 10 +++++++--- include/linux/initrd.h | 3 +++ init/do_mounts_initrd.c | 3 +++ 4 files changed, 15 insertions(+), 6 deletions(-) (limited to 'include/linux') diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c index 32e4845af2b6..438625764ccd 100644 --- a/arch/arm/mm/init.c +++ b/arch/arm/mm/init.c @@ -50,9 +50,7 @@ unsigned long __init __clear_cr(unsigned long mask) } #endif -static phys_addr_t phys_initrd_start __initdata = 0; -static unsigned long phys_initrd_size __initdata = 0; - +#ifdef CONFIG_BLK_DEV_INITRD static int __init early_initrd(char *p) { phys_addr_t start; @@ -89,6 +87,7 @@ static int __init parse_tag_initrd2(const struct tag *tag) } __tagtable(ATAG_INITRD2, parse_tag_initrd2); +#endif static void __init find_limits(unsigned long *min, unsigned long *max_low, unsigned long *max_high) diff --git a/arch/unicore32/mm/init.c b/arch/unicore32/mm/init.c index cf4eb9481fd6..02aa2c0b295e 100644 --- a/arch/unicore32/mm/init.c +++ b/arch/unicore32/mm/init.c @@ -30,9 +30,7 @@ #include "mm.h" -static unsigned long phys_initrd_start __initdata = 0x01000000; -static unsigned long phys_initrd_size __initdata = SZ_8M; - +#ifdef CONFIG_BLK_DEV_INITRD static int __init early_initrd(char *p) { unsigned long start, size; @@ -48,6 +46,7 @@ static int __init early_initrd(char *p) return 0; } early_param("initrd", early_initrd); +#endif /* * This keeps memory configuration data used by a couple memory @@ -156,6 +155,11 @@ void __init uc32_memblock_init(struct meminfo *mi) memblock_reserve(__pa(_text), _end - _text); #ifdef CONFIG_BLK_DEV_INITRD + if (!phys_initrd_size) { + phys_initrd_start = 0x01000000; + phys_initrd_size = SZ_8M; + } + if (phys_initrd_size) { memblock_reserve(phys_initrd_start, phys_initrd_size); diff --git a/include/linux/initrd.h b/include/linux/initrd.h index 84b423044088..14beaff9b445 100644 --- a/include/linux/initrd.h +++ b/include/linux/initrd.h @@ -21,4 +21,7 @@ extern int initrd_below_start_ok; extern unsigned long initrd_start, initrd_end; extern void free_initrd_mem(unsigned long, unsigned long); +extern phys_addr_t phys_initrd_start; +extern unsigned long phys_initrd_size; + extern unsigned int real_root_dev; diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c index d1a5d885ce13..45865b72f4ea 100644 --- a/init/do_mounts_initrd.c +++ b/init/do_mounts_initrd.c @@ -16,6 +16,9 @@ int initrd_below_start_ok; unsigned int real_root_dev; /* do_proc_dointvec cannot handle kdev_t */ static int __initdata mount_initrd = 1; +phys_addr_t phys_initrd_start __initdata; +unsigned long phys_initrd_size __initdata; + static int __init no_initrd(char *str) { mount_initrd = 0; -- cgit v1.2.3