* [PATCH] Input: mtk-pmic-keys - match loop with count
@ 2026-05-27 0:45 Rosen Penev
2026-05-27 1:15 ` sashiko-bot
2026-05-27 1:41 ` Dmitry Torokhov
0 siblings, 2 replies; 3+ messages in thread
From: Rosen Penev @ 2026-05-27 0:45 UTC (permalink / raw)
To: linux-input
Cc: Dmitry Torokhov, Matthias Brugger, AngeloGioacchino Del Regno,
open list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support
of_get_available_child_count is used along with
for_each_child_of_node_scoped, which can cause a mismatch when keys have
a disabled status.
If a disabled child node exists in the device tree alongside available ones,
the loop could execute more times than the initial validation accounted for.
This might increment the index variable past the allocated array bounds,
leading to out-of-bounds accesses on irqnames[] and keys->keys[].
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/input/keyboard/mtk-pmic-keys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
index c78d9f6d97c4..5d4ebbafd276 100644
--- a/drivers/input/keyboard/mtk-pmic-keys.c
+++ b/drivers/input/keyboard/mtk-pmic-keys.c
@@ -363,7 +363,7 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
return -EINVAL;
}
- for_each_child_of_node_scoped(node, child) {
+ for_each_available_child_of_node_scoped(node, child) {
keys->keys[index].regs = &mtk_pmic_regs->keys_regs[index];
keys->keys[index].irq =
--
2.54.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] Input: mtk-pmic-keys - match loop with count
2026-05-27 0:45 [PATCH] Input: mtk-pmic-keys - match loop with count Rosen Penev
@ 2026-05-27 1:15 ` sashiko-bot
2026-05-27 1:41 ` Dmitry Torokhov
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-05-27 1:15 UTC (permalink / raw)
To: Rosen Penev; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Switching to `for_each_available_child_of_node_scoped` shifts the hardware mapping index if a preceding node is disabled, leading to incorrect physical key configuration.
--
commit 999599e1545f7dcb987f1a3ede455f57f6a980e6
Author: Rosen Penev <rosenp@gmail.com>
Input: mtk-pmic-keys - match loop with count
This commit changes the child node iteration in mtk_pmic_keys_probe() to
only iterate over available child nodes, preventing an out-of-bounds array
access if a disabled node exists alongside available ones.
> diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
> index c78d9f6d97c4f..5d4ebbafd2765 100644
> --- a/drivers/input/keyboard/mtk-pmic-keys.c
> +++ b/drivers/input/keyboard/mtk-pmic-keys.c
> @@ -363,7 +363,7 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - for_each_child_of_node_scoped(node, child) {
> + for_each_available_child_of_node_scoped(node, child) {
[Severity: High]
Does this change in iteration cause a regression with the positional mapping
of keys to hardware?
The driver maps device tree child nodes to physical key hardware (IRQ and
registers) based strictly on the loop iteration index (index = 0 for
powerkey, index = 1 for homekey).
Before this patch, for_each_child_of_node_scoped() iterated over all defined
child nodes, ensuring that positional mapping was maintained even if the first
node (e.g., powerkey) had status = "disabled".
By changing the iterator to for_each_available_child_of_node_scoped(),
disabled nodes are now skipped. If the powerkey node is disabled in the
device tree, it is skipped, and the homekey node becomes the first available
child. It is then processed with index = 0.
Consequently, the driver would configure the powerkey hardware registers
and request the powerkey IRQ, but bind them to the properties
(like linux,keycodes) of the homekey node.
Could this result in the physical powerkey incorrectly emitting the homekey's
code, while the physical homekey remains uninitialized?
> keys->keys[index].regs = &mtk_pmic_regs->keys_regs[index];
>
> keys->keys[index].irq =
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260527004543.22875-1-rosenp@gmail.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Input: mtk-pmic-keys - match loop with count
2026-05-27 0:45 [PATCH] Input: mtk-pmic-keys - match loop with count Rosen Penev
2026-05-27 1:15 ` sashiko-bot
@ 2026-05-27 1:41 ` Dmitry Torokhov
1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Torokhov @ 2026-05-27 1:41 UTC (permalink / raw)
To: Rosen Penev
Cc: linux-input, Matthias Brugger, AngeloGioacchino Del Regno,
open list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support
Hi Rosen,
On Tue, May 26, 2026 at 05:45:43PM -0700, Rosen Penev wrote:
> of_get_available_child_count is used along with
> for_each_child_of_node_scoped, which can cause a mismatch when keys have
> a disabled status.
>
> If a disabled child node exists in the device tree alongside available ones,
> the loop could execute more times than the initial validation accounted for.
> This might increment the index variable past the allocated array bounds,
> leading to out-of-bounds accesses on irqnames[] and keys->keys[].
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> drivers/input/keyboard/mtk-pmic-keys.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
> index c78d9f6d97c4..5d4ebbafd276 100644
> --- a/drivers/input/keyboard/mtk-pmic-keys.c
> +++ b/drivers/input/keyboard/mtk-pmic-keys.c
> @@ -363,7 +363,7 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - for_each_child_of_node_scoped(node, child) {
> + for_each_available_child_of_node_scoped(node, child) {
> keys->keys[index].regs = &mtk_pmic_regs->keys_regs[index];
>
> keys->keys[index].irq =
I think Sashiko correctly points out that this may result in incorrect
register data and interrupts being mapped to the keys (potentially
shifting them).
Maybe we should stop counting nodes separately, iterate over all of them
here and bail out with an error if we encounter more than 2 (does not
matter if they are marked available or not), and then skip not available
nodes? WDYT?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-27 1:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-27 0:45 [PATCH] Input: mtk-pmic-keys - match loop with count Rosen Penev
2026-05-27 1:15 ` sashiko-bot
2026-05-27 1:41 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox