* [PATCH] ata: libahci_platform: ahci_platform_find_clk: oops, NULL pointer
@ 2022-12-06 8:34 Anders Roxell
2022-12-06 20:59 ` Serge Semin
2022-12-06 23:38 ` Damien Le Moal
0 siblings, 2 replies; 3+ messages in thread
From: Anders Roxell @ 2022-12-06 8:34 UTC (permalink / raw)
To: hdegoede, axboe, damien.lemoal
Cc: Sergey.Semin, linux-ide, linux-kernel, Anders Roxell, Arnd Bergmann
When booting a arm 32-bit kernel with config CONFIG_AHCI_DWC enabled on
a am57xx-evm board. This happens when the clock references are unnamed
in DT, the strcmp() produces a NULL pointer dereference, see the
following oops, NULL pointer dereference:
[ 4.673950] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[ 4.682098] [00000000] *pgd=00000000
[ 4.685699] Internal error: Oops: 5 [#1] SMP ARM
[ 4.690338] Modules linked in:
[ 4.693420] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.0-rc7 #1
[ 4.699615] Hardware name: Generic DRA74X (Flattened Device Tree)
[ 4.705749] PC is at strcmp+0x0/0x34
[ 4.709350] LR is at ahci_platform_find_clk+0x3c/0x5c
[ 4.714416] pc : [<c130c494>] lr : [<c0c230e0>] psr: 20000013
[ 4.720703] sp : f000dda8 ip : 00000001 fp : c29b1840
[ 4.725952] r10: 00000020 r9 : c1b23380 r8 : c1b23368
[ 4.731201] r7 : c1ab4cc4 r6 : 00000001 r5 : c3c66040 r4 : 00000000
[ 4.737762] r3 : 00000080 r2 : 00000080 r1 : c1ab4cc4 r0 : 00000000
[...]
[ 4.998870] strcmp from ahci_platform_find_clk+0x3c/0x5c
[ 5.004302] ahci_platform_find_clk from ahci_dwc_probe+0x1f0/0x54c
[ 5.010589] ahci_dwc_probe from platform_probe+0x64/0xc0
[ 5.016021] platform_probe from really_probe+0xe8/0x41c
[ 5.021362] really_probe from __driver_probe_device+0xa4/0x204
[ 5.027313] __driver_probe_device from driver_probe_device+0x38/0xc8
[ 5.033782] driver_probe_device from __driver_attach+0xb4/0x1ec
[ 5.039825] __driver_attach from bus_for_each_dev+0x78/0xb8
[ 5.045532] bus_for_each_dev from bus_add_driver+0x17c/0x220
[ 5.051300] bus_add_driver from driver_register+0x90/0x124
[ 5.056915] driver_register from do_one_initcall+0x48/0x1e8
[ 5.062591] do_one_initcall from kernel_init_freeable+0x1cc/0x234
[ 5.068817] kernel_init_freeable from kernel_init+0x20/0x13c
[ 5.074584] kernel_init from ret_from_fork+0x14/0x2c
[ 5.079681] Exception stack(0xf000dfb0 to 0xf000dff8)
[ 5.084747] dfa0: 00000000 00000000 00000000 00000000
[ 5.092956] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 5.101165] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 5.107818] Code: e5e32001 e3520000 1afffffb e12fff1e (e4d03001)
[ 5.114013] ---[ end trace 0000000000000000 ]---
Add an extra check in the if-statement if hpriv-clks[i].id.
Fixes: 6ce73f3a6fc0 ("ata: libahci_platform: Add function returning a clock-handle by id")
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
drivers/ata/libahci_platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index ddf17e2d266c..b9e336bacf17 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -109,7 +109,7 @@ struct clk *ahci_platform_find_clk(struct ahci_host_priv *hpriv, const char *con
int i;
for (i = 0; i < hpriv->n_clks; i++) {
- if (!strcmp(hpriv->clks[i].id, con_id))
+ if (hpriv->clks[i].id && !strcmp(hpriv->clks[i].id, con_id))
return hpriv->clks[i].clk;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ata: libahci_platform: ahci_platform_find_clk: oops, NULL pointer
2022-12-06 8:34 [PATCH] ata: libahci_platform: ahci_platform_find_clk: oops, NULL pointer Anders Roxell
@ 2022-12-06 20:59 ` Serge Semin
2022-12-06 23:38 ` Damien Le Moal
1 sibling, 0 replies; 3+ messages in thread
From: Serge Semin @ 2022-12-06 20:59 UTC (permalink / raw)
To: Anders Roxell
Cc: Serge Semin, hdegoede, axboe, damien.lemoal, Sergey.Semin,
linux-ide, linux-kernel, Arnd Bergmann
On Tue, Dec 06, 2022 at 09:34:16AM +0100, Anders Roxell wrote:
> When booting a arm 32-bit kernel with config CONFIG_AHCI_DWC enabled on
> a am57xx-evm board. This happens when the clock references are unnamed
> in DT, the strcmp() produces a NULL pointer dereference, see the
> following oops, NULL pointer dereference:
>
> [ 4.673950] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [ 4.682098] [00000000] *pgd=00000000
> [ 4.685699] Internal error: Oops: 5 [#1] SMP ARM
> [ 4.690338] Modules linked in:
> [ 4.693420] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.0-rc7 #1
> [ 4.699615] Hardware name: Generic DRA74X (Flattened Device Tree)
> [ 4.705749] PC is at strcmp+0x0/0x34
> [ 4.709350] LR is at ahci_platform_find_clk+0x3c/0x5c
> [ 4.714416] pc : [<c130c494>] lr : [<c0c230e0>] psr: 20000013
> [ 4.720703] sp : f000dda8 ip : 00000001 fp : c29b1840
> [ 4.725952] r10: 00000020 r9 : c1b23380 r8 : c1b23368
> [ 4.731201] r7 : c1ab4cc4 r6 : 00000001 r5 : c3c66040 r4 : 00000000
> [ 4.737762] r3 : 00000080 r2 : 00000080 r1 : c1ab4cc4 r0 : 00000000
> [...]
> [ 4.998870] strcmp from ahci_platform_find_clk+0x3c/0x5c
> [ 5.004302] ahci_platform_find_clk from ahci_dwc_probe+0x1f0/0x54c
> [ 5.010589] ahci_dwc_probe from platform_probe+0x64/0xc0
> [ 5.016021] platform_probe from really_probe+0xe8/0x41c
> [ 5.021362] really_probe from __driver_probe_device+0xa4/0x204
> [ 5.027313] __driver_probe_device from driver_probe_device+0x38/0xc8
> [ 5.033782] driver_probe_device from __driver_attach+0xb4/0x1ec
> [ 5.039825] __driver_attach from bus_for_each_dev+0x78/0xb8
> [ 5.045532] bus_for_each_dev from bus_add_driver+0x17c/0x220
> [ 5.051300] bus_add_driver from driver_register+0x90/0x124
> [ 5.056915] driver_register from do_one_initcall+0x48/0x1e8
> [ 5.062591] do_one_initcall from kernel_init_freeable+0x1cc/0x234
> [ 5.068817] kernel_init_freeable from kernel_init+0x20/0x13c
> [ 5.074584] kernel_init from ret_from_fork+0x14/0x2c
> [ 5.079681] Exception stack(0xf000dfb0 to 0xf000dff8)
> [ 5.084747] dfa0: 00000000 00000000 00000000 00000000
> [ 5.092956] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 5.101165] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [ 5.107818] Code: e5e32001 e3520000 1afffffb e12fff1e (e4d03001)
> [ 5.114013] ---[ end trace 0000000000000000 ]---
>
> Add an extra check in the if-statement if hpriv-clks[i].id.
>
> Fixes: 6ce73f3a6fc0 ("ata: libahci_platform: Add function returning a clock-handle by id")
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
> drivers/ata/libahci_platform.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index ddf17e2d266c..b9e336bacf17 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -109,7 +109,7 @@ struct clk *ahci_platform_find_clk(struct ahci_host_priv *hpriv, const char *con
> int i;
>
> for (i = 0; i < hpriv->n_clks; i++) {
> - if (!strcmp(hpriv->clks[i].id, con_id))
> + if (hpriv->clks[i].id && !strcmp(hpriv->clks[i].id, con_id))
I also thought to add the con_id check or if it's null then return a
first clock descriptor with null id. But since there is no users of
such semantic at this stage it's ok to accept the fix as is. It will
at least prevent the kernel from oopsing on the unnamed clocks.
So thanks for the patch
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
-Serge(y)
> return hpriv->clks[i].clk;
> }
>
> --
> 2.35.1
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ata: libahci_platform: ahci_platform_find_clk: oops, NULL pointer
2022-12-06 8:34 [PATCH] ata: libahci_platform: ahci_platform_find_clk: oops, NULL pointer Anders Roxell
2022-12-06 20:59 ` Serge Semin
@ 2022-12-06 23:38 ` Damien Le Moal
1 sibling, 0 replies; 3+ messages in thread
From: Damien Le Moal @ 2022-12-06 23:38 UTC (permalink / raw)
To: Anders Roxell, hdegoede, axboe
Cc: Sergey.Semin, linux-ide, linux-kernel, Arnd Bergmann
On 12/6/22 17:34, Anders Roxell wrote:
> When booting a arm 32-bit kernel with config CONFIG_AHCI_DWC enabled on
> a am57xx-evm board. This happens when the clock references are unnamed
> in DT, the strcmp() produces a NULL pointer dereference, see the
> following oops, NULL pointer dereference:
>
> [ 4.673950] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [ 4.682098] [00000000] *pgd=00000000
> [ 4.685699] Internal error: Oops: 5 [#1] SMP ARM
> [ 4.690338] Modules linked in:
> [ 4.693420] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.0-rc7 #1
> [ 4.699615] Hardware name: Generic DRA74X (Flattened Device Tree)
> [ 4.705749] PC is at strcmp+0x0/0x34
> [ 4.709350] LR is at ahci_platform_find_clk+0x3c/0x5c
> [ 4.714416] pc : [<c130c494>] lr : [<c0c230e0>] psr: 20000013
> [ 4.720703] sp : f000dda8 ip : 00000001 fp : c29b1840
> [ 4.725952] r10: 00000020 r9 : c1b23380 r8 : c1b23368
> [ 4.731201] r7 : c1ab4cc4 r6 : 00000001 r5 : c3c66040 r4 : 00000000
> [ 4.737762] r3 : 00000080 r2 : 00000080 r1 : c1ab4cc4 r0 : 00000000
> [...]
> [ 4.998870] strcmp from ahci_platform_find_clk+0x3c/0x5c
> [ 5.004302] ahci_platform_find_clk from ahci_dwc_probe+0x1f0/0x54c
> [ 5.010589] ahci_dwc_probe from platform_probe+0x64/0xc0
> [ 5.016021] platform_probe from really_probe+0xe8/0x41c
> [ 5.021362] really_probe from __driver_probe_device+0xa4/0x204
> [ 5.027313] __driver_probe_device from driver_probe_device+0x38/0xc8
> [ 5.033782] driver_probe_device from __driver_attach+0xb4/0x1ec
> [ 5.039825] __driver_attach from bus_for_each_dev+0x78/0xb8
> [ 5.045532] bus_for_each_dev from bus_add_driver+0x17c/0x220
> [ 5.051300] bus_add_driver from driver_register+0x90/0x124
> [ 5.056915] driver_register from do_one_initcall+0x48/0x1e8
> [ 5.062591] do_one_initcall from kernel_init_freeable+0x1cc/0x234
> [ 5.068817] kernel_init_freeable from kernel_init+0x20/0x13c
> [ 5.074584] kernel_init from ret_from_fork+0x14/0x2c
> [ 5.079681] Exception stack(0xf000dfb0 to 0xf000dff8)
> [ 5.084747] dfa0: 00000000 00000000 00000000 00000000
> [ 5.092956] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 5.101165] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [ 5.107818] Code: e5e32001 e3520000 1afffffb e12fff1e (e4d03001)
> [ 5.114013] ---[ end trace 0000000000000000 ]---
>
> Add an extra check in the if-statement if hpriv-clks[i].id.
>
> Fixes: 6ce73f3a6fc0 ("ata: libahci_platform: Add function returning a clock-handle by id")
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Applied to for-6.1-fixes. Thanks !
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-06 23:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-06 8:34 [PATCH] ata: libahci_platform: ahci_platform_find_clk: oops, NULL pointer Anders Roxell
2022-12-06 20:59 ` Serge Semin
2022-12-06 23:38 ` Damien Le Moal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).