lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240920-scm-pdev-v1-1-b76d90e06af7@quicinc.com>
Date: Fri, 20 Sep 2024 11:01:40 -0700
From: Elliot Berman <quic_eberman@...cinc.com>
To: Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio
	<konradybcio@...nel.org>,
        Bartosz Golaszewski
	<bartosz.golaszewski@...aro.org>,
        Andrew Halaney <ahalaney@...hat.com>,
        Rudraksha Gupta <guptarud@...il.com>,
        "Linux regression tracking (Thorsten
 Leemhuis)" <regressions@...mhuis.info>,
        Dmitry Baryshkov
	<dmitry.baryshkov@...aro.org>
CC: <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        "Bartosz
 Golaszewski" <brgl@...ev.pl>,
        Elliot Berman <quic_eberman@...cinc.com>
Subject: [PATCH] firmware: qcom: scm: Allow devicetree-less probe

Some devicetrees representing Qualcomm Technologies, Inc. SoCs are
missing the SCM node. Users of the SCM device assume the device is
present and the driver also assumes it has probed. This can lead to
unanticipated crashes when there isn't an SCM device. All Qualcomm
Technologies, Inc. SoCs use SCM to communicate with firmware, so create
the platform device if it's not present in the devicetree.

Tested that SCM node still probes on:
 - sm8650-qrd with the SCM DT node still present
 - sm845-mtp with the SCM DT node still present
 - sm845-mtp with the node removed

Fixes: 449d0d84bcd8 ("firmware: qcom: scm: smc: switch to using the SCM allocator")
Reported-by: Rudraksha Gupta <guptarud@...il.com>
Closes: https://lore.kernel.org/lkml/692cfe9a-8c05-4ce4-813e-82b3f310019a@gmail.com/
Link: https://lore.kernel.org/all/CAA8EJpqSKbKJ=y0LAigGdj7_uk+5mezDgnzV5XEzwbxRJgpN1w@mail.gmail.com/
Suggested-by: Bartosz Golaszewski <brgl@...ev.pl>
Signed-off-by: Elliot Berman <quic_eberman@...cinc.com>
---
 drivers/firmware/qcom/qcom_scm.c | 75 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 66 insertions(+), 9 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 10986cb11ec0..842ba490cd37 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1954,10 +1954,12 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	init_completion(&scm->waitq_comp);
 	mutex_init(&scm->scm_bw_lock);
 
-	scm->path = devm_of_icc_get(&pdev->dev, NULL);
-	if (IS_ERR(scm->path))
-		return dev_err_probe(&pdev->dev, PTR_ERR(scm->path),
-				     "failed to acquire interconnect path\n");
+	if (pdev->dev.of_node) {
+		scm->path = devm_of_icc_get(&pdev->dev, NULL);
+		if (IS_ERR(scm->path))
+			return dev_err_probe(&pdev->dev, PTR_ERR(scm->path),
+					"failed to acquire interconnect path\n");
+	}
 
 	scm->core_clk = devm_clk_get_optional(&pdev->dev, "core");
 	if (IS_ERR(scm->core_clk))
@@ -2012,10 +2014,12 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	if (of_property_read_bool(pdev->dev.of_node, "qcom,sdi-enabled") || !download_mode)
 		qcom_scm_disable_sdi();
 
-	ret = of_reserved_mem_device_init(__scm->dev);
-	if (ret && ret != -ENODEV)
-		return dev_err_probe(__scm->dev, ret,
-				     "Failed to setup the reserved memory region for TZ mem\n");
+	if (pdev->dev.of_node) {
+		ret = of_reserved_mem_device_init(__scm->dev);
+		if (ret && ret != -ENODEV)
+			return dev_err_probe(__scm->dev, ret,
+					"Failed to setup the reserved memory region for TZ mem\n");
+	}
 
 	ret = qcom_tzmem_enable(__scm->dev);
 	if (ret)
@@ -2068,6 +2072,11 @@ static const struct of_device_id qcom_scm_dt_match[] = {
 };
 MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);
 
+static const struct platform_device_id qcom_scm_id_table[] = {
+	{ .name = "qcom-scm" },
+	{}
+};
+
 static struct platform_driver qcom_scm_driver = {
 	.driver = {
 		.name	= "qcom_scm",
@@ -2076,11 +2085,59 @@ static struct platform_driver qcom_scm_driver = {
 	},
 	.probe = qcom_scm_probe,
 	.shutdown = qcom_scm_shutdown,
+	.id_table = qcom_scm_id_table,
 };
 
+static bool is_qcom_machine(void)
+{
+	struct device_node *np __free(device_node) = NULL;
+	struct property *prop;
+	const char *name;
+
+	np = of_find_node_by_path("/");
+	if (!np)
+		return false;
+
+	of_property_for_each_string(np, "compatible", prop, name)
+		if (!strncmp("qcom,", name, 5))
+			return true;
+
+	return false;
+}
+
 static int __init qcom_scm_init(void)
 {
-	return platform_driver_register(&qcom_scm_driver);
+	struct device_node *np __free(device_node) = NULL;
+	struct platform_device *pdev;
+	int ret;
+
+	ret = platform_driver_register(&qcom_scm_driver);
+	if (ret)
+		return ret;
+
+	/* Some devicetrees representing Qualcomm Technologies, Inc. SoCs are
+	 * missing the SCM node. Find out if we don't have a SCM node *and*
+	 * we are a Qualcomm-compatible SoC. If yes, then create a platform
+	 * device for the SCM driver. Assume scanning the root compatible for
+	 * "qcom," vendor prefix will be faster than searching for the
+	 * SCM DT node.
+	 */
+	if (!is_qcom_machine())
+		return 0;
+
+	np = of_find_matching_node_and_match(NULL, qcom_scm_dt_match, NULL);
+	if (np)
+		return 0;
+
+	pdev = platform_device_alloc(qcom_scm_id_table[0].name, PLATFORM_DEVID_NONE);
+	if (!pdev)
+		return -ENOMEM;
+
+	ret = platform_device_add(pdev);
+	if (ret)
+		platform_device_put(pdev);
+
+	return ret;
 }
 subsys_initcall(qcom_scm_init);
 

---
base-commit: 2adcf3941db724e1750da7094c34431d9b6b7fcb
change-id: 20240917-scm-pdev-bc8db85fad05

Best regards,
-- 
Elliot Berman <quic_eberman@...cinc.com>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