| Message ID | E1fGlYS-0001CN-Sd@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 5/10/2018 6:24 AM, Russell King wrote: > platform_domain_notifier contains a variable sized array, which the > pm_clk_notify() notifier treats as a NULL terminated array: > > for (con_id = clknb->con_ids; *con_id; con_id++) > pm_clk_add(dev, *con_id); > > Omitting the initialiser for con_ids means that the array is zero > sized, and there is no NULL terminator. This leads to pm_clk_notify() > overrunning into what ever structure follows, which may not be NULL. > This leads to an oops: > > Unable to handle kernel NULL pointer dereference at virtual address 0000008c > pgd = c0003000 > [0000008c] *pgd=80000800004003c, *pmd=00000000c > Internal error: Oops: 206 [#1] PREEMPT SMP ARM > Modules linked in:c > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.16.0+ #9 > Hardware name: Keystone > PC is at strlen+0x0/0x34 > LR is at kstrdup+0x18/0x54 > pc : [<c0623340>] lr : [<c0111d6c>] psr: 20000013 > sp : eec73dc0 ip : eed780c0 fp : 00000001 > r10: 00000000 r9 : 00000000 r8 : eed71e10 > r7 : 0000008c r6 : 0000008c r5 : 014000c0 r4 : c03a6ff4 > r3 : c09445d0 r2 : 00000000 r1 : 014000c0 r0 : 0000008c > Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user > Control: 30c5387d Table: 00003000 DAC: fffffffd > Process swapper/0 (pid: 1, stack limit = 0xeec72210) > Stack: (0xeec73dc0 to 0xeec74000) > ... > [<c0623340>] (strlen) from [<c0111d6c>] (kstrdup+0x18/0x54) > [<c0111d6c>] (kstrdup) from [<c03a6ff4>] (__pm_clk_add+0x58/0x120) > [<c03a6ff4>] (__pm_clk_add) from [<c03a731c>] (pm_clk_notify+0x64/0xa8) > [<c03a731c>] (pm_clk_notify) from [<c004614c>] (notifier_call_chain+0x44/0x84) > [<c004614c>] (notifier_call_chain) from [<c0046320>] (__blocking_notifier_call_chain+0x48/0x60) > [<c0046320>] (__blocking_notifier_call_chain) from [<c0046350>] (blocking_notifier_call_chain+0x18/0x20) > [<c0046350>] (blocking_notifier_call_chain) from [<c0390234>] (device_add+0x36c/0x534) > [<c0390234>] (device_add) from [<c047fc00>] (of_platform_device_create_pdata+0x70/0xa4) > [<c047fc00>] (of_platform_device_create_pdata) from [<c047fea0>] (of_platform_bus_create+0xf0/0x1ec) > [<c047fea0>] (of_platform_bus_create) from [<c047fff8>] (of_platform_populate+0x5c/0xac) > [<c047fff8>] (of_platform_populate) from [<c08b1f04>] (of_platform_default_populate_init+0x8c/0xa8) > [<c08b1f04>] (of_platform_default_populate_init) from [<c000a78c>] (do_one_initcall+0x3c/0x164) > [<c000a78c>] (do_one_initcall) from [<c087bd9c>] (kernel_init_freeable+0x10c/0x1d0) > [<c087bd9c>] (kernel_init_freeable) from [<c0628db0>] (kernel_init+0x8/0xf0) > [<c0628db0>] (kernel_init) from [<c00090d8>] (ret_from_fork+0x14/0x3c) > Exception stack(0xeec73fb0 to 0xeec73ff8) > 3fa0: 00000000 00000000 00000000 00000000 > 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 > Code: e3520000 1afffff7 e12fff1e c0801730 (e5d02000) > ---[ end trace cafa8f148e262e80 ]--- > > Fix this by adding the necessary initialiser. > > Fixes: fc20ffe1213b ("ARM: keystone: add PM domain support for clock management") > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > --- Looks good. Thanks Russell !! Acked-by: Santosh Shilimkar <ssantosh@kernel.org> Arnd, olof, Could you please pick this up for the fixes branch ?
On Thu, May 10, 2018 at 10:55:39AM -0700, Santosh Shilimkar wrote: > On 5/10/2018 6:24 AM, Russell King wrote: > > platform_domain_notifier contains a variable sized array, which the > > pm_clk_notify() notifier treats as a NULL terminated array: > > > > for (con_id = clknb->con_ids; *con_id; con_id++) > > pm_clk_add(dev, *con_id); > > > > Omitting the initialiser for con_ids means that the array is zero > > sized, and there is no NULL terminator. This leads to pm_clk_notify() > > overrunning into what ever structure follows, which may not be NULL. > > This leads to an oops: > > > > Unable to handle kernel NULL pointer dereference at virtual address 0000008c > > pgd = c0003000 > > [0000008c] *pgd=80000800004003c, *pmd=00000000c > > Internal error: Oops: 206 [#1] PREEMPT SMP ARM > > Modules linked in:c > > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.16.0+ #9 > > Hardware name: Keystone > > PC is at strlen+0x0/0x34 > > LR is at kstrdup+0x18/0x54 > > pc : [<c0623340>] lr : [<c0111d6c>] psr: 20000013 > > sp : eec73dc0 ip : eed780c0 fp : 00000001 > > r10: 00000000 r9 : 00000000 r8 : eed71e10 > > r7 : 0000008c r6 : 0000008c r5 : 014000c0 r4 : c03a6ff4 > > r3 : c09445d0 r2 : 00000000 r1 : 014000c0 r0 : 0000008c > > Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user > > Control: 30c5387d Table: 00003000 DAC: fffffffd > > Process swapper/0 (pid: 1, stack limit = 0xeec72210) > > Stack: (0xeec73dc0 to 0xeec74000) > > ... > > [<c0623340>] (strlen) from [<c0111d6c>] (kstrdup+0x18/0x54) > > [<c0111d6c>] (kstrdup) from [<c03a6ff4>] (__pm_clk_add+0x58/0x120) > > [<c03a6ff4>] (__pm_clk_add) from [<c03a731c>] (pm_clk_notify+0x64/0xa8) > > [<c03a731c>] (pm_clk_notify) from [<c004614c>] (notifier_call_chain+0x44/0x84) > > [<c004614c>] (notifier_call_chain) from [<c0046320>] (__blocking_notifier_call_chain+0x48/0x60) > > [<c0046320>] (__blocking_notifier_call_chain) from [<c0046350>] (blocking_notifier_call_chain+0x18/0x20) > > [<c0046350>] (blocking_notifier_call_chain) from [<c0390234>] (device_add+0x36c/0x534) > > [<c0390234>] (device_add) from [<c047fc00>] (of_platform_device_create_pdata+0x70/0xa4) > > [<c047fc00>] (of_platform_device_create_pdata) from [<c047fea0>] (of_platform_bus_create+0xf0/0x1ec) > > [<c047fea0>] (of_platform_bus_create) from [<c047fff8>] (of_platform_populate+0x5c/0xac) > > [<c047fff8>] (of_platform_populate) from [<c08b1f04>] (of_platform_default_populate_init+0x8c/0xa8) > > [<c08b1f04>] (of_platform_default_populate_init) from [<c000a78c>] (do_one_initcall+0x3c/0x164) > > [<c000a78c>] (do_one_initcall) from [<c087bd9c>] (kernel_init_freeable+0x10c/0x1d0) > > [<c087bd9c>] (kernel_init_freeable) from [<c0628db0>] (kernel_init+0x8/0xf0) > > [<c0628db0>] (kernel_init) from [<c00090d8>] (ret_from_fork+0x14/0x3c) > > Exception stack(0xeec73fb0 to 0xeec73ff8) > > 3fa0: 00000000 00000000 00000000 00000000 > > 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > > 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 > > Code: e3520000 1afffff7 e12fff1e c0801730 (e5d02000) > > ---[ end trace cafa8f148e262e80 ]--- > > > > Fix this by adding the necessary initialiser. > > > > Fixes: fc20ffe1213b ("ARM: keystone: add PM domain support for clock management") > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > > --- > Looks good. Thanks Russell !! > Acked-by: Santosh Shilimkar <ssantosh@kernel.org> > > Arnd, olof, > Could you please pick this up for the fixes branch ? Applied to fixes. Thanks! -Olof
diff --git a/arch/arm/mach-keystone/pm_domain.c b/arch/arm/mach-keystone/pm_domain.c index fe57e2692629..abca83d22ff3 100644 --- a/arch/arm/mach-keystone/pm_domain.c +++ b/arch/arm/mach-keystone/pm_domain.c @@ -29,6 +29,7 @@ static struct dev_pm_domain keystone_pm_domain = { static struct pm_clk_notifier_block platform_domain_notifier = { .pm_domain = &keystone_pm_domain, + .con_ids = { NULL }, }; static const struct of_device_id of_keystone_table[] = {
platform_domain_notifier contains a variable sized array, which the pm_clk_notify() notifier treats as a NULL terminated array: for (con_id = clknb->con_ids; *con_id; con_id++) pm_clk_add(dev, *con_id); Omitting the initialiser for con_ids means that the array is zero sized, and there is no NULL terminator. This leads to pm_clk_notify() overrunning into what ever structure follows, which may not be NULL. This leads to an oops: Unable to handle kernel NULL pointer dereference at virtual address 0000008c pgd = c0003000 [0000008c] *pgd=80000800004003c, *pmd=00000000c Internal error: Oops: 206 [#1] PREEMPT SMP ARM Modules linked in:c CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.16.0+ #9 Hardware name: Keystone PC is at strlen+0x0/0x34 LR is at kstrdup+0x18/0x54 pc : [<c0623340>] lr : [<c0111d6c>] psr: 20000013 sp : eec73dc0 ip : eed780c0 fp : 00000001 r10: 00000000 r9 : 00000000 r8 : eed71e10 r7 : 0000008c r6 : 0000008c r5 : 014000c0 r4 : c03a6ff4 r3 : c09445d0 r2 : 00000000 r1 : 014000c0 r0 : 0000008c Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user Control: 30c5387d Table: 00003000 DAC: fffffffd Process swapper/0 (pid: 1, stack limit = 0xeec72210) Stack: (0xeec73dc0 to 0xeec74000) ... [<c0623340>] (strlen) from [<c0111d6c>] (kstrdup+0x18/0x54) [<c0111d6c>] (kstrdup) from [<c03a6ff4>] (__pm_clk_add+0x58/0x120) [<c03a6ff4>] (__pm_clk_add) from [<c03a731c>] (pm_clk_notify+0x64/0xa8) [<c03a731c>] (pm_clk_notify) from [<c004614c>] (notifier_call_chain+0x44/0x84) [<c004614c>] (notifier_call_chain) from [<c0046320>] (__blocking_notifier_call_chain+0x48/0x60) [<c0046320>] (__blocking_notifier_call_chain) from [<c0046350>] (blocking_notifier_call_chain+0x18/0x20) [<c0046350>] (blocking_notifier_call_chain) from [<c0390234>] (device_add+0x36c/0x534) [<c0390234>] (device_add) from [<c047fc00>] (of_platform_device_create_pdata+0x70/0xa4) [<c047fc00>] (of_platform_device_create_pdata) from [<c047fea0>] (of_platform_bus_create+0xf0/0x1ec) [<c047fea0>] (of_platform_bus_create) from [<c047fff8>] (of_platform_populate+0x5c/0xac) [<c047fff8>] (of_platform_populate) from [<c08b1f04>] (of_platform_default_populate_init+0x8c/0xa8) [<c08b1f04>] (of_platform_default_populate_init) from [<c000a78c>] (do_one_initcall+0x3c/0x164) [<c000a78c>] (do_one_initcall) from [<c087bd9c>] (kernel_init_freeable+0x10c/0x1d0) [<c087bd9c>] (kernel_init_freeable) from [<c0628db0>] (kernel_init+0x8/0xf0) [<c0628db0>] (kernel_init) from [<c00090d8>] (ret_from_fork+0x14/0x3c) Exception stack(0xeec73fb0 to 0xeec73ff8) 3fa0: 00000000 00000000 00000000 00000000 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 Code: e3520000 1afffff7 e12fff1e c0801730 (e5d02000) ---[ end trace cafa8f148e262e80 ]--- Fix this by adding the necessary initialiser. Fixes: fc20ffe1213b ("ARM: keystone: add PM domain support for clock management") Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- This looks to be a latent bug that's been present since the above commit - whether it's a problem depends on the data structure that the linker places after platform_domain_notifier - if the first word of the following structure is NULL, then the bug will be hidden. If it's a pointer to a string, or something that reasonably looks like a string, the bug will be hidden. If it's a value that points at an invalid memory address, then an oops similar to the above will result. arch/arm/mach-keystone/pm_domain.c | 1 + 1 file changed, 1 insertion(+)