Skip to content

Commit 39a751a

Browse files
frowandfrowand
committed
of: change overlay apply input data from unflattened to FDT
Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. of_overlay_fdt_apply() is a private function for the anticipated overlay loader. Update unittest.c to use of_overlay_fdt_apply() instead of of_overlay_apply(). Move overlay fragments from artificial locations in drivers/of/unittest-data/tests-overlay.dtsi into one devicetree source file per overlay. This led to changes in drivers/of/unitest-data/Makefile and drivers/of/unitest.c. - Add overlay directives to the overlay devicetree source files so that dtc will compile them as true overlays into one FDT data chunk per overlay. - Set CFLAGS for drivers/of/unittest-data/testcases.dts so that symbols will be generated for overlay resolution of overlays that are no longer artificially contained in testcases.dts - Unflatten and apply each unittest overlay FDT using of_overlay_fdt_apply(). - Enable the of_resolve_phandles() check for whether the unflattened overlay is detached. This check was previously disabled because the overlays from tests-overlay.dtsi were not unflattened into detached trees. - Other changes to unittest.c infrastructure to manage multiple test FDTs built into the kernel image (access by name instead of arbitrary number). - of_unittest_overlay_high_level(): previously unused code to add properties from the overlay_base devicetree to the live tree was triggered by the restructuring of tests-overlay.dtsi and thus testcases.dts. This exposed two bugs: (1) the need to dup a property before adding it, and (2) property 'name' is auto-generated in the unflatten code and thus will be a duplicate in the __symbols__ node - do not treat this duplicate as an error. Signed-off-by: Frank Rowand <frank.rowand@sony.com>
1 parent 581e929 commit 39a751a

22 files changed

+562
-388
lines changed

‎drivers/of/Kconfig

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ config OF_RESOLVE
9292
config OF_OVERLAY
9393
bool "Device Tree overlays"
9494
select OF_DYNAMIC
95+
select OF_FLATTREE
9596
select OF_RESOLVE
9697
help
9798
Overlays are a method to dynamically modify part of the kernel's

‎drivers/of/overlay.c

Lines changed: 103 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@
1212
#include <linux/module.h>
1313
#include <linux/of.h>
1414
#include <linux/of_device.h>
15+
#include <linux/of_fdt.h>
1516
#include <linux/string.h>
1617
#include <linux/ctype.h>
1718
#include <linux/errno.h>
1819
#include <linux/slab.h>
20+
#include <linux/libfdt.h>
1921
#include <linux/err.h>
2022
#include <linux/idr.h>
2123

@@ -33,7 +35,9 @@ struct fragment {
3335

3436
/**
3537
* struct overlay_changeset
38+
* @id: changeset identifier
3639
* @ovcs_list: list on which we are located
40+
* @fdt: FDT that was unflattened to create @overlay_tree
3741
* @overlay_tree: expanded device tree that contains the fragment nodes
3842
* @count: count of fragment structures
3943
* @fragments: fragment nodes in the overlay expanded device tree
@@ -43,6 +47,7 @@ struct fragment {
4347
struct overlay_changeset {
4448
int id;
4549
struct list_head ovcs_list;
50+
const void *fdt;
4651
struct device_node *overlay_tree;
4752
int count;
4853
struct fragment *fragments;
@@ -503,7 +508,8 @@ static struct device_node *find_target_node(struct device_node *info_node)
503508

504509
/**
505510
* init_overlay_changeset() - initialize overlay changeset from overlay tree
506-
* @ovcs Overlay changeset to build
511+
* @ovcs: Overlay changeset to build
512+
* @fdt: the FDT that was unflattened to create @tree
507513
* @tree: Contains all the overlay fragments and overlay fixup nodes
508514
*
509515
* Initialize @ovcs. Populate @ovcs->fragments with node information from
@@ -514,7 +520,7 @@ static struct device_node *find_target_node(struct device_node *info_node)
514520
* detected in @tree, or -ENOSPC if idr_alloc() error.
515521
*/
516522
static int init_overlay_changeset(struct overlay_changeset *ovcs,
517-
struct device_node *tree)
523+
const void *fdt, struct device_node *tree)
518524
{
519525
struct device_node *node, *overlay_node;
520526
struct fragment *fragment;
@@ -535,6 +541,7 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs,
535541
pr_debug("%s() tree is not root\n", __func__);
536542

537543
ovcs->overlay_tree = tree;
544+
ovcs->fdt = fdt;
538545

539546
INIT_LIST_HEAD(&ovcs->ovcs_list);
540547

@@ -606,6 +613,7 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs,
606613
}
607614

608615
if (!cnt) {
616+
pr_err("no fragments or symbols in overlay\n");
609617
ret = -EINVAL;
610618
goto err_free_fragments;
611619
}
@@ -642,11 +650,24 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
642650
}
643651
kfree(ovcs->fragments);
644652

