Re: [PATCH 3/3] arm64: dts: qcom: Add INT2_GDSC to MDSS on Kaanapali and Glymur

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch omits `sm8750.dtsi`, failing to add the `INT2_GDSC` power domain for SM8750 despite the commit message explicitly declaring that SM8750 requires it.
- [High] Missing cleanup of the IRQ domain and chained handler when power domain attachment fails, leading to a Use-After-Free.
--

--- Patch [3]: [PATCH 3/3] arm64: dts: qcom: Add INT2_GDSC to MDSS on Kaanapali and Glymur ---
commit a757d0e4c829d9c4eb6cdd0c681fb7e3459e226e
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>

arm64: dts: qcom: Add INT2_GDSC to MDSS on Kaanapali and Glymur

Kaanapali (SM8750) and Glymur use two display power domains.
CORE_GDSC powers the main display hardware while INT2_GDSC powers
a subset of SSPP blocks (VIG2/VIG3/DMA5/DMA6).

Add DISP_CC_MDSS_CORE_INT2_GDSC alongside the existing CORE_GDSC
reference in the MDSS node on both platforms and provide matching
power-domain-names entries for the display power domains.

> Kaanapali (SM8750) and Glymur use two display power domains.
> CORE_GDSC powers the main display hardware while INT2_GDSC powers
> a subset of SSPP blocks (VIG2/VIG3/DMA5/DMA6).

[Severity: High]
The commit message explicitly states that Kaanapali (SM8750) requires the
INT2_GDSC power domain. However, the patch omits updating sm8750.dtsi.

Could this omission cause SM8750 platforms using sm8750.dtsi to fall back
to single-domain mode and fail to attach the secondary power domain, leaving
a subset of SSPP blocks (VIG2/VIG3/DMA5/DMA6) unpowered and inaccessible?

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 9087c4b290dba..63ece55c4f2ec 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
[ ... ]
@@ -421,6 +451,27 @@ static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5
 	irq_set_chained_handler_and_data(irq, msm_mdss_irq,
 					 msm_mdss);
 
+	/*
+	 * When two power domains are listed in DT, the core framework does not
+	 * auto-attach either of them.  Attach and manage both explicitly so
+	 * that INT2_GDSC (needed for VIG2/VIG3) is enabled alongside the main
+	 * CORE_GDSC.  Single-domain platforms are unaffected.
+	 */
+	if (of_count_phandle_with_args(pdev->dev.of_node, "power-domains",
+				       "#power-domain-cells") > 1) {
+		static const char * const mdss_pd_names[] = { "core", "int2" };
+		static const struct dev_pm_domain_attach_data pd_data = {
+			.pd_names = mdss_pd_names,
+			.num_pd_names = ARRAY_SIZE(mdss_pd_names),
+			.pd_flags = PD_FLAG_NO_DEV_LINK,
+		};
+
+		ret = devm_pm_domain_attach_list(&pdev->dev, &pd_data,
+						 &msm_mdss->pd_list);
+		if (ret < 0)
+			return ERR_PTR(ret);

[Severity: High]
Is it possible this error path leaves a dangling pointer registered in the IRQ
subsystem?

In msm_mdss_init(), irq_set_chained_handler_and_data() is called earlier to
install the chained handler with msm_mdss as the data pointer, and the IRQ
domain is created via irq_domain_create_linear(). If
devm_pm_domain_attach_list() fails (e.g., due to probe deferral), this block
directly returns an error without calling irq_set_chained_handler_and_data()
with NULL to clean it up, or calling irq_domain_remove().

Because msm_mdss is devres-allocated, it will be freed upon probe failure.
If a spurious or shared interrupt fires on that IRQ line, will the kernel
execute msm_mdss_irq() and dereference the freed msm_mdss pointer, causing a
use-after-free and a kernel panic?

+	}
+
 	pm_runtime_enable(&pdev->dev);
 
 	return msm_mdss;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-msm_gdsc2-v1-0-4687866d6cb0@oss.qualcomm.com?part=3