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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incorrect reset mask applied for MT6357 home key due to typo (`MTK_PMIC_HOMEKEY_INDEX` instead of `MTK_PMIC_HOMEKEY_RST`).
- [High] Device Tree child nodes are mapped purely by order, ignoring their identity.
- [High] Array bounds overflow due to mismatched DT child node counting and iteration.
- [High] Unchecked return value of `of_match_device()` leads to NULL dereference.
--

commit 576d0be432dffd670f7bb30dc9498d55a10d5703
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..8b4a89fce4fb1 100644
> --- a/drivers/input/keyboard/mtk-pmic-keys.c
> +++ b/drivers/input/keyboard/mtk-pmic-keys.c
[ ... ]
> @@ -301,6 +315,9 @@ static const struct of_device_id of_mtk_pmic_keys_match_tbl[] = {
>  	{
>  		.compatible = "mediatek,mt6397-keys",
>  		.data = &mt6397_regs,
> +	}, {
> +		.compatible = "mediatek,mt6392-keys",
> +		.data = &mt6392_regs,
>  	}, {
>  		.compatible = "mediatek,mt6323-keys",
>  		.data = &mt6323_regs,

[Severity: High]
This isn't a bug introduced by this patch, but the mt6357_regs array
erroneously uses the index constant MTK_PMIC_HOMEKEY_INDEX instead of
the correct bitmask MTK_PMIC_HOMEKEY_RST for the reset enable mask:

drivers/input/keyboard/mtk-pmic-keys.c:
	.keys_regs[MTK_PMIC_HOMEKEY_INDEX] =
		MTK_PMIC_KEYS_REGS(MT6357_TOPSTATUS,
				   0x8, MT6357_PSC_TOP_INT_CON0, 0xa,
				   MTK_PMIC_HOMEKEY_INDEX),

Could this cause incorrect bits to be modified in the MT6357_TOP_RST_MISC
register when configuring long-press reset behaviors, potentially corrupting
other hardware state?

[Severity: High]
This is a pre-existing issue, but Device Tree child nodes are mapped
purely by order in mtk_pmic_keys_probe():

drivers/input/keyboard/mtk-pmic-keys.c:mtk_pmic_keys_probe() {
	...
	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]);
	...
}

Since Device Tree does not guarantee child node ordering, if a device tree
lists the home key before the power key (or omits the power key entirely),
wouldn't the driver blindly map the first child node's properties to the
powerkey hardware resources?

[Severity: High]
This is a pre-existing issue, but is there an array bounds overflow due to
mismatched DT child node counting and iteration in mtk_pmic_keys_probe()?

drivers/input/keyboard/mtk-pmic-keys.c:mtk_pmic_keys_probe() {
	...
	keycount = of_get_available_child_count(node);
	if (keycount > MTK_PMIC_MAX_KEY_COUNT ||
	    keycount > ARRAY_SIZE(irqnames)) {
	...
	for_each_child_of_node_scoped(node, child) {
	...
}

The probe function uses of_get_available_child_count() for the bounds check
but iterates over all nodes including disabled ones. If a DT defines more
than two child nodes but leaves enough disabled, could the subsequent loop
increment the index beyond the array bounds?

[Severity: High]
This isn't a bug introduced by this patch, but the return value of
of_match_device() is unchecked in mtk_pmic_keys_probe():

drivers/input/keyboard/mtk-pmic-keys.c:mtk_pmic_keys_probe() {
	...
	const struct of_device_id *of_id =
		of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev);
	...
	mtk_pmic_regs = of_id->data;
	...
}

If the device is instantiated without a device tree node, won't
of_match_device() return NULL and result in a kernel panic on probe?

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