RE: [PATCH] clocksource/drivers/exynos_mct: Handle DTS with higher number of interrupts

From: "Alim Akhtar" <alim.akhtar@samsung.com>
To: "'Krzysztof Kozlowski'" <krzysztof.kozlowski@canonical.com>,
	"'Daniel Lezcano'" <daniel.lezcano@linaro.org>,
	"'Thomas Gleixner'" <tglx@linutronix.de>,
	"'Thomas Abraham'" <thomas.abraham@linaro.org>,
	"'Stephen Warren'" <swarren@nvidia.com>,
	"'Kukjin Kim'" <kgene.kim@samsung.com>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-samsung-soc@vger.kernel.org>
Subject: RE: [PATCH] clocksource/drivers/exynos_mct: Handle DTS with higher number of interrupts
Date: Sun, 20 Feb 2022 21:54:18 +0530	[thread overview]
Message-ID: <0a0701d82676$5100e840$f302b8c0$@samsung.com> (raw)
In-Reply-To: <20220220103815.135380-1-krzysztof.kozlowski@canonical.com>


>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>Sent: Sunday, February 20, 2022 4:08 PM
>To: Daniel Lezcano <daniel.lezcano@linaro.org>; Thomas Gleixner
><tglx@linutronix.de>; Krzysztof Kozlowski
><krzysztof.kozlowski@canonical.com>; Alim Akhtar
><alim.akhtar@samsung.com>; Thomas Abraham
><thomas.abraham@linaro.org>; Stephen Warren <swarren@nvidia.com>;
>Kukjin Kim <kgene.kim@samsung.com>; linux-kernel@vger.kernel.org; linux-
>arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org
>Subject: [PATCH] clocksource/drivers/exynos_mct: Handle DTS with higher
>number of interrupts
>
>The driver statically defines maximum number of interrupts it can handle,
>however it does not respect that limit when configuring them.
>When provided with a DTS with more interrupts than assumed, the driver will
>overwrite static array mct_irqs leading to silent memory corruption.
>
>Validate the interrupts coming from DTS to avoid this.  This does not
change
>the fact that such DTS might not boot at all, because it is simply
incompatible,
>however at least some warning will be printed.
>
>Fixes: 36ba5d527e95 ("ARM: EXYNOS: add device tree support for MCT
>controller driver")
>Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>---
Thanks, good to have this check.

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>


> drivers/clocksource/exynos_mct.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/clocksource/exynos_mct.c
>b/drivers/clocksource/exynos_mct.c
>index 6db3d5511b0f..03782b399ea1 100644
>--- a/drivers/clocksource/exynos_mct.c
>+++ b/drivers/clocksource/exynos_mct.c
>@@ -541,6 +541,11 @@ static int __init exynos4_timer_interrupts(struct
>device_node *np,
> 	 * irqs are specified.
> 	 */
> 	nr_irqs = of_irq_count(np);
>+	if (nr_irqs > ARRAY_SIZE(mct_irqs)) {
>+		pr_err("exynos-mct: too many (%d) interrupts configured in
>DT\n",
>+			nr_irqs);
>+		nr_irqs = ARRAY_SIZE(mct_irqs);
>+	}
> 	for (i = MCT_L0_IRQ; i < nr_irqs; i++)
> 		mct_irqs[i] = irq_of_parse_and_map(np, i);
>
>@@ -553,11 +558,14 @@ static int __init exynos4_timer_interrupts(struct
>device_node *np,
> 		     mct_irqs[MCT_L0_IRQ], err);
> 	} else {
> 		for_each_possible_cpu(cpu) {
>-			int mct_irq = mct_irqs[MCT_L0_IRQ + cpu];
>+			int mct_irq;
> 			struct mct_clock_event_device *pcpu_mevt =
> 				per_cpu_ptr(&percpu_mct_tick, cpu);
>
> 			pcpu_mevt->evt.irq = -1;
>+			if (MCT_L0_IRQ + cpu >= ARRAY_SIZE(mct_irqs))
>+				break;
>+			mct_irq = mct_irqs[MCT_L0_IRQ + cpu];
>
> 			irq_set_status_flags(mct_irq, IRQ_NOAUTOEN);
> 			if (request_irq(mct_irq,
>--
>2.32.0