Re: [PATCH RFC 2/4] soc: qcom: Add modemsmem for Google phones
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-06 00:49:55
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Probe fails unconditionally if the SMEM item already exists, breaking module reload and pre-allocated items. - [High] Newly allocated SMEM memory is not zeroed, exposing uninitialized data to the modem firmware. -- commit 7a67ae0180630b09d3437d4fa514d468a7075572 Author: Richard Acayan [off-list ref] soc: qcom: Add modemsmem for Google phones Modems on Google Pixel devices require extra information about the SoC to be passed through a vendor-controlled Shared Memory (SMEM) channel. This driver provides this information.
quoted hunk ↗ jump to hunk
diff --git a/drivers/soc/qcom/modemsmem.c b/drivers/soc/qcom/modemsmem.c index 0000000000000..7a733422ff7c1 --- /dev/null +++ b/drivers/soc/qcom/modemsmem.c
[ ... ]
quoted hunk ↗ jump to hunk
+static int write_socinfo(struct modem_smem_info *target) +{ + struct socinfo *socinfo; + u32 plat_ver; + + socinfo = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL); + if (IS_ERR(socinfo)) + return PTR_ERR(socinfo); + + /* hw_plat_subtype was added in socinfo format 0.6 */ + if (le32_to_cpu(socinfo->fmt) < SOCINFO_VERSION(0, 6)) + return -EOPNOTSUPP; + + plat_ver = le32_to_cpu(socinfo->plat_ver); + + target->version = cpu_to_le32(MODEM_SMEM_VERSION); + target->major_id = cpu_to_le32(PLAT_VER_TO_MAJOR_ID(plat_ver)); + target->minor_id = cpu_to_le32(PLAT_VER_TO_MINOR_ID(plat_ver)); + target->platform = socinfo->hw_plat; + target->subtype = socinfo->hw_plat_subtype;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [Severity: High] Does this code leave uninitialized fields in struct modem_smem_info exposed to the modem firmware? When the SMEM item is freshly allocated by qcom_smem_alloc(), the memory is not zeroed. This function assigns 5 of the 8 fields in struct modem_smem_info, leaving modem_flag, efs_magic, and ftm_magic uninitialized. Could this SRAM garbage cause undefined modem behavior or misconfiguration? Follow-up patches in the series do not fix this regression.
quoted hunk ↗ jump to hunk
+ + return 0; +} + +static int modemsmem_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct modem_smem_info *info; + u32 smem_id; + int ret; + + ret = of_property_read_u32(dev->of_node, "qcom,smem-id", &smem_id); + if (ret) + return dev_err_probe(dev, ret, "Could not read smem id\n"); + + ret = qcom_smem_alloc(QCOM_SMEM_HOST_ANY, smem_id, sizeof(*info)); + if (ret) + return dev_err_probe(dev, ret, "Could not allocate modem smem\n");
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [Severity: High] Will this logic fail the probe process if the SMEM item already exists? qcom_smem_alloc() returns -EEXIST when the item has already been allocated by an earlier boot stage or during a previous module load. Treating any non-zero return as a fatal error prevents the driver from updating a pre-allocated SMEM region. Follow-up patches in the series do not fix this regression.
quoted hunk ↗ jump to hunk
+ + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, smem_id, NULL); + if (IS_ERR(info)) + return dev_err_probe(dev, PTR_ERR(info), "Could not get modem smem\n");
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260906-modemsmem-v1-0-a3634b553841@ixit.cz?part=2