drivers/input/keyboard/mtk-pmic-keys.c | 3 +++ 1 file changed, 3 insertions(+)
In mtk_pmic_keys_probe function, the of_match_device function is
called to retrieve the compatible platform device info but its return
data pointer is not checked. It can lead to a null pointer deference
later when accessing the data field, if of_match_device returned a null
pointer. So, add a pointer check after calling of_match_device function
and return an EINVAL error in null case.
Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
This patch fixes a NULL pointer dereference that occurs during the
mtk_pmic_keys driver probe and observed at least on Mediatek Genio
1200-EVK board with a kernel based on linux-next (tag: 20250630),
when it is configured to have mtk_pmic_keys driver as builtin
(CONFIG_KEYBOARD_MTK_PMIC=y):
```
Unable to handle kernel NULL pointer dereference at virtual address
00000000000000c0
Mem abort info:
ESR = 0x0000000096000004
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
CM = 0, WnR = 0, TnD = 0, TagAccess = 0
GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[00000000000000c0] user address but active_mm is swapper
Internal error: Oops: 0000000096000004 [#1] SMP
Modules linked in:
CPU: 4 UID: 0 PID: 1 Comm: swapper/0 Not tainted
6.16.0-rc4-next-20250630-00001-gea99c662a089 #145 PREEMPT
Hardware name: MediaTek Genio 1200 EVK-P1V2-EMMC (DT)
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : mtk_pmic_keys_probe+0x94/0x500
lr : mtk_pmic_keys_probe+0x78/0x500
sp : ffff80008275bb30
x29: ffff80008275bb70 x28: ffff80008202bbb0 x27: ffff800081df00b0
x26: ffff800081ef9060 x25: ffff0000c6fcf400 x24: 0000000000000000
x23: 0000000000000000 x22: ffff0000c6fcf410 x21: ffff0000c09f8480
x20: ffff0000c09f4b80 x19: 0000000000000000 x18: 00000000ffffffff
x17: ffff8000824cb228 x16: 00000000d7fcbc9e x15: ffff0000c0a2b274
x14: ffff80008275bad0 x13: ffff0000c0a2ba1c x12: 786d692d696d6373
x11: 0000000000000040 x10: 0000000000000001 x9 : 0000000000000000
x8 : ffff0000c09f8500 x7 : 0000000000000000 x6 : 000000000000003f
x5 : 0000000000000040 x4 : ffff0000c6fcf410 x3 : ffff0000c6fcf6c0
x2 : ffff0000c09f8400 x1 : ffff0000c36da000 x0 : ffff0000c6fcf410
Call trace:
mtk_pmic_keys_probe+0x94/0x500 (P)
platform_probe+0x68/0xdc
really_probe+0xbc/0x2c0
__driver_probe_device+0x78/0x120
driver_probe_device+0x3c/0x154
__driver_attach+0x90/0x1a0
bus_for_each_dev+0x7c/0xdc
driver_attach+0x24/0x30
bus_add_driver+0xe4/0x208
driver_register+0x68/0x130
__platform_driver_register+0x24/0x30
pmic_keys_pdrv_init+0x1c/0x28
do_one_initcall+0x60/0x1d4
kernel_init_freeable+0x24c/0x2b4
kernel_init+0x20/0x140
ret_from_fork+0x10/0x20
Code: aa1603e0 f90006b6 f9400681 f9000aa1 (f9406261)
---[ end trace 0000000000000000 ]---
```
---
drivers/input/keyboard/mtk-pmic-keys.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
index 061d48350df661dd26832b307e1460ee8b8fd535..42fb93086db308ad87a276be4b53e9725a3a701b 100644
--- a/drivers/input/keyboard/mtk-pmic-keys.c
+++ b/drivers/input/keyboard/mtk-pmic-keys.c
@@ -316,6 +316,9 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
const struct of_device_id *of_id =
of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev);
+ if (!of_id)
+ return -EINVAL;
+
keys = devm_kzalloc(&pdev->dev, sizeof(*keys), GFP_KERNEL);
if (!keys)
return -ENOMEM;
---
base-commit: c6a68d8f7b81a6ce8962885408cc2d0c1f8b9470
change-id: 20250630-mtk-pmic-keys-fix-crash-42b55af280ef
Best regards,
--
Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>On 6/30/2025 7:03 AM, Louis-Alexis Eyraud wrote: > In mtk_pmic_keys_probe function, the of_match_device function is > called to retrieve the compatible platform device info but its return > data pointer is not checked. It can lead to a null pointer deference > later when accessing the data field, if of_match_device returned a null > pointer. So, add a pointer check after calling of_match_device function > and return an EINVAL error in null case. > > Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com> > --- > This patch fixes a NULL pointer dereference that occurs during the > mtk_pmic_keys driver probe and observed at least on Mediatek Genio > 1200-EVK board with a kernel based on linux-next (tag: 20250630), > when it is configured to have mtk_pmic_keys driver as builtin > (CONFIG_KEYBOARD_MTK_PMIC=y): > ``` > Unable to handle kernel NULL pointer dereference at virtual address > 00000000000000c0 > Mem abort info: > ESR = 0x0000000096000004 > EC = 0x25: DABT (current EL), IL = 32 bits > SET = 0, FnV = 0 > EA = 0, S1PTW = 0 > FSC = 0x04: level 0 translation fault > Data abort info: > ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 > CM = 0, WnR = 0, TnD = 0, TagAccess = 0 > GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 > [00000000000000c0] user address but active_mm is swapper > Internal error: Oops: 0000000096000004 [#1] SMP > Modules linked in: > CPU: 4 UID: 0 PID: 1 Comm: swapper/0 Not tainted > 6.16.0-rc4-next-20250630-00001-gea99c662a089 #145 PREEMPT > Hardware name: MediaTek Genio 1200 EVK-P1V2-EMMC (DT) > pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : mtk_pmic_keys_probe+0x94/0x500 > lr : mtk_pmic_keys_probe+0x78/0x500 > sp : ffff80008275bb30 > x29: ffff80008275bb70 x28: ffff80008202bbb0 x27: ffff800081df00b0 > x26: ffff800081ef9060 x25: ffff0000c6fcf400 x24: 0000000000000000 > x23: 0000000000000000 x22: ffff0000c6fcf410 x21: ffff0000c09f8480 > x20: ffff0000c09f4b80 x19: 0000000000000000 x18: 00000000ffffffff > x17: ffff8000824cb228 x16: 00000000d7fcbc9e x15: ffff0000c0a2b274 > x14: ffff80008275bad0 x13: ffff0000c0a2ba1c x12: 786d692d696d6373 > x11: 0000000000000040 x10: 0000000000000001 x9 : 0000000000000000 > x8 : ffff0000c09f8500 x7 : 0000000000000000 x6 : 000000000000003f > x5 : 0000000000000040 x4 : ffff0000c6fcf410 x3 : ffff0000c6fcf6c0 > x2 : ffff0000c09f8400 x1 : ffff0000c36da000 x0 : ffff0000c6fcf410 > Call trace: > mtk_pmic_keys_probe+0x94/0x500 (P) > platform_probe+0x68/0xdc > really_probe+0xbc/0x2c0 > __driver_probe_device+0x78/0x120 > driver_probe_device+0x3c/0x154 > __driver_attach+0x90/0x1a0 > bus_for_each_dev+0x7c/0xdc > driver_attach+0x24/0x30 > bus_add_driver+0xe4/0x208 > driver_register+0x68/0x130 > __platform_driver_register+0x24/0x30 > pmic_keys_pdrv_init+0x1c/0x28 > do_one_initcall+0x60/0x1d4 > kernel_init_freeable+0x24c/0x2b4 > kernel_init+0x20/0x140 > ret_from_fork+0x10/0x20 > Code: aa1603e0 f90006b6 f9400681 f9000aa1 (f9406261) > ---[ end trace 0000000000000000 ]--- > ``` > --- > drivers/input/keyboard/mtk-pmic-keys.c | 3 +++ > 1 file changed, 3 insertions(+) It's preferred to have the stack trace in the commit message body rather than below the cut line to allow for searching for the oops message in git history. Also, it may make sense to CC: stable@vger.kernel.org for backports Thanks, Easwar (he/him)
On Mon, Jun 30, 2025 at 01:18:40PM -0700, Easwar Hariharan wrote: > > Also, it may make sense to CC: stable@vger.kernel.org for backports What for? Stable does not need a patch papering over an oops, it needs a patch making the keypad working on the affected device. Thanks. -- Dmitry
On 6/30/2025 2:46 PM, Dmitry Torokhov wrote: > On Mon, Jun 30, 2025 at 01:18:40PM -0700, Easwar Hariharan wrote: >> >> Also, it may make sense to CC: stable@vger.kernel.org for backports > > What for? Stable does not need a patch papering over an oops, it needs a > patch making the keypad working on the affected device. > > Thanks. > I don't have a stake either way, it was simply a suggestion, since it qualifies IMHO, per https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html, the patch "...fixes a problem like an oops..." The proper fix might well be, as Nicolas suggested, adding the required compatibles. - Easwar (he/him)
On Mon, Jun 30, 2025 at 03:23:36PM -0700, Easwar Hariharan wrote: > On 6/30/2025 2:46 PM, Dmitry Torokhov wrote: > > On Mon, Jun 30, 2025 at 01:18:40PM -0700, Easwar Hariharan wrote: > >> > >> Also, it may make sense to CC: stable@vger.kernel.org for backports > > > > What for? Stable does not need a patch papering over an oops, it needs a > > patch making the keypad working on the affected device. > > > > Thanks. > > > > I don't have a stake either way, it was simply a suggestion, since it qualifies > IMHO, per https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html, > the patch "...fixes a problem like an oops..." > > The proper fix might well be, as Nicolas suggested, adding the required compatibles. It looks like it is not only about adding compatibles but actually adding proper support for the missed variants... Thanks. -- Dmitry
Il 30/06/25 16:03, Louis-Alexis Eyraud ha scritto: > In mtk_pmic_keys_probe function, the of_match_device function is > called to retrieve the compatible platform device info but its return > data pointer is not checked. It can lead to a null pointer deference > later when accessing the data field, if of_match_device returned a null > pointer. So, add a pointer check after calling of_match_device function > and return an EINVAL error in null case. > > Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com> > --- > This patch fixes a NULL pointer dereference that occurs during the > mtk_pmic_keys driver probe and observed at least on Mediatek Genio > 1200-EVK board with a kernel based on linux-next (tag: 20250630), > when it is configured to have mtk_pmic_keys driver as builtin > (CONFIG_KEYBOARD_MTK_PMIC=y): > ``` > Unable to handle kernel NULL pointer dereference at virtual address > 00000000000000c0 > Mem abort info: > ESR = 0x0000000096000004 > EC = 0x25: DABT (current EL), IL = 32 bits > SET = 0, FnV = 0 > EA = 0, S1PTW = 0 > FSC = 0x04: level 0 translation fault > Data abort info: > ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 > CM = 0, WnR = 0, TnD = 0, TagAccess = 0 > GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 > [00000000000000c0] user address but active_mm is swapper > Internal error: Oops: 0000000096000004 [#1] SMP > Modules linked in: > CPU: 4 UID: 0 PID: 1 Comm: swapper/0 Not tainted > 6.16.0-rc4-next-20250630-00001-gea99c662a089 #145 PREEMPT > Hardware name: MediaTek Genio 1200 EVK-P1V2-EMMC (DT) > pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : mtk_pmic_keys_probe+0x94/0x500 > lr : mtk_pmic_keys_probe+0x78/0x500 > sp : ffff80008275bb30 > x29: ffff80008275bb70 x28: ffff80008202bbb0 x27: ffff800081df00b0 > x26: ffff800081ef9060 x25: ffff0000c6fcf400 x24: 0000000000000000 > x23: 0000000000000000 x22: ffff0000c6fcf410 x21: ffff0000c09f8480 > x20: ffff0000c09f4b80 x19: 0000000000000000 x18: 00000000ffffffff > x17: ffff8000824cb228 x16: 00000000d7fcbc9e x15: ffff0000c0a2b274 > x14: ffff80008275bad0 x13: ffff0000c0a2ba1c x12: 786d692d696d6373 > x11: 0000000000000040 x10: 0000000000000001 x9 : 0000000000000000 > x8 : ffff0000c09f8500 x7 : 0000000000000000 x6 : 000000000000003f > x5 : 0000000000000040 x4 : ffff0000c6fcf410 x3 : ffff0000c6fcf6c0 > x2 : ffff0000c09f8400 x1 : ffff0000c36da000 x0 : ffff0000c6fcf410 > Call trace: > mtk_pmic_keys_probe+0x94/0x500 (P) > platform_probe+0x68/0xdc > really_probe+0xbc/0x2c0 > __driver_probe_device+0x78/0x120 > driver_probe_device+0x3c/0x154 > __driver_attach+0x90/0x1a0 > bus_for_each_dev+0x7c/0xdc > driver_attach+0x24/0x30 > bus_add_driver+0xe4/0x208 > driver_register+0x68/0x130 > __platform_driver_register+0x24/0x30 > pmic_keys_pdrv_init+0x1c/0x28 > do_one_initcall+0x60/0x1d4 > kernel_init_freeable+0x24c/0x2b4 > kernel_init+0x20/0x140 > ret_from_fork+0x10/0x20 > Code: aa1603e0 f90006b6 f9400681 f9000aa1 (f9406261) > ---[ end trace 0000000000000000 ]--- > ``` > --- > drivers/input/keyboard/mtk-pmic-keys.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c > index 061d48350df661dd26832b307e1460ee8b8fd535..42fb93086db308ad87a276be4b53e9725a3a701b 100644 > --- a/drivers/input/keyboard/mtk-pmic-keys.c > +++ b/drivers/input/keyboard/mtk-pmic-keys.c > @@ -316,6 +316,9 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev) > const struct of_device_id *of_id = > of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev); > > + if (!of_id) > + return -EINVAL; Please, change this to `return -ENODEV;` after which: Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
On Mon, Jun 30, 2025 at 04:06:53PM +0200, AngeloGioacchino Del Regno wrote: > Il 30/06/25 16:03, Louis-Alexis Eyraud ha scritto: [... snip ...] > > @@ -316,6 +316,9 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev) > > const struct of_device_id *of_id = > > of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev); > > + if (!of_id) > > + return -EINVAL; > > Please, change this to `return -ENODEV;` No, this definitely should not be a "silent" error because it indicates there is something wrong with the kernel. Stepping back, why do we even enter mtk_pmic_keys_probe() if there is not a matching OF ID? Are there any other patches that are not upstream? Thanks. -- Dmitry
On Mon, 2025-06-30 at 08:25 -0700, Dmitry Torokhov wrote: > On Mon, Jun 30, 2025 at 04:06:53PM +0200, AngeloGioacchino Del Regno > wrote: > > Il 30/06/25 16:03, Louis-Alexis Eyraud ha scritto: > > [... snip ...] > > > > @@ -316,6 +316,9 @@ static int mtk_pmic_keys_probe(struct > > > platform_device *pdev) > > > const struct of_device_id *of_id = > > > of_match_device(of_mtk_pmic_keys_match_tbl, > > > &pdev->dev); > > > + if (!of_id) > > > + return -EINVAL; > > > > Please, change this to `return -ENODEV;` > > No, this definitely should not be a "silent" error because it > indicates > there is something wrong with the kernel. > > Stepping back, why do we even enter mtk_pmic_keys_probe() if there is > not a matching OF ID? Are there any other patches that are not > upstream? I'm guessing it's because the driver can be probed by a parent MFD driver, drivers/mfd/mt6397-core.c, and the compatibles defined in the MFD don't necessarily match the ones in the pmic-keys driver, for instance 'mediatek,mt6359-keys' is only listed in the MFD. Adding the missing compatibles to the pmic-keys driver should fix this. -- Thanks, Nícolas
On Mon, Jun 30, 2025 at 03:59:46PM -0400, Nícolas F. R. A. Prado wrote: > On Mon, 2025-06-30 at 08:25 -0700, Dmitry Torokhov wrote: > > On Mon, Jun 30, 2025 at 04:06:53PM +0200, AngeloGioacchino Del Regno > > wrote: > > > Il 30/06/25 16:03, Louis-Alexis Eyraud ha scritto: > > > > [... snip ...] > > > > > > @@ -316,6 +316,9 @@ static int mtk_pmic_keys_probe(struct > > > > platform_device *pdev) > > > > const struct of_device_id *of_id = > > > > of_match_device(of_mtk_pmic_keys_match_tbl, > > > > &pdev->dev); > > > > + if (!of_id) > > > > + return -EINVAL; > > > > > > Please, change this to `return -ENODEV;` > > > > No, this definitely should not be a "silent" error because it > > indicates > > there is something wrong with the kernel. > > > > Stepping back, why do we even enter mtk_pmic_keys_probe() if there is > > not a matching OF ID? Are there any other patches that are not > > upstream? > > I'm guessing it's because the driver can be probed by a parent MFD > driver, drivers/mfd/mt6397-core.c, and the compatibles defined in the > MFD don't necessarily match the ones in the pmic-keys driver, for > instance 'mediatek,mt6359-keys' is only listed in the MFD. Adding the > missing compatibles to the pmic-keys driver should fix this. I think if we stop using the generic "mtk-pmic-keys" in MFD core it should solve the issue. I just sent out a patch... Thanks. -- Dmitry
© 2016 - 2025 Red Hat, Inc.