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

Krzysztof Kozlowski posted 1 patch 4 years, 4 months ago
There is a newer version of this series
drivers/clocksource/exynos_mct.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
[PATCH] clocksource/drivers/exynos_mct: Handle DTS with higher number of interrupts
Posted by Krzysztof Kozlowski 4 years, 4 months ago
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>
---
 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

RE: [PATCH] clocksource/drivers/exynos_mct: Handle DTS with higher number of interrupts
Posted by Alim Akhtar 4 years, 4 months ago

>-----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


[tip: timers/core] clocksource/drivers/exynos_mct: Handle DTS with higher number of interrupts
Posted by tip-bot2 for Krzysztof Kozlowski 4 years, 3 months ago
The following commit has been merged into the timers/core branch of tip:

Commit-ID:     ab8da93dc06d82f464c47ab30e6c75190702f369
Gitweb:        https://git.kernel.org/tip/ab8da93dc06d82f464c47ab30e6c75190702f369
Author:        Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
AuthorDate:    Sun, 20 Feb 2022 11:38:15 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Mon, 07 Mar 2022 18:27:22 +01:00

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>
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
Link: https://lore.kernel.org/r/20220220103815.135380-1-krzysztof.kozlowski@canonical.com
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 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 6db3d55..03782b3 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,