Fix child-node lookup during probe, which ended up searching the whole device tree depth-first starting at the parent rather than just matching on its children.
Note that the original premature free of the parent node has already been fixed separately, but that fix was apparently never backported to stable.
Fixes: 47654a162081 ("usb: chipidea: msm: Restore wrapper settings after reset") Fixes: b74c43156c0c ("usb: chipidea: msm: ci_hdrc_msm_probe() missing of_node_get()") Cc: stable stable@vger.kernel.org # 4.10: b74c43156c0c Cc: Stephen Boyd stephen.boyd@linaro.org Cc: Frank Rowand frank.rowand@sony.com Signed-off-by: Johan Hovold johan@kernel.org --- drivers/usb/chipidea/ci_hdrc_msm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c index 3593ce0ec641..880009987460 100644 --- a/drivers/usb/chipidea/ci_hdrc_msm.c +++ b/drivers/usb/chipidea/ci_hdrc_msm.c @@ -247,7 +247,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev) if (ret) goto err_mux;
- ulpi_node = of_find_node_by_name(of_node_get(pdev->dev.of_node), "ulpi"); + ulpi_node = of_get_child_by_name(pdev->dev.of_node, "ulpi"); if (ulpi_node) { phy_node = of_get_next_available_child(ulpi_node, NULL); ci->hsic = of_device_is_compatible(phy_node, "qcom,usb-hsic-phy");
On Mon, Nov 13, 2017 at 11:12:58AM +0100, Johan Hovold wrote:
Fix child-node lookup during probe, which ended up searching the whole device tree depth-first starting at the parent rather than just matching on its children.
Note that the original premature free of the parent node has already been fixed separately, but that fix was apparently never backported to stable.
Fixes: 47654a162081 ("usb: chipidea: msm: Restore wrapper settings after reset") Fixes: b74c43156c0c ("usb: chipidea: msm: ci_hdrc_msm_probe() missing of_node_get()") Cc: stable stable@vger.kernel.org # 4.10: b74c43156c0c Cc: Stephen Boyd stephen.boyd@linaro.org Cc: Frank Rowand frank.rowand@sony.com Signed-off-by: Johan Hovold johan@kernel.org
drivers/usb/chipidea/ci_hdrc_msm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c index 3593ce0ec641..880009987460 100644 --- a/drivers/usb/chipidea/ci_hdrc_msm.c +++ b/drivers/usb/chipidea/ci_hdrc_msm.c @@ -247,7 +247,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev) if (ret) goto err_mux;
- ulpi_node = of_find_node_by_name(of_node_get(pdev->dev.of_node), "ulpi");
- ulpi_node = of_get_child_by_name(pdev->dev.of_node, "ulpi");
Stephen, would you comment on it? I am afraid I can't find the benefit for this change.
On Tue, Dec 12, 2017 at 11:08:17AM +0800, Peter Chen wrote:
On Mon, Nov 13, 2017 at 11:12:58AM +0100, Johan Hovold wrote:
Fix child-node lookup during probe, which ended up searching the whole device tree depth-first starting at the parent rather than just matching on its children.
Note that the original premature free of the parent node has already been fixed separately, but that fix was apparently never backported to stable.
Fixes: 47654a162081 ("usb: chipidea: msm: Restore wrapper settings after reset") Fixes: b74c43156c0c ("usb: chipidea: msm: ci_hdrc_msm_probe() missing of_node_get()") Cc: stable stable@vger.kernel.org # 4.10: b74c43156c0c Cc: Stephen Boyd stephen.boyd@linaro.org Cc: Frank Rowand frank.rowand@sony.com Signed-off-by: Johan Hovold johan@kernel.org
drivers/usb/chipidea/ci_hdrc_msm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c index 3593ce0ec641..880009987460 100644 --- a/drivers/usb/chipidea/ci_hdrc_msm.c +++ b/drivers/usb/chipidea/ci_hdrc_msm.c @@ -247,7 +247,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev) if (ret) goto err_mux;
- ulpi_node = of_find_node_by_name(of_node_get(pdev->dev.of_node), "ulpi");
- ulpi_node = of_get_child_by_name(pdev->dev.of_node, "ulpi");
Stephen, would you comment on it? I am afraid I can't find the benefit for this change.
The general problem is that several drivers were using the wrong OF-helper when looking up child nodes. This meant that instead of just matching on child nodes, a tree-wide, depth-first search was done, something which could end up matching an unrelated node elsewhere in the device tree.
To make things worse, most erroneous users of of_find_node_by_name(), also failed to notice that that function drops a reference to its first argument, something which can lead to use-after-free and crashes, for example, after probe deferrals.
In this case, it looks like the child node is looked-up to determine whether to enable a hardware quirk. The potential use-after-free has already been fixed up (by adding the missing of_node_get()), but that fix was never backported to stable.
This patch addresses both issues (tree-wide search + unbalanced put in stable), while removing buggy code which could otherwise end up being reproduced in yet another driver.
Johan
On Tue, Dec 12, 2017 at 01:59:29PM +0100, Johan Hovold wrote:
On Tue, Dec 12, 2017 at 11:08:17AM +0800, Peter Chen wrote:
On Mon, Nov 13, 2017 at 11:12:58AM +0100, Johan Hovold wrote:
Fix child-node lookup during probe, which ended up searching the whole device tree depth-first starting at the parent rather than just matching on its children.
Note that the original premature free of the parent node has already been fixed separately, but that fix was apparently never backported to stable.
Fixes: 47654a162081 ("usb: chipidea: msm: Restore wrapper settings after reset") Fixes: b74c43156c0c ("usb: chipidea: msm: ci_hdrc_msm_probe() missing of_node_get()") Cc: stable stable@vger.kernel.org # 4.10: b74c43156c0c Cc: Stephen Boyd stephen.boyd@linaro.org Cc: Frank Rowand frank.rowand@sony.com Signed-off-by: Johan Hovold johan@kernel.org
drivers/usb/chipidea/ci_hdrc_msm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c index 3593ce0ec641..880009987460 100644 --- a/drivers/usb/chipidea/ci_hdrc_msm.c +++ b/drivers/usb/chipidea/ci_hdrc_msm.c @@ -247,7 +247,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev) if (ret) goto err_mux;
- ulpi_node = of_find_node_by_name(of_node_get(pdev->dev.of_node), "ulpi");
- ulpi_node = of_get_child_by_name(pdev->dev.of_node, "ulpi");
Stephen, would you comment on it? I am afraid I can't find the benefit for this change.
The general problem is that several drivers were using the wrong OF-helper when looking up child nodes. This meant that instead of just matching on child nodes, a tree-wide, depth-first search was done, something which could end up matching an unrelated node elsewhere in the device tree.
To make things worse, most erroneous users of of_find_node_by_name(), also failed to notice that that function drops a reference to its first argument, something which can lead to use-after-free and crashes, for example, after probe deferrals.
In this case, it looks like the child node is looked-up to determine whether to enable a hardware quirk. The potential use-after-free has already been fixed up (by adding the missing of_node_get()), but that fix was never backported to stable.
This patch addresses both issues (tree-wide search + unbalanced put in stable), while removing buggy code which could otherwise end up being reproduced in yet another driver.
Johan
Thanks, if Stephen does not have comments, I will send Greg it next Monday.
linux-stable-mirror@lists.linaro.org
-
Johan Hovold -
Peter Chen