653+
/*
654+
* TODO
655+
*
656+
* would like to: kfree(ovcs->overlay_tree);
657+
* but can not since drivers may have pointers into this data
658+
*
659+
* would like to: kfree(ovcs->fdt);
660+
* but can not since drivers may have pointers into this data
661+
*/
662+
645663
kfree(ovcs);
646664
}
647665

648-
/**
666+
/*
667+
* internal documentation
668+
*
649669
* of_overlay_apply() - Create and apply an overlay changeset
670+
* @fdt: the FDT that was unflattened to create @tree
650671
* @tree: Expanded overlay device tree
651672
* @ovcs_id: Pointer to overlay changeset id
652673
*
@@ -685,21 +706,29 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
685706
* id is returned to *ovcs_id.
686707
*/
687708

688-
int of_overlay_apply(struct device_node *tree, int *ovcs_id)
709+
static int of_overlay_apply(const void *fdt, struct device_node *tree,
710+
int *ovcs_id)
689711
{
690712
struct overlay_changeset *ovcs;
691713
int ret = 0, ret_revert, ret_tmp;
692714

693-
*ovcs_id = 0;
715+
/*
716+
* As of this point, fdt and tree belong to the overlay changeset.
717+
* overlay changeset code is responsible for freeing them.
718+
*/
694719

695720
if (devicetree_corrupt()) {
696721
pr_err("devicetree state suspect, refuse to apply overlay\n");
722+
kfree(fdt);
723+
kfree(tree);
697724
ret = -EBUSY;
698725
goto out;
699726
}
700727

701728
ovcs = kzalloc(sizeof(*ovcs), GFP_KERNEL);
702729
if (!ovcs) {
730+
kfree(fdt);
731+
kfree(tree);
703732
ret = -ENOMEM;
704733
goto out;
705734
}
@@ -709,12 +738,17 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id)
709738

710739
ret = of_resolve_phandles(tree);
711740
if (ret)
712-
goto err_free_overlay_changeset;
741+
goto err_free_tree;
713742

714-
ret = init_overlay_changeset(ovcs, tree);
743+
ret = init_overlay_changeset(ovcs, fdt, tree);
715744
if (ret)
716-
goto err_free_overlay_changeset;
745+
goto err_free_tree;
717746

747+
/*
748+
* after overlay_notify(), ovcs->overlay_tree related pointers may have
749+
* leaked to drivers, so can not kfree() tree, aka ovcs->overlay_tree;
750+
* and can not free fdt, aka ovcs->fdt
751+
*/
718752
ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY);
719753
if (ret) {
720754
pr_err("overlay changeset pre-apply notify error %d\n", ret);
@@ -754,6 +788,10 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id)
754788

755789
goto out_unlock;
756790

791+
err_free_tree:
792+
kfree(fdt);
793+
kfree(tree);
794+
757795
err_free_overlay_changeset:
758796
free_overlay_changeset(ovcs);
759797

@@ -766,7 +804,63 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id)
766804

767805
return ret;
768806
}
769-
EXPORT_SYMBOL_GPL(of_overlay_apply);
807+
808+
int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
809+
int *ovcs_id)
810+
{
811+
const void *new_fdt;
812+
int ret;
813+
u32 size;
814+
struct device_node *overlay_root;
815+
816+
*ovcs_id = 0;
817+
ret = 0;
818+
819+
if (overlay_fdt_size < sizeof(struct fdt_header) ||
820+
fdt_check_header(overlay_fdt)) {
821+
pr_err("Invalid overlay_fdt header\n");
822+
return -EINVAL;
823+
}
824+
825+
size = fdt_totalsize(overlay_fdt);
826+
if (overlay_fdt_size < size)
827+
return -EINVAL;
828+
829+
/*
830+
* Must create permanent copy of FDT because of_fdt_unflatten_tree()
831+
* will create pointers to the passed in FDT in the unflattened tree.
832+
*/
833+
new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL);
834+
if (!new_fdt)
835+
return -ENOMEM;
836+
837+
of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root);
838+
if (!overlay_root) {
839+
pr_err("unable to unflatten overlay_fdt\n");
840+
ret = -EINVAL;
841+
goto out_free_new_fdt;
842+
}
843+
844+
ret = of_overlay_apply(new_fdt, overlay_root, ovcs_id);
845+
if (ret < 0) {
846+
/*
847+
* new_fdt and overlay_root now belong to the overlay
848+
* changeset.
849+
* overlay changeset code is responsible for freeing them.
850+
*/
851+
goto out;
852+
}
853+
854+
return 0;
855+
856+
857+
out_free_new_fdt:
858+
kfree(new_fdt);
859+
860+
out:
861+
return ret;
862+
}
863+
EXPORT_SYMBOL_GPL(of_overlay_fdt_apply);
770864

