On 01/02/2023 11:15, Johan Hovold wrote:
The current interconnect provider registration interface is inherently racy as nodes are not added until the after adding the provider. This can specifically cause racing DT lookups to trigger a NULL-pointer deference when either a NULL pointer or not fully initialised node is returned from exynos_generic_icc_xlate().
Switch to using the new API where the provider is not registered until after it has been fully initialised.
Fixes: 2f95b9d5cf0b ("interconnect: Add generic interconnect driver for Exynos SoCs") Cc: stable@vger.kernel.org # 5.11 Cc: Sylwester Nawrocki s.nawrocki@samsung.com Signed-off-by: Johan Hovold johan+linaro@kernel.org
drivers/interconnect/samsung/exynos.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/interconnect/samsung/exynos.c b/drivers/interconnect/samsung/exynos.c index e70665899482..72e42603823b 100644 --- a/drivers/interconnect/samsung/exynos.c +++ b/drivers/interconnect/samsung/exynos.c @@ -98,12 +98,13 @@ static int exynos_generic_icc_remove(struct platform_device *pdev) struct exynos_icc_priv *priv = platform_get_drvdata(pdev); struct icc_node *parent_node, *node = priv->node;
- icc_provider_deregister(&priv->provider);
- parent_node = exynos_icc_get_parent(priv->dev->parent->of_node); if (parent_node && !IS_ERR(parent_node)) icc_link_destroy(node, parent_node);
icc_nodes_remove(&priv->provider);
- icc_provider_del(&priv->provider);
return 0; } @@ -132,15 +133,11 @@ static int exynos_generic_icc_probe(struct platform_device *pdev) provider->inter_set = true; provider->data = priv;
- ret = icc_provider_add(provider);
- if (ret < 0)
return ret;
- icc_provider_init(provider);
icc_node = icc_node_create(pdev->id);
- if (IS_ERR(icc_node)) {
ret = PTR_ERR(icc_node);goto err_prov_del;- }
- if (IS_ERR(icc_node))
return PTR_ERR(icc_node);priv->node = icc_node; icc_node->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%pOFn", @@ -171,14 +168,17 @@ static int exynos_generic_icc_probe(struct platform_device *pdev) goto err_pmqos_del; }
- ret = icc_provider_register(provider);
- if (ret < 0)
goto err_pmqos_del;
If I understand correctly there is no need for icc_link_destroy() in error path here, right? Even in case of probe retry (defer or whatever reason) - the link will be removed with icc_nodes_remove()?
Best regards, Krzysztof