of: fix compatible-child-node lookups

Add of_get_compatible_child() helper that can be used to lookup compatible child nodes.

Several drivers currently use of_find_compatible_node() to lookup child nodes while failing to notice that the of_find_ functions search the entire tree depth-first (from a given start node) and therefore can match unrelated nodes. The fact that these functions also drop a reference to the node they start searching from (e.g. the parent node) is typically also overlooked, something which can lead to use-after-free bugs.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/of/base.c  | 25 +++++++++++++++++++++++++
 include/linux/of.h |  8 ++++++++
 2 files changed, 33 insertions(+)

Use the new of_get_compatible_child() helper to lookup the slot child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

While at it, also fix up the related slot-node reference leak.

Fixes ed80a13bb4c4 ("mmc: meson-mx-sdio: Add a driver for the Amlogic Meson8 and Meson8b SoCs")

Cc stable <[email protected]> # 4.15

Cc Carlo Caione <[email protected]>

Cc Martin Blumenstingl <[email protected]>

Cc Ulf Hansson <[email protected]>

Acked-by Martin Blumenstingl <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/mmc/host/meson-mx-sdio.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/mmc/host/meson-mx-sdio.c +6 -3

index 09cb89645d06..2cfec33178c1 100644
@@ -517,19 +517,23 @@ static struct mmc_host_ops meson_mx_mmc_ops = {
 static struct platform_device *meson_mx_mmc_slot_pdev(struct device *parent)
 {
        struct device_node *slot_node;
+       struct platform_device *pdev;
 
        /*
         * TODO: the MMC core framework currently does not support
         * controllers with multiple slots properly. So we only register
         * the first slot for now
         */
-       slot_node = of_find_compatible_node(parent->of_node, NULL, "mmc-slot");
+       slot_node = of_get_compatible_child(parent->of_node, "mmc-slot");
        if (!slot_node) {
                dev_warn(parent, "no 'mmc-slot' sub-node found\n");
                return ERR_PTR(-ENOENT);
        }
 
-       return of_platform_device_create(slot_node, NULL, parent);
+       pdev = of_platform_device_create(slot_node, NULL, parent);
+       of_node_put(slot_node);
+
+       return pdev;
 }
 
 static int meson_mx_mmc_add_host(struct meson_mx_mmc_host *host)
--

2.18.0

Quoted 11 lines · Johan Hovold

Use the new of_get_compatible_child() helper to lookup the slot child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

While at it, also fix up the related slot-node reference leak.

Quoted 7 lines

Fixes ed80a13bb4c4 ("mmc: meson-mx-sdio: Add a driver for the Amlogic Meson8 and Meson8b SoCs")

Cc stable <[email protected]> # 4.15

Cc Carlo Caione <[email protected]>

Cc Martin Blumenstingl <[email protected]>

Cc Ulf Hansson <[email protected]>

Acked-by Martin Blumenstingl <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Acked-by Ulf Hansson <[email protected]>

Quoted 20 lines · Ulf Hansson

Quoted 18 lines · Johan Hovold

Use the new of_get_compatible_child() helper to lookup the slot child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

While at it, also fix up the related slot-node reference leak.

Fixes ed80a13bb4c4 ("mmc: meson-mx-sdio: Add a driver for the Amlogic Meson8 and Meson8b SoCs")

Cc stable <[email protected]> # 4.15

Cc Carlo Caione <[email protected]>

Cc Martin Blumenstingl <[email protected]>

Cc Ulf Hansson <[email protected]>

Acked-by Martin Blumenstingl <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Quoted 1 line

Acked-by Ulf Hansson <[email protected]>

Thanks for the ack. Rob's gotten the helper into -rc2, so feel free to pick this one up directly to whichever mmc branch you prefer. I've been able to trigger crashes after probe deferrals due to the use-after-free, but this seems unlikely to be exploitable.

Use the new of_get_compatible_child() helper to lookup the nfc child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

While at it, also fix a related nfc-node reference leak.

Fixes f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")

Cc stable <[email protected]> # 4.11

Cc Nicolas Ferre <[email protected]>

Cc Josh Wu <[email protected]>

Cc Boris Brezillon <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/mtd/nand/raw/atmel/nand-controller.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

drivers/mtd/nand/raw/atmel/nand-controller.c +7 -4

index a068b214ebaa..d3dfe63956ac 100644
@@ -2061,8 +2061,7 @@ atmel_hsmc_nand_controller_legacy_init(struct atmel_hsmc_nand_controller *nc)
 	int ret;
 
 	nand_np = dev->of_node;
-	nfc_np = of_find_compatible_node(dev->of_node, NULL,
-					 "atmel,sama5d3-nfc");
+	nfc_np = of_get_compatible_child(dev->of_node, "atmel,sama5d3-nfc");
 
 	nc->clk = of_clk_get(nfc_np, 0);
 	if (IS_ERR(nc->clk)) {

@@ -2472,15 +2471,19 @@ static int atmel_nand_controller_probe(struct platform_device *pdev)
 	}
 
 	if (caps->legacy_of_bindings) {
+		struct device_node *nfc_node;
 		u32 ale_offs = 21;
 
 		/*
 		 * If we are parsing legacy DT props and the DT contains a
 		 * valid NFC node, forward the request to the sama5 logic.
 		 */
-		if (of_find_compatible_node(pdev->dev.of_node, NULL,
-					    "atmel,sama5d3-nfc"))
+		nfc_node = of_get_compatible_child(pdev->dev.of_node,
+						   "atmel,sama5d3-nfc");
+		if (nfc_node) {
 			caps = &atmel_sama5_nand_caps;
+			of_node_put(nfc_node);
+		}
 
 		/*
 		 * Even if the compatible says we are dealing with an

Hi Johan

Quoted 11 lines · On Mon, 27 Aug 2018 10:21:49 +0200 Johan Hovold <[email protected]> wrote:

Use the new of_get_compatible_child() helper to lookup the nfc child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

While at it, also fix a related nfc-node reference leak.

Quoted 6 lines

Fixes f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")

Cc stable <[email protected]> # 4.11

Cc Nicolas Ferre <[email protected]>

Cc Josh Wu <[email protected]>

Cc Boris Brezillon <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Acked-by Boris Brezillon <[email protected]>

I'll let Miquel queue this patch to the nand/next branch, unless you want it to be merged in 4.19, in which case I'll queue it to the mtd/fixes branch.

Quoted 16 lines · Boris Brezillon

Hi Johan

Quoted 11 lines · On Mon, 27 Aug 2018 10:21:49 +0200 Johan Hovold <[email protected]> wrote:

Use the new of_get_compatible_child() helper to lookup the nfc child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

While at it, also fix a related nfc-node reference leak.

Quoted 8 lines

Quoted 6 lines

Fixes f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")

Cc stable <[email protected]> # 4.11

Cc Nicolas Ferre <[email protected]>

Cc Josh Wu <[email protected]>

Cc Boris Brezillon <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Acked-by Boris Brezillon <[email protected]>

Thanks for the ack.

Quoted 3 lines

I'll let Miquel queue this patch to the nand/next branch, unless you want it to be merged in 4.19, in which case I'll queue it to the mtd/fixes branch.

Note that there's a dependency on the first patch of the series which adds the new helper.

Rob can pick up the entire series if the various maintainers agree, otherwise I'll try to get at the least the helper into -rc2.

Quoted 28 lines · On Mon, 27 Aug 2018 10:44:14 +0200 Johan Hovold <[email protected]> wrote:

Quoted 16 lines · Boris Brezillon

Hi Johan

Quoted 11 lines · On Mon, 27 Aug 2018 10:21:49 +0200 Johan Hovold <[email protected]> wrote:

Use the new of_get_compatible_child() helper to lookup the nfc child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

While at it, also fix a related nfc-node reference leak.

Quoted 8 lines

Quoted 6 lines

Fixes f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")

Cc stable <[email protected]> # 4.11

Cc Nicolas Ferre <[email protected]>

Cc Josh Wu <[email protected]>

Cc Boris Brezillon <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Acked-by Boris Brezillon <[email protected]>

Thanks for the ack.

Quoted 6 lines

Quoted 3 lines

I'll let Miquel queue this patch to the nand/next branch, unless you want it to be merged in 4.19, in which case I'll queue it to the mtd/fixes branch.

Note that there's a dependency on the first patch of the series which adds the new helper.

I was not Cc-ed on this patch :P.

Quoted 3 lines

Rob can pick up the entire series if the various maintainers agree, otherwise I'll try to get at the least the helper into -rc2.

If everything goes in 4.19-rc2 through Rob's tree that's fine, but if it's queued for 4.20 we might need an immutable tag just in case we queue conflicting changes to the NAND tree.

Quoted 31 lines · Boris Brezillon

Quoted 28 lines · On Mon, 27 Aug 2018 10:44:14 +0200 Johan Hovold <[email protected]> wrote:

Quoted 16 lines · Boris Brezillon

Hi Johan

Quoted 11 lines · On Mon, 27 Aug 2018 10:21:49 +0200 Johan Hovold <[email protected]> wrote:

Use the new of_get_compatible_child() helper to lookup the nfc child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

While at it, also fix a related nfc-node reference leak.

Quoted 8 lines

Quoted 6 lines

Fixes f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")

Cc stable <[email protected]> # 4.11

Cc Nicolas Ferre <[email protected]>

Cc Josh Wu <[email protected]>

Cc Boris Brezillon <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Acked-by Boris Brezillon <[email protected]>

Thanks for the ack.

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

I'll let Miquel queue this patch to the nand/next branch, unless you want it to be merged in 4.19, in which case I'll queue it to the mtd/fixes branch.

Note that there's a dependency on the first patch of the series which adds the new helper.

I was not Cc-ed on this patch :P.

Yeah, sorry about that. I made sure everyone was CCed on the cover letter, but guess I could have reused that list for the helper as well.

Quoted 4 lines

Quoted 3 lines

Rob can pick up the entire series if the various maintainers agree, otherwise I'll try to get at the least the helper into -rc2.

Quoted 3 lines

If everything goes in 4.19-rc2 through Rob's tree that's fine, but if it's queued for 4.20 we might need an immutable tag just in case we queue conflicting changes to the NAND tree.

Ok, thanks.

Quoted 50 lines · Johan Hovold

Quoted 31 lines · Boris Brezillon

Quoted 28 lines · On Mon, 27 Aug 2018 10:44:14 +0200 Johan Hovold <[email protected]> wrote:

Quoted 16 lines · Boris Brezillon

Hi Johan

Quoted 11 lines · On Mon, 27 Aug 2018 10:21:49 +0200 Johan Hovold <[email protected]> wrote:

Use the new of_get_compatible_child() helper to lookup the nfc child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

While at it, also fix a related nfc-node reference leak.

Quoted 8 lines

Quoted 6 lines

Fixes f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")

Cc stable <[email protected]> # 4.11

Cc Nicolas Ferre <[email protected]>

Cc Josh Wu <[email protected]>

Cc Boris Brezillon <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Acked-by Boris Brezillon <[email protected]>

Thanks for the ack.

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

I'll let Miquel queue this patch to the nand/next branch, unless you want it to be merged in 4.19, in which case I'll queue it to the mtd/fixes branch.

Note that there's a dependency on the first patch of the series which adds the new helper.

I was not Cc-ed on this patch :P.

Yeah, sorry about that. I made sure everyone was CCed on the cover letter, but guess I could have reused that list for the helper as well.

Quoted 4 lines

Quoted 3 lines

Rob can pick up the entire series if the various maintainers agree, otherwise I'll try to get at the least the helper into -rc2.

Quoted 5 lines

Quoted 3 lines

If everything goes in 4.19-rc2 through Rob's tree that's fine, but if it's queued for 4.20 we might need an immutable tag just in case we queue conflicting changes to the NAND tree.

Ok, thanks.

Hi Boris, can you pick this one up. It conflicts with "mtd: rawnand:
atmel: Fix potential NULL pointer dereference"

Quoted 55 lines · On Tue, 23 Oct 2018 13:28:09 -0500 Rob Herring <[email protected]> wrote:

Quoted 54 lines · Johan Hovold

Quoted 31 lines · Boris Brezillon

Quoted 28 lines · On Mon, 27 Aug 2018 10:44:14 +0200 Johan Hovold <[email protected]> wrote:

Quoted 16 lines · Boris Brezillon

Hi Johan

Quoted 11 lines · On Mon, 27 Aug 2018 10:21:49 +0200 Johan Hovold <[email protected]> wrote:

Use the new of_get_compatible_child() helper to lookup the nfc child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

While at it, also fix a related nfc-node reference leak.

Quoted 8 lines

Quoted 6 lines

Fixes f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")

Cc stable <[email protected]> # 4.11

Cc Nicolas Ferre <[email protected]>

Cc Josh Wu <[email protected]>

Cc Boris Brezillon <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Acked-by Boris Brezillon <[email protected]>

Thanks for the ack.

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

I'll let Miquel queue this patch to the nand/next branch, unless you want it to be merged in 4.19, in which case I'll queue it to the mtd/fixes branch.

Note that there's a dependency on the first patch of the series which adds the new helper.

I was not Cc-ed on this patch :P.

Yeah, sorry about that. I made sure everyone was CCed on the cover letter, but guess I could have reused that list for the helper as well.

Quoted 7 lines

Quoted 3 lines

Rob can pick up the entire series if the various maintainers agree, otherwise I'll try to get at the least the helper into -rc2.

If everything goes in 4.19-rc2 through Rob's tree that's fine, but if it's queued for 4.20 we might need an immutable tag just in case we queue conflicting changes to the NAND tree.

Quoted 4 lines

Quoted 1 line

Ok, thanks.

Hi Boris, can you pick this one up. It conflicts with "mtd: rawnand:
atmel: Fix potential NULL pointer dereference"

Sure, I'll queue it for -rc2.

Quoted 58 lines · Boris Brezillon

Quoted 55 lines · On Tue, 23 Oct 2018 13:28:09 -0500 Rob Herring <[email protected]> wrote:

Quoted 54 lines · Johan Hovold

Quoted 31 lines · Boris Brezillon

Quoted 28 lines · On Mon, 27 Aug 2018 10:44:14 +0200 Johan Hovold <[email protected]> wrote:

Quoted 16 lines · Boris Brezillon

Hi Johan

Quoted 11 lines · On Mon, 27 Aug 2018 10:21:49 +0200 Johan Hovold <[email protected]> wrote:

Use the new of_get_compatible_child() helper to lookup the nfc child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

While at it, also fix a related nfc-node reference leak.

Quoted 8 lines

Quoted 6 lines

Fixes f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")

Cc stable <[email protected]> # 4.11

Cc Nicolas Ferre <[email protected]>

Cc Josh Wu <[email protected]>

Cc Boris Brezillon <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Acked-by Boris Brezillon <[email protected]>

Thanks for the ack.

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

I'll let Miquel queue this patch to the nand/next branch, unless you want it to be merged in 4.19, in which case I'll queue it to the mtd/fixes branch.

Note that there's a dependency on the first patch of the series which adds the new helper.

I was not Cc-ed on this patch :P.

Yeah, sorry about that. I made sure everyone was CCed on the cover letter, but guess I could have reused that list for the helper as well.

Quoted 7 lines

Quoted 3 lines

Rob can pick up the entire series if the various maintainers agree, otherwise I'll try to get at the least the helper into -rc2.

If everything goes in 4.19-rc2 through Rob's tree that's fine, but if it's queued for 4.20 we might need an immutable tag just in case we queue conflicting changes to the NAND tree.

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

Ok, thanks.

Hi Boris, can you pick this one up. It conflicts with "mtd: rawnand:
atmel: Fix potential NULL pointer dereference"

Sure, I'll queue it for -rc2.

This one hasn't showed up in -next yet, so sending a reminder.

Use the new of_get_compatible_child() helper to lookup the mdio child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

Fixes aa09677cba42 ("net: bcmgenet: add MDIO routines")

Cc stable <[email protected]> # 3.15

Cc Florian Fainelli <[email protected]>

Cc David S. Miller <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Quoted 9 lines · Johan Hovold

Use the new of_get_compatible_child() helper to lookup the mdio child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the node of the device being probed).

Quoted 5 lines

Fixes aa09677cba42 ("net: bcmgenet: add MDIO routines")

Cc stable <[email protected]> # 3.15

Cc Florian Fainelli <[email protected]>

Cc David S. Miller <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Reviewed-by Florian Fainelli <[email protected]>

Use the new of_get_compatible_child() helper to lookup the mdio-internal child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the mdio-mux node). Fortunately, this was inadvertently balanced by a failure to drop the mdio-mux reference after lookup.

While at it, also fix the related mdio-internal- and phy-node reference leaks.

Fixes 634db83b8265 ("net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs")

Cc Corentin Labbe <[email protected]>

Cc Andrew Lunn <[email protected]>

Cc Giuseppe Cavallaro <[email protected]>

Cc Alexandre Torgue <[email protected]>

Cc Jose Abreu <[email protected]>

Cc David S. Miller <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Quoted 13 lines · Johan Hovold

Use the new of_get_compatible_child() helper to lookup the mdio-internal child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the mdio-mux node). Fortunately, this was inadvertently balanced by a failure to drop the mdio-mux reference after lookup.

While at it, also fix the related mdio-internal- and phy-node reference leaks.

Quoted 8 lines

Fixes 634db83b8265 ("net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs")

Cc Corentin Labbe <[email protected]>

Cc Andrew Lunn <[email protected]>

Cc Giuseppe Cavallaro <[email protected]>

Cc Alexandre Torgue <[email protected]>

Cc Jose Abreu <[email protected]>

Cc David S. Miller <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

You should have CCed sunxi maintainers
Maxime Ripard <[email protected]> (maintainer:ARM/Allwinner sunXi SoC support) Chen-Yu Tsai <[email protected]> (maintainer:ARM/Allwinner sunXi SoC support)

Since I am just back from holidays, I will test this patch this week.

Use the new of_get_compatible_child() helper to lookup the mdio-internal child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the mdio-mux node). Fortunately, this was inadvertently balanced by a failure to drop the mdio-mux reference after lookup.

While at it, also fix the related mdio-internal- and phy-node reference leaks.

Fixes 634db83b8265 ("net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs")

Cc Corentin Labbe <[email protected]>

Cc Andrew Lunn <[email protected]>

Cc Giuseppe Cavallaro <[email protected]>

Cc Alexandre Torgue <[email protected]>

Cc Jose Abreu <[email protected]>

Cc David S. Miller <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +10 -2

index f9a61f90cfbc..0f660af01a4b 100644
@@ -714,8 +714,9 @@ static int get_ephy_nodes(struct stmmac_priv *priv)
 		return -ENODEV;
 	}
 
-	mdio_internal = of_find_compatible_node(mdio_mux, NULL,
+	mdio_internal = of_get_compatible_child(mdio_mux,
 						"allwinner,sun8i-h3-mdio-internal");
+	of_node_put(mdio_mux);
 	if (!mdio_internal) {
 		dev_err(priv->device, "Cannot get internal_mdio node\n");
 		return -ENODEV;

@@ -729,13 +730,20 @@ static int get_ephy_nodes(struct stmmac_priv *priv)
 		gmac->rst_ephy = of_reset_control_get_exclusive(iphynode, NULL);
 		if (IS_ERR(gmac->rst_ephy)) {
 			ret = PTR_ERR(gmac->rst_ephy);
-			if (ret == -EPROBE_DEFER)
+			if (ret == -EPROBE_DEFER) {
+				of_node_put(iphynode);
+				of_node_put(mdio_internal);
 				return ret;
+			}
 			continue;
 		}
 		dev_info(priv->device, "Found internal PHY node\n");
+		of_node_put(iphynode);
+		of_node_put(mdio_internal);
 		return 0;
 	}
+
+	of_node_put(mdio_internal);
 	return -ENODEV;
 }
 

Signature · 1 line

2.18.0

Quoted 13 lines · Johan Hovold

Use the new of_get_compatible_child() helper to lookup the mdio-internal child node instead of using of_find_compatible_node(), which searches the entire tree from a given start node and thus can return an unrelated (i.e. non-child) node.

This also addresses a potential use-after-free (e.g. after probe deferral) as the tree-wide helper drops a reference to its first argument (i.e. the mdio-mux node). Fortunately, this was inadvertently balanced by a failure to drop the mdio-mux reference after lookup.

While at it, also fix the related mdio-internal- and phy-node reference leaks.

Quoted 8 lines

Fixes 634db83b8265 ("net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs")

Cc Corentin Labbe <[email protected]>

Cc Andrew Lunn <[email protected]>

Cc Giuseppe Cavallaro <[email protected]>

Cc Alexandre Torgue <[email protected]>

Cc Jose Abreu <[email protected]>

Cc David S. Miller <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

Tested-by Corentin Labbe <[email protected]>

Several drivers currently use of_find_compatible_node() to lookup child nodes while failing to notice that the of_find_ functions search the entire tree depth-first (from a given start node) and therefore can match unrelated nodes.

The fact that these functions also drop a reference to the node they start searching from (e.g. the parent node) is typically also overlooked, something which can lead to use-after-free bugs (e.g. after probe deferrals).

This series adds a new helper, similar to of_get_child_by_name(), that can be used to lookup compatible child nodes, and uses the new helper to fix child-node lookups throughout the tree.

This is related to the fixes I posted about a year ago, which addressed a similar anti-pattern when looking up child nodes by name. Since it took me more than a year to get all those fixes into Linus' tree (one fix is still pending), and as these fixes depend on the new helper, I'm suggesting that these all go in through Rob's or Greg's trees.

Alternatively, the helper could go into to -rc2, and I'll be pinging submaintainers for the coming year as well. ;)

Johan Hovold (9):
of: add helper to lookup compatible child node drm/mediatek: fix OF sibling-node lookup drm/msm: fix OF child-node lookup
mmc: meson-mx-sdio: fix OF child-node lookup
mtd: nand: atmel: fix OF child-node lookup
net: bcmgenet: fix OF child-node lookup
net: stmmac: dwmac-sun8i: fix OF child-node lookup
NFC: nfcmrvl_uart: fix OF child-node lookup
power: supply: twl4030-charger: fix OF sibling-node lookup

 drivers/gpu/drm/mediatek/mtk_hdmi.c           |  5 ++--
 drivers/gpu/drm/msm/adreno/adreno_gpu.c       |  5 ++--
 drivers/mmc/host/meson-mx-sdio.c              |  8 ++++--
 drivers/mtd/nand/raw/atmel/nand-controller.c  | 11 +++++---
 drivers/net/ethernet/broadcom/genet/bcmmii.c  |  2 +-
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 12 +++++++--
 drivers/nfc/nfcmrvl/uart.c                    |  5 ++--
 drivers/of/base.c                             | 25 +++++++++++++++++++
 drivers/power/supply/twl4030_charger.c        |  5 ++--
 include/linux/of.h                            |  8 ++++++
 10 files changed, 68 insertions(+), 18 deletions(-)

Hi all,

Quoted 14 lines · Johan Hovold

Several drivers currently use of_find_compatible_node() to lookup child nodes while failing to notice that the of_find_ functions search the entire tree depth-first (from a given start node) and therefore can match unrelated nodes.

The fact that these functions also drop a reference to the node they start searching from (e.g. the parent node) is typically also overlooked, something which can lead to use-after-free bugs (e.g. after probe deferrals).

This series adds a new helper, similar to of_get_child_by_name(), that can be used to lookup compatible child nodes, and uses the new helper to fix child-node lookups throughout the tree.

Quoted 8 lines

This is related to the fixes I posted about a year ago, which addressed a similar anti-pattern when looking up child nodes by name. Since it took me more than a year to get all those fixes into Linus' tree (one fix is still pending), and as these fixes depend on the new helper, I'm suggesting that these all go in through Rob's or Greg's trees.

Alternatively, the helper could go into to -rc2, and I'll be pinging submaintainers for the coming year as well. ;)

Rob has gotten the helper into -rc2 now:

36156f9241cb of: add helper to lookup compatible child node

so feel free to pick these fixes up directly for 4.19-rc or -next, whichever you prefer. I've been able to trigger crashes after probe deferrals due to the use-after-free, but this seems unlikely to be exploitable.

I think Rob will be picking up any patches that remain by the end of the release cycle for 4.20.

Quoted 22 lines

Johan Hovold (9):
of: add helper to lookup compatible child node drm/mediatek: fix OF sibling-node lookup drm/msm: fix OF child-node lookup
mmc: meson-mx-sdio: fix OF child-node lookup
mtd: nand: atmel: fix OF child-node lookup
net: bcmgenet: fix OF child-node lookup
net: stmmac: dwmac-sun8i: fix OF child-node lookup
NFC: nfcmrvl_uart: fix OF child-node lookup
power: supply: twl4030-charger: fix OF sibling-node lookup

 drivers/gpu/drm/mediatek/mtk_hdmi.c           |  5 ++--
 drivers/gpu/drm/msm/adreno/adreno_gpu.c       |  5 ++--
 drivers/mmc/host/meson-mx-sdio.c              |  8 ++++--
 drivers/mtd/nand/raw/atmel/nand-controller.c  | 11 +++++---
 drivers/net/ethernet/broadcom/genet/bcmmii.c  |  2 +-
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 12 +++++++--
 drivers/nfc/nfcmrvl/uart.c                    |  5 ++--
 drivers/of/base.c                             | 25 +++++++++++++++++++
 drivers/power/supply/twl4030_charger.c        |  5 ++--
 include/linux/of.h                            |  8 ++++++
 10 files changed, 68 insertions(+), 18 deletions(-)

Hi Rob,

Quoted 35 lines · Johan Hovold

Hi all,

Quoted 14 lines · Johan Hovold

Several drivers currently use of_find_compatible_node() to lookup child nodes while failing to notice that the of_find_ functions search the entire tree depth-first (from a given start node) and therefore can match unrelated nodes.

The fact that these functions also drop a reference to the node they start searching from (e.g. the parent node) is typically also overlooked, something which can lead to use-after-free bugs (e.g. after probe deferrals).

This series adds a new helper, similar to of_get_child_by_name(), that can be used to lookup compatible child nodes, and uses the new helper to fix child-node lookups throughout the tree.

Quoted 8 lines

This is related to the fixes I posted about a year ago, which addressed a similar anti-pattern when looking up child nodes by name. Since it took me more than a year to get all those fixes into Linus' tree (one fix is still pending), and as these fixes depend on the new helper, I'm suggesting that these all go in through Rob's or Greg's trees.

Alternatively, the helper could go into to -rc2, and I'll be pinging submaintainers for the coming year as well. ;)

Rob has gotten the helper into -rc2 now:

36156f9241cb of: add helper to lookup compatible child node

so feel free to pick these fixes up directly for 4.19-rc or -next, whichever you prefer. I've been able to trigger crashes after probe deferrals due to the use-after-free, but this seems unlikely to be exploitable.

Quoted 2 lines

I think Rob will be picking up any patches that remain by the end of the release cycle for 4.20.

So far only Ulf has picked up the mmc patch below directly, so if you could take the rest through your tree for -rc1 that would be great.

Quoted 4 lines · Johan Hovold

Hi Rob,

On Tue, Sep 04, 2018 at 03:05:57PM +0200, Johan Hovold wrote:

Quoted 5 lines

Quoted 2 lines

I think Rob will be picking up any patches that remain by the end of the release cycle for 4.20.

So far only Ulf has picked up the mmc patch below directly, so if you could take the rest through your tree for -rc1 that would be great.

Thanks for the reminder, though before the merge window opened would have been better. I've applied all but the mtd patch.