771865
/*
772866
* Find @np in @tree.

‎drivers/of/resolver.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -269,17 +269,11 @@ int of_resolve_phandles(struct device_node *overlay)
269269
goto out;
270270
}
271271

272-
#if 0
273-
Temporarily disable check so that old style overlay unittests
274-
do not fail when of_resolve_phandles() is moved into
275-
of_overlay_apply().
276-
277272
if (!of_node_check_flag(overlay, OF_DETACHED)) {
278273
pr_err("overlay not detached\n");
279274
err = -EINVAL;
280275
goto out;
281276
}
282-
#endif
283277

284278
phandle_delta = live_tree_max_phandle() + 1;
285279
adjust_overlay_phandles(overlay, phandle_delta);

‎drivers/of/unittest-data/Makefile

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,37 @@
11
# SPDX-License-Identifier: GPL-2.0
2-
DTC_FLAGS_testcases := -Wno-interrupts_property
32
obj-y += testcases.dtb.o
43

54
obj-$(CONFIG_OF_OVERLAY) += overlay.dtb.o \
5+
overlay_0.dtb.o \
6+
overlay_1.dtb.o \
7+
overlay_2.dtb.o \
8+
overlay_3.dtb.o \
9+
overlay_4.dtb.o \
10+
overlay_5.dtb.o \
11+
overlay_6.dtb.o \
12+
overlay_7.dtb.o \
13+
overlay_8.dtb.o \
14+
overlay_9.dtb.o \
15+
overlay_10.dtb.o \
16+
overlay_11.dtb.o \
17+
overlay_12.dtb.o \
18+
overlay_13.dtb.o \
19+
overlay_15.dtb.o \
620
overlay_bad_phandle.dtb.o \
721
overlay_bad_symbol.dtb.o \
822
overlay_base.dtb.o
923

1024
targets += $(foreach suffix, dtb dtb.S, $(patsubst %.dtb.o,%.$(suffix),$(obj-y)))
1125

1226
# enable creation of __symbols__ node
13-
DTC_FLAGS_overlay := -@
14-
DTC_FLAGS_overlay_bad_phandle := -@
15-
DTC_FLAGS_overlay_bad_symbol := -@
16-
DTC_FLAGS_overlay_base := -@
27+
DTC_FLAGS_overlay += -@
28+
DTC_FLAGS_overlay_bad_phandle += -@
29+
DTC_FLAGS_overlay_bad_symbol += -@
30+
DTC_FLAGS_overlay_base += -@
31+
DTC_FLAGS_testcases += -@
32+
33+
# suppress warnings about intentional errors
34+
DTC_FLAGS_testcases += -Wno-interrupts_property
1735

1836
.PRECIOUS: \
1937
$(obj)/%.dtb.S \

‎drivers/of/unittest-data/overlay_0.dts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
/dts-v1/;
3+
/plugin/;
4+
5+
/ {
6+
/* overlay_0 - enable using absolute target path */
7+
8+
fragment@0 {
9+
target-path = "/testcase-data/overlay-node/test-bus/test-unittest0";
10+
__overlay__ {
11+
status = "okay";
12+
};
13+
};
14+
};

‎drivers/of/unittest-data/overlay_1.dts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
/dts-v1/;
3+
/plugin/;
4+
5+
/ {
6+
/* overlay_1 - disable using absolute target path */
7+
8+
fragment@0 {
9+
target-path = "/testcase-data/overlay-node/test-bus/test-unittest1";
10+
__overlay__ {
11+
status = "disabled";
12+
};
13+
};
14+
};

‎drivers/of/unittest-data/overlay_10.dts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
/dts-v1/;
3+
/plugin/;
4+
5+
/ {
6+
/* overlay_10 */
7+
/* overlays 8, 9, 10, 11 application and removal in bad sequence */
8+
9+
fragment@0 {
10+
target-path = "/testcase-data/overlay-node/test-bus";
11+
__overlay__ {
12+
13+
/* suppress DTC warning */
14+
#address-cells = <1>;
15+
#size-cells = <0>;
16+
17+
test-unittest10 {
18+
compatible = "unittest";
19+
status = "okay";
20+
reg = <10>;
21+
22+
#address-cells = <1>;
23+
#size-cells = <0>;
24+
25+
test-unittest101 {
26+
compatible = "unittest";
27+
status = "okay";
28+
reg = <1>;
29+
};
30+
31+
};
32+
};
33+
};
34+
};

‎drivers/of/unittest-data/overlay_11.dts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
/dts-v1/;
3+
/plugin/;
4+
5+
/ {
6+
/* overlay_11 */
7+
/* overlays 8, 9, 10, 11 application and removal in bad sequence */
8+
9+
fragment@0 {
10+
target-path = "/testcase-data/overlay-node/test-bus";
11+
__overlay__ {
12+
13+
/* suppress DTC warning */
14+
#address-cells = <1>;
15+
#size-cells = <0>;
16+
17+
test-unittest11 {
18+
compatible = "unittest";
19+
status = "okay";
20+
reg = <11>;
21+
22+
#address-cells = <1>;
23+
#size-cells = <0>;
24+
25+
test-unittest111 {
26+
compatible = "unittest";
27+
status = "okay";
28+
reg = <1>;
29+
};
30+
31+
};
32+
};
33+
};
34+
};

0 commit comments

Comments
 (0)