Re: [PATCH v10 6/9] input: keyboard: mtk-pmic-keys: Add MT6392 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Array out-of-bounds read and write due to mismatched DT child node counting and iteration.
- [High] Incorrect macro MTK_PMIC_HOMEKEY_INDEX used for homekey reset mask in mt6357_regs configuration.
--

commit 93a2f6ac06eb96960eb8f140af657b42448c3d24
Author: Val Packett <val@packett.cool>

input: keyboard: mtk-pmic-keys: Add MT6392 support

Add support for the MT6392 PMIC to the keys driver.

> diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
> index c78d9f6d97c4f..d758c7743f6a1 100644
> --- a/drivers/input/keyboard/mtk-pmic-keys.c
> +++ b/drivers/input/keyboard/mtk-pmic-keys.c

[ ... ]

While reviewing this patch, I noticed two pre-existing issues in the same
file.

In mt6357_regs:

> 		MTK_PMIC_KEYS_REGS(MT6357_TOPSTATUS,
> 				   0x8, MT6357_PSC_TOP_INT_CON0, 0xa,
> 				   MTK_PMIC_HOMEKEY_INDEX),

[Severity: High]
This is a pre-existing issue, but does this incorrectly use
MTK_PMIC_HOMEKEY_INDEX instead of the intended reset mask
MTK_PMIC_HOMEKEY_RST?

If MTK_PMIC_HOMEKEY_INDEX is used, it expands to 1 instead of the correct
bitmask. This causes mtk_pmic_keys_lp_reset_setup() to clear the wrong bit
in the MT6357_TOP_RST_MISC register and fail to configure the home key's
reset mask.


In mtk_pmic_keys_probe():

> 	keycount = of_get_available_child_count(node);
> 	if (keycount > MTK_PMIC_MAX_KEY_COUNT ||
> 	    keycount > ARRAY_SIZE(irqnames)) {
> 		dev_err(keys->dev, "too many keys defined (%d)\n", keycount);
> 		return -EINVAL;
> 	}
> 
> 	for_each_child_of_node_scoped(node, child) {
> 		keys->keys[index].regs = &mtk_pmic_regs->keys_regs[index];
> 
> 		keys->keys[index].irq =
> 			platform_get_irq_byname(pdev, irqnames[index]);

[Severity: High]
This is a pre-existing issue, but can this lead to an out-of-bounds array
access if disabled nodes are present in the device tree?

The bounds check counts available children using
of_get_available_child_count(). However, the loop uses
for_each_child_of_node_scoped(), which iterates over all child nodes
regardless of their status.

If a device tree contains three or more child nodes, but some are disabled
such that the available count is 2 or fewer, the bounds check passes. The
loop then processes all nodes, causing the index variable to exceed
MTK_PMIC_MAX_KEY_COUNT and corrupt memory.

Could we change the loop to use for_each_available_child_of_node_scoped()
to match the bounds check?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902193351.185771-1-l.scorcia@gmail.com?part=6